Skip to content

jhm: bound the test-output echo on every reap, not just timed-out ones (#570) - #571

Merged
ohohoreilly merged 1 commit into
masterfrom
ohohoreilly/570-bounded-test-echo
Sep 18, 2026
Merged

ohohoreilly merged 1 commit into
masterfrom
ohohoreilly/570-bounded-test-echo

Conversation

@ohohoreilly

Copy link
Copy Markdown
Member

jhm: bound the test-output echo on every reap, not just timed-out ones (#570)

jhm --test can hang forever reaping a test that has ALREADY EXITED, taking
the rest of the suite with it and burning the whole CI job with no diagnostic.
An aarch64 run sat in it for 110 minutes with 61 tests still queued, having
printed one line past the previous progress update:

TESTS: 4 running, 61 queued
TEST: orly/server/import_replication.test

No TIMEOUT:, no EXITCODE:, no echoed output.

The per-test timeout cannot catch this. Its sweep only runs when TryWaitAll()
returns nothing; the absence of a "TIMEOUT: ... still running" line proves the
test exited on its own, putting jhm in the reap where the deadline no longer
applies. It then blocked in Base::EchoOutput, which reads to EOF -- and EOF on
a pipe needs EVERY write end closed. import_replication.test forks orlyi
servers; any that outlive the test still hold its stdout.

EchoOutputBounded already exists for precisely this, written for #537, and its
comment names the failure exactly. It was simply scoped too narrowly: only the
SIGKILLed branch used it, on the assumption that only a killed test strands
children. A test that exits on its own can stand up servers and fail before
reaping them just as easily.

So use it on both paths. It costs nothing in the normal case -- a pipe whose
writers are all gone polls readable immediately and reads zero, so the quiet
period is only ever spent in the pathological case it exists for.

Verified both directions with a probe that forks a "test" which spawns a
grandchild holding stdout and then exits non-zero: the read-to-EOF shape hangs
and has to be killed, the bounded one returns in 2s and still prints the test's
output. jhm rebuilds clean via bootstrap.

#570)

`jhm --test` can hang forever reaping a test that has ALREADY EXITED, taking
the rest of the suite with it and burning the whole CI job with no diagnostic.
An aarch64 run sat in it for 110 minutes with 61 tests still queued, having
printed one line past the previous progress update:

    TESTS: 4 running, 61 queued
    TEST: orly/server/import_replication.test

No TIMEOUT:, no EXITCODE:, no echoed output.

The per-test timeout cannot catch this. Its sweep only runs when TryWaitAll()
returns nothing; the absence of a "TIMEOUT: ... still running" line proves the
test exited on its own, putting jhm in the reap where the deadline no longer
applies. It then blocked in Base::EchoOutput, which reads to EOF -- and EOF on
a pipe needs EVERY write end closed. import_replication.test forks orlyi
servers; any that outlive the test still hold its stdout.

EchoOutputBounded already exists for precisely this, written for #537, and its
comment names the failure exactly. It was simply scoped too narrowly: only the
SIGKILLed branch used it, on the assumption that only a killed test strands
children. A test that exits on its own can stand up servers and fail before
reaping them just as easily.

So use it on both paths. It costs nothing in the normal case -- a pipe whose
writers are all gone polls readable immediately and reads zero, so the quiet
period is only ever spent in the pathological case it exists for.

Verified both directions with a probe that forks a "test" which spawns a
grandchild holding stdout and then exits non-zero: the read-to-EOF shape hangs
and has to be killed, the bounded one returns in 2s and still prints the test's
output. jhm rebuilds clean via bootstrap.
@ohohoreilly ohohoreilly self-assigned this Sep 18, 2026
@ohohoreilly
ohohoreilly merged commit 8fecf43 into master Sep 18, 2026
10 checks passed
@ohohoreilly
ohohoreilly deleted the ohohoreilly/570-bounded-test-echo branch September 18, 2026 06:07
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