Skip to content

HOLD (owner area #32): fix(net): stop and join pw-reaper threads at interpreter exit - #53

Open
wikithoughts wants to merge 1 commit into
mainfrom
overnight/reaper-shutdown-safe
Open

wikithoughts wants to merge 1 commit into
mainfrom
overnight/reaper-shutdown-safe

Conversation

@wikithoughts

Copy link
Copy Markdown
Owner

HOLD: owner area (#32). passiveworkers/net/store.py was last shaped by the owner-reviewed ledger PR #32. The owner reviews this before merge. The overnight coordinator will not merge it.

What changed

  • passiveworkers/net/store.py (+29 lines)
    • Every Store adds itself to a module-level weakref.WeakSet. The set survives importlib.reload, which the tests do constantly.
    • One atexit hook, registered once even across reloads, sets each Store's _stop and then joins every pw-reaper thread under one shared 5 s deadline.
    • A reaper stuck mid-pass can delay exit by at most 5 s and can never wedge it.
    • This is the same stop+join pattern fix(research): hard wall-clock fetch deadline that also cuts header and gzip drips #48 used: its _FetchWatchdog.close() cancels and joins the timer.
  • _store_jobs.py, ledger.py and the other _store_*.py files are untouched.
  • tests/test_reaper_shutdown.py (new) runs a real child interpreter with a temp SQLite DB and no network.
    • The child shortens the reaper tick and makes every pass fail with a 2 MB message.
    • The parent holds off reading the child's stdout, so the reaper is blocked in print(), holding stdout's buffer lock, when the child's main thread returns.
    • [plain] and [after-module-reload]: the child must exit 0 with no Fatal Python error.
    • test_a_stuck_reaper_cannot_wedge_interpreter_exit: a pass that never returns must not hang exit.

Finding addressed

Backlog item from the #48 follow-up: "pw-reaper threads in _store_jobs.py carry the same 3.10 shutdown hazard as the baseline thread".

  • A reaper that is mid-print at finalization aborts CPython with Fatal Python error: _enter_buffered_busy: could not acquire lock for <_io.BufferedWriter name='<stdout>'> at interpreter shutdown, possibly due to daemon threads.
  • This is the same crash as test (3.10) in CI run 38124448503.
  • About 170 idle reapers are alive at the end of the test suite.

Evidence that the test catches the hazard

Code Python Result
old main (a48f06a) 3.12.3 [plain] and [after-module-reload] FAIL with the exact _enter_buffered_busy fatal error, rc -6
old main 3.10.21 same 2 FAIL
old main, standalone repro 3.10 / 3.11 / 3.12 / 3.13 aborts 6/6, 3/3, 6/6, 3/3
first version of this fix, without the reload guard 3.12.3 [after-module-reload] FAIL. An adversarial review found this, and the reload guard was added
this PR 3.12.3 and 3.10.21 3 passed
this PR, standalone repro 3.10 / 3.11 / 3.12 / 3.13 0 aborts in 6/3/6/3 runs

The stuck-reaper test also passes on old code, because a daemon thread never blocked exit there. It guards the new join's bound, not the crash.

Verify (final content, Python 3.12.3)

Command Exit Counts
ruff check . && pytest tests/ -q --cov=passiveworkers --cov-fail-under=75 (declared gate) 0 All checks passed; 810 passed, 1 skipped; 83.57%
python -m py_compile $(git ls-files '*.py') 0 –
pyright (blocking for passiveworkers/net) 0 0 errors, 0 warnings

No UI was touched, so no browser check was needed.

Unverified

  • Full suite on 3.10/3.11/3.13/3.14 not run locally. Only the new test file was run on 3.10, and only the standalone repro on 3.11 and 3.13. Python 3.14 is not installed on this host. CI's matrix covers these.
  • CI not observed. Polling CI was out of scope for this run.
  • The 5 s join deadline is a judgment call. If a reaper is still stuck after 5 s, finalization proceeds and the old hazard can still bite. The fix removes the realistic window, not every possible one.
  • The test assumes a timing. It assumes the reaper reaches print() within the child's 0.3 s sleep. On an extremely slow runner it could pass falsely on old code, but it cannot fail falsely on new code.

🤖 Generated with Claude Code

A pw-reaper daemon thread that is mid-print (its failed-pass log line) when
the interpreter finalizes holds stdout's buffer lock, and CPython aborts with
"Fatal Python error: _enter_buffered_busy". This is the same hazard #48 fixed
for the pw-baseline thread. store.py now tracks live Stores in a WeakSet that
survives importlib.reload, and a single atexit hook sets each _stop and joins
every reaper under one shared 5 s deadline.

tests/test_reaper_shutdown.py runs a child interpreter whose reaper is blocked
in print() at exit, both plain and after a module reload. It aborts on the old
code on 3.10-3.13 and exits 0 with the fix. A third test checks that a stuck
reaper cannot wedge exit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant