Keep a stop signal from hanging an idle worker - #46
Merged
Merged
Conversation
ox_worker's SIGTERM and SIGINT handler logged and called worker.request_stop(), which sets a threading.Event. Python runs signal handlers on the main thread, and an idle worker's main thread spends its time in Event.wait() on that same Event. A signal that arrived while Event.wait() held the Event's lock ran a handler that waited for that lock on the thread holding it, and the worker hung instead of draining. The handler now only records the signal on a SimpleQueue, which is documented as safe to call from a signal handler, and a second signal calls os._exit(130) with nothing before it. A daemon thread requests the stop and then logs. The handlers are installed before that thread starts, and if it cannot start the previous handlers come back and a signal already queued stops the worker. The supervisor's handler also only records. Its run loop logs, stops, forwards and escalates, including between child starts and while it waits for children to exit after the loop has ended, so a second or third signal still reaches SIGKILL after an error. The supervisor sets OX_SUPERVISOR_PID in each child's environment. The child removes it, arms PR_SET_PDEATHSIG after its handlers are in place, and compares its parent with that pid before every poll, so a child whose supervisor died while it was starting drains having claimed nothing. A child running an older release ignores the variable. The regression test raises SIGTERM from inside Condition.__exit__ on the worker's stop Event, where the lock is still held, so it fails on the old code every time rather than by timing.
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
1 participant
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 #45
A stop signal could hang an idle
ox_workerinstead of draining it. The handler set athreading.Eventwhile the main thread could be insidewait()on that Event with its lock held, andset()then waited on a lock its own thread owned.What changed
queue.SimpleQueue. A helper thread stops the worker and then logs. A second signal still exits with 130, now without logging first, since nothing may stand between the handler and that exit.OX_SUPERVISOR_PID. The child removes the variable, arms the parent-death signal after its handlers are in place, and compares its parent with that pid on every pass. A child whose supervisor died while it was starting now drains having claimed nothing. An older child ignores the variable, so a rollback does not stop children starting.Tests
tests/test_stop_signals.pyruns the handler at the exact moment the Event's lock is held, by wrappingCondition.__exit__and callingsignal.raise_signal, so it fails on the old code every time rather than by timing. Other tests cover the second-signal exit, what the handler touches, the helper's order, the supervisor's signal processing and start-up, and the orphan check through the real command.Beyond the suite, a loop sending SIGTERM to an idle worker ran about 80,000 times on Linux with Python 3.12 and 3.14 without a hang. The old code hung within a few hundred runs.
The changelog has one Changed entry, for the forced exit no longer logging, and three Fixed entries.