Bound the pty hangup reads with an alarm - #352
Conversation
60785bf to
6ac7308
Compare
The hangup checks in tests/test-pty.c read from a master whose slaves have all closed. Linux answers EIO there, but on a tree without the support nothing fails the read at all: elfuse's keepalive slave holds the host pty open, so the host has no reason to. The read parks, and a run against such a tree stalls instead of reporting. A control run took ten minutes and had to be killed. A test that guards a fix has to fail when the fix is absent. tests/test-devpts.c already takes that position for its pty round trip, and the readv case at the bottom of this file does too, with signal(SIGALRM, ...) plus alarm(10) around the blocking call and alarm(0) once it returns. The first hangup block never got the same treatment. It gets it now. Both reads sit in one window: the second is the wedge, and the drain above it reads the same hung-up master with no deadline of its own. Hoisting the second read out of EXPECT_TRUE is what lets the alarm be cleared before the verdict is printed, as the readv case does. The handler was silent, and its _exit(2) discarded whatever stdout still held, so a wedged run through a pipe produced no output at all -- neither the results collected so far nor a word about why it stopped. It now names the timeout, and stdout is line-buffered, so the log ends with every check that did complete followed by the reason the run went no further. The message goes to stdout rather than stderr because test-matrix.sh's run_elfuse discards stderr, and that lane is the one most likely to meet a timeout with nobody watching. Measured against a tree with proc_pty_master_hung_up stubbed to false, which is what "without the support" means here. Before: the run sat past a 60 second cap having printed nothing. After: the POLLHUP check fails, the drain passes, the EIO read trips the alarm at ten seconds, and the process exits 2 with 37 lines of results behind it. An unmodified tree still reports 70 passed, 0 failed.
6ac7308 to
2848eb0
Compare
jserv
left a comment
There was a problem hiding this comment.
One note outside the diff: the description says "It now names the timeout on stderr", but the handler writes STDOUT_FILENO and the new comment argues at length for stdout. One word to fix in the prose.
| * rather than in one of its own. | ||
| */ | ||
| signal(SIGALRM, hup_on_alarm); | ||
| alarm(10); |
There was a problem hiding this comment.
The alarm stays armed while the drain verdict prints. If the drain read burns most of the window, SIGALRM can fire after that read already returned, and it can land between TEST()'s label (printed without a newline, so still buffered) and the newline EXPECT_TRUE adds. The handler comment claims both guarded reads sit at a line boundary; that holds for the reads, not for the gap between them, and the TIMEOUT line would then name a read that did not block. Clearing at line 850 and re-arming just before the EIO read costs two lines.
Closes #275.
The hangup checks in
tests/test-pty.cread from a master whose slaves haveall closed. Linux answers EIO there, but on a tree without the support nothing
fails the read at all -- elfuse's keepalive slave holds the host pty open, so
the host has no reason to. The read parks, and a run against such a tree stalls
instead of reporting.
tests/test-devpts.calready takes the position that a test guarding a fix hasto fail when the fix is absent, and the
readvcase at the bottom of this filedoes too:
signal(SIGALRM, ...)plusalarm(10)around the blocking call,cleared with
alarm(0)once it returns. The first hangup block never got thesame treatment. This applies it there.
Both reads sit in one window. The second is the wedge; the drain above it reads
the same hung-up master with no deadline of its own. Hoisting the second read
out of
EXPECT_TRUEis what lets the alarm be cleared before the verdict isprinted, as the
readvcase does.Two smaller things came out of measuring it. The handler was silent, and its
_exit(2)discarded whatever stdout still held, so a wedged run through a pipeproduced no output at all -- neither the results collected so far nor a word
about why it stopped. It now names the timeout on stderr, and stdout is
line-buffered, so the log ends with every check that did complete followed by
the reason the run went no further.
Evidence
Measured against a tree with
proc_pty_master_hung_up()stubbed toreturn false, which is what "without the support" means here.master reports POLLHUP once the slave closes FAIL, then the timeout lineAfter, on the stubbed tree:
On an unmodified tree the file still reports
70 passed, 0 failed - PASS,exit 0, in 3 seconds.
make check-formatpasses (clang-format 22.1.8, commentflow),make indentleaves the file unchanged, and the cross build is clean under
-Wall -Wextra.A full
make checkdid not run locally for want ofaarch64-none-elf-as, soeverything outside
test-ptyis on CI.Summary by cubic
Fixes #275 by bounding the hung-up master reads in
tests/test-pty.cwith an alarm: on trees without hangup support those reads previously blocked forever, now the run exits 2 at 10 seconds with a timeout message.Bug Fixes
SIGALRM+alarm(10)pattern from thereadvcase, clearing the alarm before printing the verdict.test-matrix.shdiscards stderr), and line-buffers stdout so the log retains all completed checks when the alarm fires.Written for commit 2848eb0. Summary will update on new commits.