Exit cleanly on SIGTERM instead of crashing during teardown - #207
Merged
willyborankin merged 1 commit intoAug 25, 2026
Merged
Conversation
Stopping journalpump killed it with a traceback and exit code 1, which
systemd records as a service failure:
KeyError: 10
journalpump.py run -> journalpump.py _close_stale_readers
-> journalpump.py unregister_from_poll -> select.poll.unregister
unregister_from_poll treated "fd is a key in reader_by_fd" as "fd is
still in the poller". Unregistering writes _STALE_FD and leaves the
key; only the run loop deletes it after processing poll results.
shutdown() unregisters every reader and queues it stale;
_close_stale_readers() unregisters the queue again. If SIGTERM arrives
after the loop already purged those markers, the second unregister
raises KeyError and systemd records exit 1.
Key the guard on ownership (`get(fd) is reader`) so a stale marker no
longer looks registered, and a recycled fd belonging to a different
reader is left alone. Deleting the key would also make the guard
honest, but the marker is how the loop tells "unregistered during this
batch" from "never ours". Dropping the second unregister would satisfy
today's callers and leave the trap armed.
The test raises SIGTERM at the end of the first iteration, where the
traceback placed it.
Co-authored-by: Cursor <cursoragent@cursor.com>
mbertheau
force-pushed
the
mbertheau-shutdown-double-unregister
branch
from
August 25, 2026 07:54
a8da352 to
7a8522d
Compare
mbertheau
marked this pull request as ready for review
August 25, 2026 07:55
willyborankin
approved these changes
Aug 25, 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
2 participants
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.
Stopping journalpump killed it with a traceback and exit code 1, which systemd records as a service failure:
unregister_from_polltreated "fd is a key inreader_by_fd" as "fd is still in the poller". Unregistering writes_STALE_FDand leaves the key; only the run loop deletes it, after the poll results are processed.shutdown()unregisters every reader and also queues it stale, and_close_stale_readers()unregisters that queue again, so a SIGTERM arriving after the loop already purged its markers makes the second unregister raise.Keying the guard on ownership (
get(fd) is reader) makes unregistering idempotent, and leaves alone an fd number that has meanwhile been recycled by a different reader. The regression test drives the real run loop and raises SIGTERM at the end of the first iteration, where the reported traceback places it.Stacked on #206, so until that merges the diff here carries its commits too. The fix itself is the single commit a8da352.
Made with Cursor