From 9a4703aa78ad6bb700da92a939934631a1fcd524 Mon Sep 17 00:00:00 2001 From: Sky Eckstrom Date: Thu, 16 Jul 2026 17:41:55 -0400 Subject: [PATCH] fix: don't mistake a dead decode thread for EOF 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) --- lddecode/core.py | 22 ++++- tests/test_decodefield_errors.py | 140 +++++++++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 tests/test_decodefield_errors.py diff --git a/lddecode/core.py b/lddecode/core.py index df7437b40..d9d3d3af7 100644 --- a/lddecode/core.py +++ b/lddecode/core.py @@ -4030,6 +4030,9 @@ def decodefield(self, start, mtf_level, prevfield=None, initphase=False, redo=Fa rv['field'] = None rv['offset'] = None + # Always present so readfield() can test it without a .get() dance; see + # the handler around lpf_wrapper() below for why it exists. + rv['exception'] = None readloc = int(start - self.rf.blockcut) if readloc < 0: @@ -4080,7 +4083,16 @@ def decodefield(self, start, mtf_level, prevfield=None, initphase=False, redo=Fa except (KeyboardInterrupt, SystemExit): raise except Exception as e: - raise e + # When decodefield() runs on a worker thread (see readfield), raising + # only unwinds that thread -- the exception never reaches the main one. + # rv is then left holding the field/offset None sentinels set above, + # which is exactly what a genuine end-of-input returns (see the + # `rawdecode is None` early return), so readfield() cannot tell a dead + # worker from EOF and the decode ends early, successfully, with a + # truncated .tbc.json. Hand the exception back on rv so readfield() can + # re-raise it on the main thread. + rv['exception'] = e + raise rv['field'] = f rv['offset'] = f.nextfieldoffset - (readloc - rawdecode["startloc"]) @@ -4146,6 +4158,14 @@ def readfield(self, initphase=False): # In non-threaded mode self.threadreturn was filled earlier... # ... but if the first call, this is empty if len(self.threadreturn) > 0: + # A decode thread that died left field/offset as None, which is + # indistinguishable from the EOF sentinel and would be mistaken + # for one below ("if f is None and offset is None"), ending the + # decode early with a zero exit code. Re-raise on the main thread + # instead, where main()'s handler can report it and exit non-zero. + if self.threadreturn['exception'] is not None: + raise self.threadreturn['exception'] + f, offset = self.threadreturn['field'], self.threadreturn['offset'] # Start new thread diff --git a/tests/test_decodefield_errors.py b/tests/test_decodefield_errors.py new file mode 100644 index 000000000..017a3b7cc --- /dev/null +++ b/tests/test_decodefield_errors.py @@ -0,0 +1,140 @@ +"""Tests that a failed decode thread is not mistaken for end-of-input. + +decodefield() pre-populates its result dict with field/offset None sentinels +before doing any work. Those sentinels are also exactly what a genuine EOF +returns, so when decodefield() runs on a worker thread (the default path) and +raises, readfield() used to read the untouched sentinels back as EOF: the decode +stopped early but exited 0 and wrote a .tbc.json whose field count described the +truncated output as complete. + +decodefield() now records the exception on the shared dict and readfield() +re-raises it on the main thread, while a real EOF still reads as EOF. + +This is not specific to any exception type -- the sentinels are pre-populated +before any code that can raise, so anything a field's process() throws is +swallowed the same way. The tests therefore use a meaningless exception class, +with the ZeroDivisionError from the original report kept as a single named case. + +These drive the two functions against stub objects: constructing a real LDdecode +requires an RF source and a populated DemodCache, which is far more scaffolding +than the None-sentinel/exception contract under test needs. +""" + +import types + +import pytest + +from lddecode.core import LDdecode + + +class _DecodeBoom(Exception): + """An arbitrary decode-time failure. + + Deliberately not a builtin: the sentinel/EOF collision swallows *any* + exception raised on the decode thread, so the tests use an exception with no + meaning of its own. The ZeroDivisionError that surfaced this in the field is + covered separately below, as one instance of the general case rather than the + thing being fixed. + """ + + +class _BoomField: + """Stands in for FieldClass; blows up in process() like a real bad field.""" + + def __init__(self, *args, **kwargs): + self.valid = False + + def process(self): + raise _DecodeBoom("field failed to process") + + +def _decodefield_stub(rawdecode, field_class=_BoomField): + """Minimal `self` carrying only what decodefield() touches before process().""" + return types.SimpleNamespace( + rf=types.SimpleNamespace(blockcut=0), + blocksize=32768, + readlen=65536, + demodcache=types.SimpleNamespace(read=lambda *a, **kw: rawdecode), + FieldClass=field_class, + fields_written=0, + wow_level_adjust_smoothing=None, + wow_interpolation_method=None, + curfield=None, + use_profiler=False, + system="NTSC", + ) + + +def _readfield_stub(threadreturn): + """Minimal `self` carrying only what readfield() touches before the raise.""" + return types.SimpleNamespace( + fieldstack=[], + second_decode=None, + fields_written=0, + decodethread=None, + threadreturn=threadreturn, + fdoffset=0, + mtf_level=0, + numthreads=0, + decodefield=lambda *a, **kw: (None, None), + ) + + +def test_decodefield_records_exception_on_rv_and_still_raises(): + """A field that fails to process leaves the exception on the shared dict.""" + rv = {} + stub = _decodefield_stub({"startloc": 0}) + + with pytest.raises(_DecodeBoom): + LDdecode.decodefield(stub, 0, 0, rv=rv) + + assert isinstance(rv["exception"], _DecodeBoom) + # The sentinels the caller would otherwise read as EOF are still in place -- + # the exception is what distinguishes this dict from a real end-of-input. + assert rv["field"] is None + assert rv["offset"] is None + + +def test_decodefield_leaves_exception_none_at_eof(): + """A real EOF (no rawdecode) returns the bare sentinels and no exception.""" + rv = {} + stub = _decodefield_stub(None) + + assert LDdecode.decodefield(stub, 0, 0, rv=rv) == (None, None) + assert rv["exception"] is None + + +def test_readfield_reraises_a_dead_decode_thread(): + """The failure surfaces on the main thread instead of reading as EOF.""" + boom = _DecodeBoom("field failed to process") + stub = _readfield_stub({"field": None, "offset": None, "exception": boom}) + + with pytest.raises(_DecodeBoom) as excinfo: + LDdecode.readfield(stub) + + # The original exception object, so its traceback points at the real cause. + assert excinfo.value is boom + + +def test_readfield_reraises_the_reported_zerodivisionerror(): + """The specific failure this was found through, as one case of the general one. + + A ZeroDivisionError out of a field's process() is what truncated a real + capture at 43% while exiting 0. It gets its own test only so the report has a + regression test that names it -- nothing here is specific to the exception + type, which is the whole point of the fix. + """ + boom = ZeroDivisionError("float division by zero") + stub = _readfield_stub({"field": None, "offset": None, "exception": boom}) + + with pytest.raises(ZeroDivisionError) as excinfo: + LDdecode.readfield(stub) + + assert excinfo.value is boom + + +def test_readfield_still_reports_eof_as_eof(): + """A clean (None, None) with no exception must keep meaning end-of-input.""" + stub = _readfield_stub({"field": None, "offset": None, "exception": None}) + + assert LDdecode.readfield(stub) is None