Tolerate EBADF in control-bus reads; keep the control watcher alive - #33
Open
TON14 wants to merge 1 commit into
Open
Tolerate EBADF in control-bus reads; keep the control watcher alive#33TON14 wants to merge 1 commit into
TON14 wants to merge 1 commit into
Conversation
A run that completed all rounds and wrote its final reply crashed at shutdown with OSError: [Errno 9] Bad file descriptor raised from os.close() in _read_json_file: the stored exception of the control watcher task resurfaced when the CLI awaited it in the finally block, turning a fully successful run into a traceback and a non-zero exit. Three layers, all consistent with the tolerant-close style the module already uses in its walk-cleanup paths: 1. control_bus: the two remaining bare os.close() calls in finally blocks (_read_json_file, control-log reader) tolerate OSError - a stray double-close elsewhere in the process can recycle the descriptor number between our open and close. 2. cli watch_control: one failed poll costs that poll, not the watcher - a dead watcher would silently stop honoring stop/abort requests for the rest of the run. 3. cli shutdown: an OSError stored in the watcher must not override the outcome of the finished run. tests: _read_json_file returns the parsed payload when its own close raises EBADF (the walk's directory closes stay real).
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.
Fixes #32.
A run that completed all rounds and wrote its final reply crashed at shutdown with
OSError: [Errno 9] Bad file descriptorraised fromos.close()in_read_json_file: the control-watcher task had died on that exception mid-run, and the stored error resurfaced when the CLI awaited the watcher in itsfinallyblock — turning a fully successful run into a traceback and a non-zero exit (full analysis in #32).Three small layers, all consistent with the tolerant-close style
control_busalready uses in its walk-cleanup paths (_walk_nofollow,_ensure_dir_nofollow):control_bus: the two remaining bareos.close()calls infinallyblocks (_read_json_fileand the control-log reader) now tolerateOSError— a stray double-close elsewhere in the process can recycle the descriptor number between our open and close, and crashing the reader over it takes down an otherwise healthy run.cli/watch_control: one failed poll costs that poll, not the watcher — a dead watcher silently stops honoringstop/abortrequests for the rest of the run.clishutdown: anOSErrorstored in the watcher no longer overrides the outcome of the finished run.Testing
tests/test_control_bus_close.py:_read_json_filestill returns the parsed payload when the close of its own descriptor raisesEBADF(directory closes inside the path walk stay real, so the walk itself is untouched).Independent of #28 (different subsystem, different failure mode); branched directly off
mainso the two can be reviewed and merged in any order.🤖 Generated with Claude Code