fix(daemon_pool): guard against missing _initializer attr on Python 3.14 - #59157
fix(daemon_pool): guard against missing _initializer attr on Python 3.14#59157wesleysimplicio wants to merge 1 commit into
Conversation
Python 3.14 removed the private _initializer attribute from ThreadPoolExecutor. Direct access via self._initializer crashes DaemonThreadPoolExecutor._adjust_thread_count with AttributeError. Use getattr with a safe default (None / empty tuple) so the class works transparently on both CPython 3.8-3.13 and 3.14+. Closes NousResearch#58596.
Duplicate of #57459 — both patch |
AmirF194
left a comment
There was a problem hiding this comment.
Thanks for jumping on #58596. I dug into this against the 3.14 root cause and I do not think this change actually fixes it, so I would hold off.
The issue is not just that _initializer/_initargs are gone in 3.14, the _worker function itself changed shape. On 3.13 and earlier it is _worker(executor_ref, work_queue, initializer, initargs) (4 params); on 3.14 it is _worker(executor_ref, ctx, work_queue) (3 params, with a WorkerContext). This PR still passes a 4-element args tuple and still puts self._work_queue in the second slot. So on 3.14 the thread starts (the AttributeError is gone) but the worker immediately dies with TypeError: _worker() takes 3 positional arguments but 4 were given. That error only shows up on the thread excepthook, and since no worker ever drains the queue, submit(...).result() just hangs. So we would be trading a loud, obvious crash for a silent deadlock, which is harder to debug. I reproduced this by simulating the 3-param _worker locally.
Related: guarding the attribute read with a None/() default also means an initializer that the caller actually passed would be silently dropped on any version where the attr is missing, since the 3.14 value lives in self._create_worker_context(), not in a default.
Worth noting #58699 targets the same file, method, and lines for the same issue and takes the signature-detection approach the issue itself proposes (branch on len(inspect.signature(_worker).parameters) == 3, pass the WorkerContext on 3.14, keep the old args on <=3.13, plus regression tests). The two PRs conflict, and that one handles the arity change correctly. I would suggest closing this in favor of #58699, or reworking this to do the same branching and add a test that fakes the 3-param signature so it is exercised on 3.11-3.13 CI. I ran tests/tools/test_daemon_pool.py in a clean Python 3.11 container (4 passed), which confirms no regression on supported versions, but there is no 3.14 job today and requires-python is capped at <3.14, so nothing here is actually run against the version it targets.
|
Leaving this closed — not due to staleness (that closure reason was inaccurate, an automation bug on our side that missed this PR's existing maintainer review), but because @AmirF194's review above concluded the fix doesn't address the actual root cause. Thanks for taking the time to dig into it. |
Python 3.14 removed the private _initializer attribute from
ThreadPoolExecutor. Direct access via self._initializer crashes
DaemonThreadPoolExecutor._adjust_thread_count with AttributeError.
Use getattr with a safe default (None / empty tuple) so the class
works transparently on both CPython 3.8-3.13 and 3.14+.
Closes #58596.