7.3 KiB
The test_emacs flake — SIGPIPE, and a socket that outlived its process
Closed. It was neither of the two mechanisms this repository had seen
before. It was a third: SIGPIPE.
The symptom
One run of dune test --root . exited 1; the next on the same tree exited 0.
FAIL the client notices a stop nobody asked about
FAIL and the modeline says so, with the condition
Error: error ("flan dev: cannot reconnect to /tmp/build_997a01_dune/flan-emacs-dev.sock: make client process failed: Connection refused, ...")
Which mechanism it was
Neither of the two named in the brief.
-
Not a listen/connect ordering bug.
bindandlistenare adjacent in bothDev.two_processandDev.merged_setup, nothing else creates that path, and the connection that failed was one that had already been working for forty checks. -
Not a timeout too short for a cold build. That failure mode is now reproducible on demand (see The tick budget below) and it produces an entirely different message.
-
It was
SIGPIPE. The daemon answers an editor by callingWire.send, which isUnix.write_substring. Nothing in the OCaml half of the daemon ignoredSIGPIPE, so a reply written into a socket whose reader had gone killed the process outright. In the merged one-process build that process is the program, the compiler and the listener at once — so the program died without ever reporting a stop (the twoFAILs), and the socket file was left on disk with nothing accepting on it (Connection refusedon a path that plainly exists). One event, all three lines.It is not a contrived shape.
flan-dev--live-connectionreopens a dead connection, Emacs tears the old process down when it does, and a reply already on its way out lands in the gap.flan_agent.chad reached the same conclusion from the other side years ago and passesMSG_NOSIGNAL; the half written in OCaml never got the same treatment.
How it was reproduced
A client that sends one request and closes before the reply arrives:
daemon DIED after 1 hit-and-run connections
daemon wait status: 141 (128+13=141 is SIGPIPE)
socket file left behind: YES
and a fresh client gets:
[Errno 111] Connection refused
That is the reported failure, exactly, in under a second and every time. With the fix, 300 such connections leave the daemon serving.
The reproduction is now a check in test_dev.ml, in the robustness daemon's
block: eight hit-and-run connections, then a request proving the session is
still there and still knows what it installed. Against the parent of the fix it
fails with killed by SIGPIPE.
Both transports, and only one of them has a test. ignore_sigpipe is
called from two places and the suite's daemons are all merged, so the check
above covers merged_setup and nothing covers two_process — which NEXT.md
describes as the escape hatch for a machine that cannot build the compiler
object, so it is live code somewhere. Driven by hand: before the fix a
--two-process daemon dies on the first hit-and-run connection with status
141 and leaves its socket behind, exactly as the merged one does; after it,
a hundred of them leave it serving and answering describe. If that call site
is ever removed, nothing in dune test will say so.
The fix
lib/dev.ml, three parts:
ignore_sigpipe ()where the listening socket is bound — in bothtwo_processandmerged_setup. Ignored rather than handled, becauseservealready treats a failed write as a closed connection.- The reply write is guarded. An
exceptioncase on amatchcovers the scrutinee, not the branch body, so theEPIPEthat ignoring the signal produces went straight past the two handlers below it and out of the accept loop — which ended the session just as surely, only quietly. Without this the first fix convertsSIGPIPEintoflan dev: Unix.Unix_error(EPIPE)and the daemon still stops. - The socket is unlinked on the ways out the daemon does not control as well
as the one it does: an
atexitin the mergedmainforexit(3), which is whatflan_rt.c'srt_dietakes on any runtime trap; by hand inflan_agent.c'sdie_now, and inmain's_exit(rc)fallback, both of which skip theatexitchain deliberately. A stale socket can now only come from aSIGKILL.
emacs/flan-dev.el: a refusal on a path that exists is reported as a leftover
socket rather than as Connection refused. The raw message is what sent two
investigations here the wrong way.
test/test_emacs.ml and test/test_repl.ml: the daemon's wait status is kept
instead of discarded, and the program's output is read before the temporary
files are removed rather than after. The old order deleted the evidence and
then printed the exit code — which is why nobody could tell, twice running,
whether the daemon had exited or been killed.
The sibling call sites
Four files are in this family and the answer differs for each:
test_dev.mlandtest_repl.mldrive the same daemon, solib/dev.ml's fix covers them. Neither had a separate copy of the bug.test_cider.mlhas no daemon and no running program — it drives the inspector and break buffer from fixtures. It is not in this family at all, despite the name.test_emacs.mlwas the only one whose harness destroyed the evidence before reading the exit code;test_repl.mlhad the same ordering and is fixed too.
So: one bug, one site, three harnesses — not the three-way split the last one had.
The tick budget — a second hazard, and not this one
test/programs/dev-repl.flan ran (dotimes [i 4000] (agent/wait 5) ...): a
twenty-second program under a two-minute test, and test_emacs drives one
daemon through the whole run. On a loaded machine the client can still be
working when the program reaches its last tick.
This is real and it is reproducible — sweep the budget and the same two
break-loop checks fail at 165, 175 and 185 ticks — but it is not the flake
above, because it fails honestly: the program exited; restart flan dev at one
budget, nothing is listening on ... at another. Never Connection refused.
That is what ruled it out.
Raised to 24000, which is what dev-robust.flan already uses for the same
reason. The 600s watchdogs are what actually bound the run.
Verification
dune test --root .— three runs, 232 checks, 0 failures, exit 0 each time. All three under sixteen busy loops; two of them alongside a runningspike/x86/survey.sh.spike/x86/survey.sh— 103 MATCH / 0 DIFFER / 0 REFUSED.
Open questions for the author
lib/cimport.ml:1138still spells its owncachedirunder$TMPDIR, which NEXT.md:694 flags as the last remnant of the cold-cache bug. Another lane holds that file. Folding it ontoBuild.cachediris still a one-line change.Build.cachediris now underXDG_CACHE_HOME, which means it is shared between concurrently running lanes rather than private per dune run. The writes aretmp-then-rename, so nothing can read a half-written object, and nothing in this investigation pointed at it. Noted only because "several compiler lanes at once" is now a condition the suite is expected to survive.runtime/flan_dev.c:51reachesabort()on a full name table, andabortruns neither theatexitchain nor anything else. It is a 4096-name limit and no test comes near it, so it is left as it is.