Skip to content

fix(utils): let daemon executor workers exit without waiting - #1029

Open
u7-u7 wants to merge 1 commit into
modelscope:mainfrom
u7-u7:fix/daemon-executor-process-exit
Open

u7-u7 wants to merge 1 commit into
modelscope:mainfrom
u7-u7:fix/daemon-executor-process-exit

Conversation

@u7-u7

@u7-u7 u7-u7 commented Oct 27, 2026

Copy link
Copy Markdown

Change Summary

Prevent DaemonThreadPoolExecutor workers from blocking interpreter exit after a tool call times out and cleanup calls shutdown(wait=False, cancel_futures=True).

The executor already marks its threads as daemons, but also registers them in CPython's concurrent.futures.thread._threads_queues. CPython's interpreter-exit hook joins every thread in that registry, including daemon threads. A stuck synchronous network call can therefore keep the CLI alive indefinitely despite non-waiting shutdown. The web-search and content-optimizer paths use this executor specifically to avoid that behavior.

Do not register these daemon workers in the interpreter-exit join registry. Explicit shutdown(wait=True) continues to join self._threads, and ordinary future results, exceptions and queued-work cancellation remain unchanged. Pending work may be abandoned on interpreter exit, as required by this executor's existing documented daemon contract.

Adds four regression tests, including an actual child interpreter with a blocked worker. On the base revision, it prints shutdown returned but exceeds the 15-second process-exit timeout. With the fix, it exits normally. No executor or threading mocks are used.

Related issue number

No existing matching issue or PR found in the duplicate search.

Validation

Tested on CPython 3.12.14 with the SDK installed from source:

  • python -m pytest tests/utils/test_thread_util.py -q: 4 passed (before fix: 1 failed, 3 passed).
  • Focused utility subset (new executor tests, atomic files, workspace context and policy): 21 passed.
  • python -m pytest tests/utils -q: 151 passed, 30 subtests passed, 1 unrelated baseline failure. TestAgentToolTranscript.test_save_transcript_writes_message_objects_and_dicts expects subagents/worker-1.jsonl, while current code writes .ms_agent/subagents/worker-1.jsonl. Reproduced with the original unmodified executor as well.
  • pre-commit install completed. pre-commit run --files ms_agent/utils/thread_util.py tests/utils/test_thread_util.py: all applicable hooks passed.
  • pre-commit run --all-files: run, but not passing on the base tree's existing lint/formatting problems. Unrelated autoformat changes were discarded; only the executor and regression test are included.
  • git diff --check: passed.

Not tested: full repository suite, real external search/LLM calls, Windows, or other Python versions. This class already depends on CPython's private _worker implementation; this change does not add a new private dependency.

AI-generated change and tests, independently reviewed by another automated reviewer. No human-review attestation is claimed.

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Run pre-commit install and pre-commit run --all-files before git commit, and passed lint check. (Run; full-repo baseline failures disclosed above. Changed-file checks pass.)
  • Documentation reflects the changes where applicable (existing daemon contract preserved; rationale added inline)

@u7-u7
u7-u7 marked this pull request as ready for review October 28, 2026 03:12

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant