fix(keeper): roll back partial state when Beacon.start() bind fails (gap-hunt round 2) - #3
Open
SuperInstance wants to merge 1 commit into
Open
fix(keeper): roll back partial state when Beacon.start() bind fails (gap-hunt round 2)#3SuperInstance wants to merge 1 commit into
SuperInstance wants to merge 1 commit into
Conversation
Beacon.start() set self._running = True before sock.bind(), so a failed bind (bad port, in use, permission denied) latched the flag True and left self._sock pointing at an open-but-unusable socket. The object was then bricked: subsequent start() calls early-return on the latched flag, no listener thread exists, and the socket leaks until GC. Defer setting _running until bind succeeds, and on failure close + clear the socket. OverflowError (raised for out-of-range ports) is not a subclass of OSError, so it is caught explicitly. Adds test_start_failure_does_not_latch covering the post-failure state and instance reusability.
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.
Gap-hunt pass (opencode), independently re-verified by me (rebuilt on the exact pushed commit, reran the full suite: 18/18 pass; reverted the fix locally and confirmed the new regression test genuinely fails on pre-fix code).
Bug:
Beacon.start()setself._running = Truebefore callingsock.bind(). If the bind fails (e.g. an out-of-range port, which Python's socket module raises asOverflowError— notably not a subclass ofOSError), the object is left permanently bricked:start()early-returns wheneverself._runningis alreadyTrue, so a failed bind could never be retried, and the socket itself was never cleaned up.Fix: moved
self._running = Trueto after a successfulbind(); on failure, catches bothOSErrorandOverflowError, clearsself._sock, and closes the socket before re-raising — so the Beacon is left in a clean, retryable state rather than a half-initialized one.Verification (independently redone):
PYTHONPATH=src python3 -m pytest tests/ -vmyself: 18/18 pass.keeper.pyto the pre-fix version and reran the newtest_start_failure_does_not_latchtest alone — it genuinely fails on old code (assert True is False—_runningstayed latchedTrue), confirming it's a real regression guard, not a trivial pass.flake8 . --count --select=E9,F63,F7,F82(the real CI gate) is clean; the broader style-only flake8 pass shows pre-existing warnings unrelated to this change.