Repository navigation
fix: act on worker signals only for the app that owns the container - #44
Merged
Merged
Conversation
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.
Closes #43
setup_diconnected its handlers to the four worker signals with no filter, so in a process with two apps one worker's signals opened and closed both containers. Each handler now checks that its app owns the container before acting.worker_initandworker_shutdown: Celery sends these with theWorkControlleras sender (celery/worker/worker.py). The handler readssender.app, resolves a proxy to the real app, and acts only when that app's root container is this one. A sender with noappfalls back to the current app, which keeps the existingsender=Nonetest working.worker_process_initandworker_process_shutdown: Celery sends these withsender=None(celery/concurrency/prefork.py,solo.py). Under prefork they run in the child afterprocess_initializerhas made the worker's app current; under solo,worker_process_initruns in the main process, where the current app is the worker's app unless it was created withset_as_current=False. The handler checks the current app. The init handler still binds its container to a spawn child's unpickled copy bymainfirst, then checks ownership, so the spawn path works as before.Ownership is a lookup in the existing
_ROOT_CONTAINERSregistry, so an app that already has a container is never claimed by another app with the samemain.One behavior change for a single app: under the solo pool, an app created with
set_as_current=Falsethat is not current no longer opens onworker_process_init. It still opens onworker_init, which every pool sends with the worker as sender.The binding step moved to a module-level helper to keep
setup_diunder ruff's complexity limit. The_restore_current_appfixture moved fromtests/test_spawn.pytotests/conftest.pyso the new tests can change the current app without leaking it.Tests: two new tests in
tests/test_lifespan.py, one per signal pair, failed onmainand pass now. I also ran the repro from the issue and a realWorkControllerwith the threads pool: only the owning app's container opens and closes.