Skip to content

fix(tools): rebuild DaemonThreadPoolExecutor on public Executor ABC - #96000

Open
Aoshi-Dev wants to merge 1 commit into
NousResearch:mainfrom
Aoshi-Dev:fix/daemon-pool-py314-executor
Open

Aoshi-Dev wants to merge 1 commit into
NousResearch:mainfrom
Aoshi-Dev:fix/daemon-pool-py314-executor

Conversation

@Aoshi-Dev

Copy link
Copy Markdown

Summary

tools/daemon_pool.py's DaemonThreadPoolExecutor subclassed ThreadPoolExecutor and mirrored its private _adjust_thread_count to get daemon workers (so a wedged tool call can't block interpreter exit — see the module's own docstring for the full rationale). CPython 3.14 restructured those private internals (__init__ now stores a _create_worker_context indirection instead of _initializer/_initargs), which breaks the override with AttributeError: 'DaemonThreadPoolExecutor' object has no attribute '_initializer' on every single submit() call under 3.14.

Concretely: any install that ends up running under a 3.14 interpreter (a stray venv, a future managed-runtime migration, etc.) gets every concurrent tool call failing at once, since all four call sites (agent/conversation_compression.py, agent/memory_manager.py, agent/relay_runtime.py, agent/tool_executor.py) go through this class.

Fix

Rebuilds the same daemon-worker behavior directly on the public concurrent.futures.Executor ABC using only stable primitives (threading, queue.SimpleQueue, weakref, concurrent.futures.Future) — no CPython-private ThreadPoolExecutor internals at all, so a future internals change can't break it again. Same semantics preserved: workers are daemon threads never registered in concurrent.futures.thread._threads_queues (so the atexit join hook can't block exit on a wedged worker), lazy idle-thread reuse, initializer/initargs support, and the weakref-triggered auto-cleanup if a pool is dropped without an explicit shutdown().

Test plan

  • Existing suite (tests/tools/test_daemon_pool.py) passes unchanged on 3.13.12
  • Verified directly against a 3.14.4 interpreter (the exact version that broke the original) — the same test scenarios pass
  • Manual checks for behavior not covered by the existing suite: cancel_futures=True, initializer/initargs, exception propagation through Future.result(), submit-after-shutdown() raising RuntimeError, context-manager protocol

ThreadPoolExecutor._adjust_thread_count is CPython-private and changed
shape in 3.14 (initializer/initargs moved behind a new
_create_worker_context indirection), so the subclass override here broke
with `AttributeError: 'DaemonThreadPoolExecutor' object has no attribute
'_initializer'` on every submit under 3.14 — which is how a stray 3.14
venv picked up by the Windows launcher self-heal turned into every tool
call failing in the same session.

Rebuilds the same daemon-worker behavior (never registered in
concurrent.futures.thread._threads_queues, so the atexit join hook can't
block interpreter exit on a wedged worker) directly on the public
Executor ABC with only stable primitives, so a future CPython internals
change can't break it again. Verified against the existing test suite
plus manual checks for cancel_futures, initializer/initargs, exception
propagation, and submit-after-shutdown, on both 3.13.12 and 3.14.4.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
(cherry picked from commit 2594c3e)
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tools Tool registry, model_tools, toolsets labels Aug 27, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

96000 — fix(tools): rebuild DaemonThreadPoolExecutor on public Executor ABC — compatibility fix.

  • tools/daemon_thread_pool.py now inherits from concurrent.futures.Executor ABC (public API) instead of private ThreadPoolExecutor internals — replaces direct _threads/_work_queue access with public submit/shutdown contract, fixing breakage on Python 3.12+ where internals changed.
  • Correctly preserves daemon thread factory and max_workers semantics. shutdown(cancel_futures=...) forwarded correctly (3.9+). No thread leak; existing call sites see same Executor interface.
  • Recommend adding __enter__/__exit__ forwarding. Clean modernization.

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

Labels

comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants