Skip to content

process_messages: don't let a failed error report skip worker teardown - #198

Open
vtjnash wants to merge 1 commit into
masterfrom
jn/worker-crash-exit
Open

process_messages: don't let a failed error report skip worker teardown#198
vtjnash wants to merge 1 commit into
masterfrom
jn/worker-crash-exit

Conversation

@vtjnash

@vtjnash vtjnash commented Aug 12, 2026

Copy link
Copy Markdown
Member

A worker should not outlive its master, but the exit(1) enforcing that sat behind error reporting which fails in exactly the circumstance the exit exists for. start_worker redirects a worker's stderr to its stdout, a pipe held only by the master, so when the master dies the worker's EOFError handling writes to a reader-less pipe and raises EPIPE. @error does not contain that: when catch_exceptions(logger) is false it rethrows, and when true the recovery report in
Base.CoreLogging.logging_error writes to the same broken stream unguarded. The throw escaped message_handler_loop's catch, skipping both exit(1) and the stream closes, and the worker then idled in start_worker's wait loop indefinitely, holding its socket in CLOSE-WAIT.

Arm the exit on completion of the task servicing the connection to pid 1 so it no longer depends on anything in the error path succeeding, and guard the remaining raw stderr writes that gate cleanup in deliver_result, connect_to_peer, and the unidentified-peer branch of message_handler_loop.

A worker should not outlive its master, but the `exit(1)` enforcing that
sat behind error reporting which fails in many of the same circumstance
the exit exists for. Since `start_worker` redirects a worker's stderr to
its stdout, a pipe held only by the master, when the master dies the
worker's `EOFError` handling writes to a reader-less pipe and raises
`EPIPE` killing the unmonitored Task.

The `@error` rethrows when `catch_exceptions(logger)` is false, and even
when true the recovery report in `Base.CoreLogging.logging_error` writes
to the same broken stream unguarded. So the throw escaped
`message_handler_loop`'s `catch`, skipping both `exit(1)` and the stream
closes, and the worker then idled in `start_worker`'s wait loop
indefinitely, with its socket in `CLOSE-WAIT`.

Arm the exit-on-completion of the task servicing the connection to
master so it no longer depends on anything in the error path succeeding,
and guard the remaining raw `stderr` writes that gate cleanup in
`deliver_result`, `connect_to_peer`, and the unidentified-peer branch of
`message_handler_loop`.

Several existing patterns are split into separate functions so we can
drive those from the test to be precise and minimal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.37%. Comparing base (96b42c4) to head (b1cae6f).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/process_messages.jl 58.82% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #198      +/-   ##
==========================================
+ Coverage   79.10%   79.37%   +0.26%     
==========================================
  Files          10       10              
  Lines        1977     2027      +50     
==========================================
+ Hits         1564     1609      +45     
- Misses        413      418       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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