Don't mistake a failed decode thread for end-of-input#1
Closed
skyeckstrom wants to merge 1 commit into
Closed
Conversation
decodefield() pre-populates its result dict with field/offset None sentinels before any code that can raise. Those sentinels are also exactly what a genuine end-of-input returns (the `rawdecode is None` early return), so when decodefield() runs on a worker thread and raises, readfield() reads the untouched sentinels back and cannot tell the two apart: the (None, None) flows into the "EOF, probably" return, main.py sets done=True, prints "Completed: saving JSON and exiting" and exits 0. The traceback is still printed by the default threading excepthook, but the decode is silent in exit code, control flow and the .tbc.json -- which records the truncated field count as authoritative, so downstream tools treat a partial capture as a complete one. main.py's bug-report handler never fires because no exception ever crosses the main thread. Record the exception on the shared dict and re-raise it in readfield() after the join, before the sentinels can be read as EOF. The redo path is untouched: it calls decodefield() directly on the main thread, where exceptions already propagate, and it discards the speculative thread's result by design. This is deliberately minimal and makes no structural change to the threading, so the planned DAG/controller rewrite can drop it wholesale. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Checklist
Description
decodefield()setsrv['field'] = None/rv['offset'] = Nonebefore any code that can raise —the same values the EOF path returns. On the worker thread, an exception leaves those untouched, so
readfield()reads(None, None)and treats a dead thread as end-of-input: the decode exits 0 witha truncated
.tbc. This records the exception onrvand re-raises it after the join.Motivation
A decode-time exception on the worker thread silently truncates the output — exit 0, and a
.tbc.jsonreporting the short length as the whole tape. It hit me as 40546 of 98135 frames (41%)reported as complete. (The trigger in my case, a
_sync_to_burstdivide-by-zero, has since beenfixed in oyvindln
e79332f0— but any decode-thread exception has the same effect, which is why thefix belongs here.) Same event as oyvindln#311.
Related Issues
Fixes happycube#1051
Changes Made
decodefield(): initrv['exception'] = Nonewith the other sentinels; set it in the existingexcept Exceptionhandler (which was a no-opraise e) before re-raising.readfield(): on the threaded path, re-raise a recorded exception after the join, before(None, None)is read as EOF.tests/test_decodefield_errors.py(new).The
redopath is untouched — it runsdecodefield()on the main thread (exceptions alreadypropagate) and discards the speculative result by design.
Testing
pytest)tests/test_decodefield_errors.py— 5 tests; 4 fail on unpatchedmain(a deadthread reads as EOF), all 5 pass with the change.
Screenshots (if applicable)
N/A.
Additional Notes
Deliberately minimal and structure-free, so the DAG/controller rewrite noted at
core.py:4126candelete it wholesale. On failure the traceback now prints twice (worker excepthook + the re-raise);
suppressing the first would mean overriding
threading.excepthook, out of scope here. Written withAI assistance (disclosed, as is normal in this repo).