Abandoned worker processes wedge interpreter exit via the multiprocessing atexit join — Closes #294#305
Draft
conradbzura wants to merge 4 commits into
Draft
Conversation
A worker abandoned by pool teardown never exits on its own: its parent-death watchdog only fires once the parent is gone, while multiprocessing's atexit handler joins every live non-daemonic child, leaving the parent blocked in os.waitpid forever. Daemonic children are terminated before that join instead, and the worker's SIGTERM handler turns the termination into an immediate graceful stop, so the join completes promptly. The watchdog still covers the reverse direction, so neither process can outlive the other. Callers that need workers to spawn multiprocessing children of their own can override with daemon=False.
Unit tests pin the daemonic default and the explicit daemon=False override. Integration tests add a regression guard that abandons a live worker at interpreter exit and requires a clean, prompt parent exit — it wedges without the daemonic default — plus a check that SIGTERM produces a graceful zero-exit stop, the mechanism the atexit terminate-then-join relies on.
MockWorker.stop did not accept the timeout keyword WorkerPool passes at teardown, so the pool's stop call raised TypeError into gather(return_exceptions=True) and mock workers were never actually stopped — leaving the teardown test asserting nothing. Accept the keyword and assert workers are genuinely stopped. Also cover the previously untested abandonment branches: a stop that raises TimeoutError counts as abandoned, the configured shutdown timeout is forwarded to each worker's stop, a None timeout waits unbounded without a warning, and the abandonment warning names the abandoned worker's uid.
A heavy example on a loaded machine could overrun the shared 30s per-example bound, and the resulting cancellation is exactly what strands live workers in pool teardown. Give the exploration its own 90s bound; the deterministic pairwise suite keeps 30s.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WorkerPoolteardown abandons a worker that fails to stop withinshutdown_timeout— by design. ButWorkerProcessextendsmultiprocessing.Processwithout the daemon flag, and multiprocessing registers an atexit handler that joins every live non-daemonic child. An abandoned worker never exits on its own: its parent-death watchdog fires only when the parent dies, which the join prevents. The parent blocks inos.waitpidindefinitely, and a one-test flake becomes a whole-suite hang.Spawn
WorkerProcessdaemonic by default. The same atexit handler terminates daemonic children before joining them, and the worker's SIGTERM handler already turns that termination into an immediate graceful stop, so the join completes promptly. The parent-death watchdog covers the reverse direction, so neither process outlives the other regardless of which exits first.One trade-off: daemonic processes cannot spawn multiprocessing children, so a routine that must create subprocesses requires a worker constructed with
daemon=False. The class docstring documents the contract and the escape hatch.Two adjacent hardenings ride along: the load-sensitive Hypothesis dispatch exploration gets its own 90s per-example bound (its timeout cancellation is what strands workers in the first place), and a coverage audit of the pool teardown suite surfaced a silently broken mock, repaired here with the previously untested abandonment branches now covered.
Closes #294
Proposed changes
Spawn workers daemonic
Set
daemon=TrueinWorkerProcess.__init__viakwargs.setdefault, so an explicitdaemon=Falsestill wins. Document the contract in the class docstring: atexit terminates daemonic children before joining them, the SIGTERM handler turns that into a graceful stop, the watchdog guarantees the reverse direction, and daemonic workers cannot spawn multiprocessing children of their own.Pin the contract at both levels
Add unit tests for the daemonic default and the
daemon=Falseoverride. Add an integration regression guard: a helper process starts aLocalWorkerand reaches interpreter exit without stopping it; the guard requires a clean exit within a bound and the worker terminated. Without the daemon default the guard wedges for its full 30s bound. Add a SIGTERM test asserting a started worker exits with code 0 inside the grace window — the graceful-stop mechanism the atexit terminate-then-join relies on. The helper scripts must remainpython -cstrings: they carry no__main__guard, so as files the spawn bootstrap would re-run them in every worker child.Repair hollow pool teardown coverage
MockWorker.stopdid not accept thetimeoutkeyword the pool passes at teardown; theTypeErrordisappeared intogather(return_exceptions=True), so mock workers were never actually stopped and the teardown test asserted nothing. Accept the keyword, rewrite the test to assert workers are genuinely stopped, and cover the remaining abandonment branches: a stop that raisesTimeoutErrorcounts as abandoned,shutdown_timeoutis forwarded to each worker's stop,shutdown_timeout=Nonewaits unbounded, and the abandonment warning names the abandoned uid.Give the Hypothesis exploration timeout headroom
Split
_HYPOTHESIS_TIMEOUT = 90from the shared 30s bound. A heavy example on a loaded machine overran the shared bound, and the resultingasyncio.timeoutcancellation is exactly the abandonment path this PR hardens. The deterministic pairwise suite keeps 30s.Test cases
TestWorkerProcessWorkerProcessis instantiatedTestWorkerProcessdaemon=FalsekwargWorkerProcessis instantiatedTestWorkerOrphanPreventionLocalWorkerand never stops itTestWorkerOrphanPreventionWorkerProcessTestWorkerPoolTestWorkerPoolTestWorkerPoolTimeoutErrorunder a generous boundTestWorkerPoolshutdown_timeout=5.0timeout=5.0TestWorkerPoolshutdown_timeout=Nonetimeout=Noneand no warning is emitted