A socket file must not outlive the process that bound it
The merged daemon now unlinks its socket on the ways out it does not control as well as the one it does: an atexit for exit(3), which is what a runtime trap takes, and by hand in die_now and in main's fallback, which are _exit and skip the chain on purpose. The client says so too. A refusal on a path that exists is a leftover, not a daemon declining, and the raw 'Connection refused' has now misdirected two investigations. Separately, and it is separate: dev-repl.flan gets dev-robust.flan's 24000-tick budget. Twenty seconds of program under a two-minute test is a second flake waiting its turn, and it is not the one fixed above -- that one fails honestly, saying the program exited.
This commit is contained in:
parent
4074eb7b7f
commit
ff2bd1da12
@ -405,9 +405,21 @@ Set to nil to leave the program's state to whatever replies happen to say."
|
||||
(error
|
||||
(setq flan-dev--connection nil)
|
||||
(force-mode-line-update t)
|
||||
(error "flan dev: cannot reconnect to %s: %s"
|
||||
(abbreviate-file-name flan-dev--socket)
|
||||
(error-message-string err)))))))
|
||||
;; A refusal on a path that exists is a socket file outliving the
|
||||
;; process that bound it -- a daemon killed outright rather than one
|
||||
;; that ended. `flan dev' removes its own socket on every way out it
|
||||
;; controls, so reaching here means it was killed, and the raw
|
||||
;; "Connection refused" is the least useful thing that could be said
|
||||
;; about that: it reads as a daemon that is there and declining. Two
|
||||
;; investigations in this repository have started from that reading
|
||||
;; and gone the wrong way.
|
||||
(if (string-match-p "[Cc]onnection refused" (error-message-string err))
|
||||
(error "flan dev: %s is a leftover socket -- whatever bound it is \
|
||||
gone; remove it and start `flan dev program.flan' again"
|
||||
(abbreviate-file-name flan-dev--socket))
|
||||
(error "flan dev: cannot reconnect to %s: %s"
|
||||
(abbreviate-file-name flan-dev--socket)
|
||||
(error-message-string err))))))))
|
||||
flan-dev--connection)
|
||||
|
||||
;;;###autoload
|
||||
|
||||
19
lib/dev.ml
19
lib/dev.ml
@ -2659,6 +2659,22 @@ static void flan_merged_exit(int32_t status) {
|
||||
for (;;) pause();
|
||||
}
|
||||
|
||||
/* The socket is this process's to remove, and on every way out of it and not
|
||||
* only the tidy one. A merged daemon that dies through a runtime trap -- a
|
||||
* bounds failure, an unhandled condition, an abort taken at the break loop --
|
||||
* leaves the path on disk with nothing behind it, and the next client's
|
||||
* connect is then ECONNREFUSED: a socket that plainly exists, refusing. That
|
||||
* message has already sent two investigations in this repository to the wrong
|
||||
* place. Gone is the honest state, and the client already has words for it.
|
||||
*
|
||||
* [atexit] covers exit(3), which is what flan_rt.c's rt_die uses. _exit and
|
||||
* abort skip it by design, so the places that take those routes unlink for
|
||||
* themselves; see die_now in flan_agent.c. */
|
||||
static void flan_merged_unlink_sock(void) {
|
||||
const char *s = getenv("FLAN_DEV_SOCK");
|
||||
if (s != NULL && *s != '\0') unlink(s);
|
||||
}
|
||||
|
||||
static char **g_argv;
|
||||
/* Atomic, not a plain int: this is the only happens-before edge between the
|
||||
* two threads, and everything the compiler set up before it — the listening
|
||||
@ -2698,6 +2714,7 @@ int main(int argc, char **argv) {
|
||||
pthread_t compiler;
|
||||
int rc;
|
||||
g_argv = argv;
|
||||
atexit(flan_merged_unlink_sock);
|
||||
flan_exit_hook = flan_merged_exit;
|
||||
if (pthread_create(&compiler, NULL, flan_merged_compiler, NULL) != 0) {
|
||||
fprintf(stderr, "flan dev: could not start the compiler thread\n");
|
||||
@ -2723,6 +2740,8 @@ int main(int argc, char **argv) {
|
||||
fprintf(stderr,
|
||||
"flan dev: the program returned %d; the session ends with it\n", rc);
|
||||
fflush(NULL);
|
||||
/* By hand, because _exit does not run the atexit chain registered above. */
|
||||
flan_merged_unlink_sock();
|
||||
_exit(rc);
|
||||
}
|
||||
|c}
|
||||
|
||||
@ -18,7 +18,16 @@
|
||||
|
||||
(defn main [] i32
|
||||
(agent/start "/tmp/flan-dev-repl-fallback.sock")
|
||||
(dotimes [i 4000]
|
||||
;; 24000 and not the 4000 the other fixtures use, because this one is the
|
||||
;; whole of test_emacs: one daemon carries every check from `flan-connect' to
|
||||
;; the break loop, and 4000 ticks of 5ms is a two-minute test with a
|
||||
;; twenty-second program in it. Run the suite on a machine with several
|
||||
;; compilers on it and the program can reach its last tick while the client
|
||||
;; is still working — which fails honestly ("the program exited; restart flan
|
||||
;; dev") but fails for a reason that has nothing to do with what was being
|
||||
;; checked. dev-robust.flan is 24000 for the same reason and got there first.
|
||||
;; The watchdogs, at 600s, are what actually bounds the run.
|
||||
(dotimes [i 24000]
|
||||
(agent/wait 5)
|
||||
(set ticks (step)))
|
||||
0)
|
||||
|
||||
@ -339,12 +339,51 @@ let () =
|
||||
ignore (request c "(:op \"close\")");
|
||||
Unix.close c
|
||||
end;
|
||||
(try Unix.kill pid Sys.sigterm with Unix.Unix_error _ -> ());
|
||||
(try ignore (Unix.waitpid [] pid) with Unix.Unix_error _ -> ());
|
||||
(* Asked before it is told: a daemon that has already died says so in its
|
||||
wait status, and that is the difference between "the test's last request
|
||||
was wrong" and "the session was not there to ask". The kill below then
|
||||
has nothing to do. See test_emacs.ml, where this was the fact two
|
||||
diagnoses of the same flake were missing. *)
|
||||
let died =
|
||||
match Unix.waitpid [ Unix.WNOHANG ] pid with
|
||||
| 0, _ ->
|
||||
(try Unix.kill pid Sys.sigterm with Unix.Unix_error _ -> ());
|
||||
(try ignore (Unix.waitpid [] pid) with Unix.Unix_error _ -> ());
|
||||
None
|
||||
| _, st -> Some st
|
||||
| exception Unix.Unix_error _ -> None
|
||||
in
|
||||
(* Read before the removal, because on a failure this is the evidence. *)
|
||||
let prog_out =
|
||||
if !failures = 0 then ""
|
||||
else
|
||||
match open_in_bin out with
|
||||
| ic ->
|
||||
let n = in_channel_length ic in
|
||||
let want = min n 4000 in
|
||||
seek_in ic (n - want);
|
||||
let s = really_input_string ic want in
|
||||
close_in ic;
|
||||
s
|
||||
| exception Sys_error _ -> "<no such file>"
|
||||
in
|
||||
List.iter (fun f -> try Sys.remove f with Sys_error _ -> ()) [ sock; out ];
|
||||
if !failures = 0 then print_endline "repl: all tests passed"
|
||||
else begin
|
||||
Printf.printf "\n%d failure(s)\n" !failures;
|
||||
(match died with
|
||||
| Some (Unix.WEXITED n) ->
|
||||
Printf.printf "the daemon had already exited with status %d\n" n
|
||||
| Some (Unix.WSIGNALED n) when n = Sys.sigpipe ->
|
||||
print_endline
|
||||
"the daemon had already been killed by SIGPIPE — a reply written \
|
||||
into a socket whose reader had gone"
|
||||
| Some (Unix.WSIGNALED n) ->
|
||||
Printf.printf "the daemon had already been killed by signal %d\n" n
|
||||
| Some (Unix.WSTOPPED n) ->
|
||||
Printf.printf "the daemon was stopped on signal %d\n" n
|
||||
| None -> print_endline "the daemon was still running at the end");
|
||||
Printf.printf "\n-- the program's own output, last 4k --\n%s\n" prog_out;
|
||||
exit 1
|
||||
end
|
||||
| _ -> print_endline "repl: skipped (no clang or llc on PATH)"
|
||||
|
||||
11
vendor/agent/flan_agent.c
vendored
11
vendor/agent/flan_agent.c
vendored
@ -463,6 +463,17 @@ int32_t flan_agent_poll(void);
|
||||
static _Noreturn void die_now(void) {
|
||||
fflush(stdout);
|
||||
fflush(stderr);
|
||||
/* The editor's socket, by hand. In a merged `flan dev' this process is the
|
||||
* listener as well as the program, and _exit skips the atexit handler that
|
||||
* would otherwise remove it -- deliberately, for the loader-lock reason
|
||||
* above. A socket file left behind with nothing accepting on it answers the
|
||||
* next client with ECONNREFUSED, which reads like a daemon that is there and
|
||||
* refusing rather than one that died. FLAN_DEV_SOCK is unset in an ordinary
|
||||
* run, and then this does nothing. */
|
||||
{
|
||||
const char *sock = getenv("FLAN_DEV_SOCK");
|
||||
if (sock != NULL && *sock != '\0') unlink(sock);
|
||||
}
|
||||
_exit(134);
|
||||
}
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user