Skip to content

Reply when a child dies instead of stalling the parent for the full budget - #4

Open
davidoj wants to merge 1 commit into
mainfrom
fix/daemon-stall-on-dead-child
Open

Reply when a child dies instead of stalling the parent for the full budget#4
davidoj wants to merge 1 commit into
mainfrom
fix/daemon-stall-on-dead-child

Conversation

@davidoj

@davidoj davidoj commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

88% of grading wall clock is dead time, not work. This removes it, with no change to any verdict.

The bug

A forked child that exits without writing a result (os._exit, SIGKILL, an RLIMIT kill) leaves its pipe at EOF. Connection.poll() reports EOF as readable, so the daemon takes the recv() branch, recv() raises EOFError, and the outer except Exception: logger.exception(...) swallows it without replying — the else branch that would send "No result from subprocess" is unreachable in that case.

With no response, the parent can't learn the request is over and blocks its entire time budget, even though the daemon knew the child was dead ~2ms after forking.

os._exit(0) inside f    n_tests=10   e2e=22.05s   status=FAILED
kill(self, SIGKILL)     n_tests=10   e2e=22.00s   status=FAILED
infinite loop in f      n_tests=10   e2e=10.01s   status=FAILED   <- correct, alarm-driven

Profiling a 160-call sweep: 211s total = 186s (88%) stalls, 24s genuine user code, ~1s fixed overhead. A dead-child verification went 22.0s → 0.004s; the sweep 211s → 25.1s.

This hits process_exit (67 problems), mock_functionality_replacement, memory-killed resource_exhaustion, and any submission that exits or segfaults — i.e. exactly the exploit types the dataset is built around.

Three fixes, all in the daemon loops

  1. Catch EOFError around recv() and fall through to the no-result reply. This is the main one.
  2. Stamp request_id on the secure loop's replies. The parent correlates on request_id and discards anything that doesn't match, so the secure loop's unstamped "No result from subprocess" was ignored even when it was sent — the same stall by a quieter route. The insecure loop already did this.
  3. Reply best-effort from the outer exception handler. Logging alone meant any unexpected exception cost the parent its whole budget. The send is guarded, so a genuinely absent parent still yields a log line rather than a BrokenPipe.

Safety

Timeout enforcement for merely-slow code is untouched — that path keys off child.is_alive(), and a test asserts runaway code is still bounded.

Verified no semantic change: across 160 verifications (40 problems × {ground truth, exploit} × {secure, insecure}) verdicts are identical before and after. Only the waiting disappears.

Relationship to #3

This is very likely the pressure behind af88072 (cut timeout): capping the budget at 1s bounds the stall, but treats the symptom, and scores slow-but-correct submissions as FAILED. With the stall fixed, a generous budget costs little because it is rarely reached — so this PR and #3 are complementary, and I'd merge this one first.

Testing

New djinn/tests/test_daemon_stall.py — dead child via os._exit and via SIGKILL, on both secure and insecure paths, plus a guard that runaway code is still bounded and that correct code is unaffected. Fails on the parent commit with an 8.02s stall.

Also validated with a correctness+timing harness over 40 real problems: 0 new correctness failures against baseline.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MYYDDvJbdup3v5LPnkcYMW

…udget

A forked child that exits without writing a result (os._exit, SIGKILL, an RLIMIT
kill) leaves its pipe at EOF. `Connection.poll()` reports EOF as *readable*, so
the daemon took the `recv()` branch, `recv()` raised EOFError, and the outer
`except Exception: logger.exception(...)` swallowed it without replying. The
`else` branch that would have sent "No result from subprocess" is unreachable in
that case. With no response, the parent has no way to learn the request is over
and blocks for its entire time budget -- even though the daemon knew the child
was dead within ~2ms of forking.

Profiling a 160-call grading sweep: 211s total, of which 186s (88%) was this
dead time, against 24s of genuine user code. A single dead-child verification
cost 22.0s; it now costs 0.004s.

This hits process_exit (67 problems), mock_functionality_replacement,
memory-killed resource_exhaustion, and any submission that exits or segfaults --
i.e. exactly the exploit types the dataset is built around.

Three fixes, all in the daemon loops:

1. Catch EOFError around `recv()` and fall through to the no-result reply. This
   is the main one.
2. Stamp `request_id` on the secure loop's replies. The parent correlates on
   request_id and discards anything that doesn't match, so the secure loop's
   unstamped "No result from subprocess" was ignored even when it was sent --
   the same stall by a quieter route. The insecure loop already did this.
3. Reply best-effort from the outer exception handler. Logging alone meant ANY
   unexpected exception cost the parent its whole budget. The send is guarded,
   so a genuinely absent parent still yields a log line rather than a
   BrokenPipe.

Timeout enforcement for merely-slow code is untouched: that path keys off
`child.is_alive()` and is covered by a test asserting runaway code is still
bounded.

Verified no semantic change: across 160 verifications (40 problems x
{ground truth, exploit} x {secure, insecure}) the verdicts are identical before
and after -- only the waiting disappears.

Worth noting this is very likely the pressure behind af88072 ("cut timeout"):
capping the budget at 1s bounds the stall, but treats the symptom, and scores
slow-but-correct submissions as FAILED (see the companion timeout PR). With the
stall fixed, a generous budget costs little because it is rarely reached.

Covered by djinn/tests/test_daemon_stall.py, which fails on the parent commit
with an 8.02s stall.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MYYDDvJbdup3v5LPnkcYMW
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


root seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

2 participants