Exit the pool worker after a task raises SystemExit or KeyboardInterrupt - #457
Merged
Merged
Conversation
Since bd2f803 (celery#452) the worker loop catches BaseException around the task call, so the SystemExit raised by the SIGTERM handler during a hard time limit is reported as a task failure and the worker goes back to waiting for jobs. Blocking-pool workers wait inside the inqueue's shared read lock, and the SIGKILL that follows 0.1s later kills the worker while it holds that lock: the replacement blocks on _rlock forever, the pool never processes another job, and terminate() blocks on the same lock. Keep reporting the exception to the caller, but exit the worker afterwards when it is SystemExit or KeyboardInterrupt, so the pool replaces it as it did before celery#452.
auvipy
self-requested a review
September 9, 2026 09:51
auvipy
approved these changes
Sep 9, 2026
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
Since bd2f803 (#452) the worker loop catches
BaseExceptionaround the task call so that a task raising e.g.BaseExceptionis reported to the caller instead of surfacing asWorkerLostError. That also catches theSystemExitraised by billiard's own SIGTERM handler, which changes what happens on a hard time limit:on_hard_timeout()→_trywaitkill()sends SIGTERM to the worker; the handler installed byreset_signals()raisesSystemExitinside the running task.READY, and goes back towait_for_job().SimpleQueue.get_payload(), i.e.with self._rlock: return self._reader.recv_bytes()— holding the queue's shared read lock._trywaitkill()follows up with SIGKILL 0.1 s later, so the worker dies while holding_rlock._rlock.__enter__forever and the pool never processes another job.Pool.terminate()blocks on the same lock (_help_stuff_finish()acquiresinqueue._rlock), so the pool can't be shut down cleanly either.Before #452 the
SystemExitpropagated out ofworkloop(), the worker exited through_do_exit()without touching the queue, and the SIGKILL was harmless.This PR keeps the #452 behaviour (the exception is still reported to the caller) but exits the worker afterwards when the exception is
SystemExitorKeyboardInterrupt, since either means the process was asked to go away. The pool then replaces the worker as it did before #452.How it shows up
With Celery this is a prefork worker (blocking pool, e.g. the
filesystem://transport) that stops executing tasks after the first hardtime_limitkill: the replacement child starts, the next task is "received", and nothing runs until the worker is restarted. On macOS, where billiard 4.3 defaults tospawn, the stuck replacement also survives the parent's shutdown as an orphan blocked insem_wait. AsynPool is not affected because each of its workers has its own inqueue and no shared read lock.Minimal reproduction on
mainwithout Celery:Bisected:
v4.3.0rc1… 001b9f0 are fine, bd2f803 and later stall. A py-spy dump of the stuck replacement:Tests
test_system_exit_from_task_replaces_worker: a task raisingSystemExitis reported to the caller and the worker is replaced (fails onmain: the same pid keeps serving).test_hard_timeout_does_not_stall_pool: after a hard time limit the replacement worker processes the next job (fails onmainwith aTimeoutErroronce the killed worker has been replaced; the test kills the workers itself on failure becauseterminate()would hang).test_base_exception_propagatesfrom Propagate BaseException raised in pool workers #452 still passes. Fullt/unitpasses on Linux (fork) and macOS (spawn).