Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion lddecode/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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"])
Expand Down Expand Up @@ -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
Expand Down
140 changes: 140 additions & 0 deletions tests/test_decodefield_errors.py
Original file line number Diff line number Diff line change
@@ -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
Loading