diff --git a/NEXT.md b/NEXT.md index 0c97646..566098a 100644 --- a/NEXT.md +++ b/NEXT.md @@ -13,7 +13,14 @@ handler that returns normally has not answered it, so only a transfer gets past; with nothing transferring the program stops and names the condition. `flan_error` is where the break loop goes. -**The next task is the dev-build break loop, `spec-conditions.md` §2** — where +**The break loop is in.** An unhandled `error` now stops the program on the +frame that erred, lists the restarts on offer, and waits; a choice sent to the +agent's socket resumes it. See "The break loop" below. What is left of §2 is the +*editor* half — a `break` notification on the daemon protocol and a minibuffer +prompt — which is what makes it something you use rather than something you +drive with a socket. + +**The old next task, now done, was the dev-build break loop, `spec-conditions.md` §2** — where an unhandled `error` stops and talks to the daemon instead of `rt_die()`, and where **"a crash kills the program"** finally gets fixed. The transfer it needs exists now: §6's channel is in every signature and `restart-case` catches on @@ -1089,6 +1096,58 @@ Which gives the two refusals, both by the house rule rather than by accident: on the way out and an early exit would leave them on the stack pointing into a function that has gone. Same shape as `defer` inside a block. +### The break loop — conditions step 3 + +Where **"a crash kills the program"** stops being true. An unhandled `error` +runs a hook instead of `rt_die()`, on the frame that erred with nothing +unwound, so the condition and every restart between there and the top are still +live. + +``` +flan: unhandled Missing — stopped, not dead. + restart: retry + restart: use-placeholder +``` + +Four decisions, each of which is the reason something is where it is: + +- **It is a hook, not a call.** The loop lives in `vendor/agent/`, which is an + optional package; `flan_rt.c` is the release runtime and must not depend on + something a program may never import. A program with no agent leaves the hook + null and dies exactly as it did before. +- **The hook resumes by writing a restart into the transfer channel** — the + same channel an `invoke-restart` writes, reaching the same guard. Choosing a + restart from the break loop and choosing one from a handler are therefore the + same act, lowered once. Nothing about §6 needed changing to support it. +- **The break loop *is* the poll loop**, run from the error instead of from the + frame boundary. That is not a convenience: an expression evaluated while + stopped is a module the listener queues and the game thread runs, so a loop + that did not drain that queue would hang `C-x C-e` exactly when it is most + wanted. +- **Installing while stopped is allowed**, which contradicts a rule stated + above and should. "A redefined function must not be swapped while it is on + the stack" is about *mid-frame consistency* — half a frame of old code and + half of new — and there is no frame in progress here. The old body on the + stack keeps running; a `retry` restart calls through the cell and reaches the + new one. That is the fix-it-and-retry loop, and refusing the install would + remove the point of stopping. + +**A restart frame carries its name now**, beside the hash. Matching never needs +it — that is what the hash is for — but a break loop has to *show* someone +their choices and nothing at run time can turn a hash back into a name. It is +also `compute-restarts`' data, whenever that arrives. + +**A choice is validated on the listener thread**, against a stack the stopped +game thread is holding still, and refused there. Accepting it and discovering +on the game thread that no frame offers it would answer `ok` for something that +cannot happen. + +The socket verbs are `restarts`, `restart ` and `abort`, and all three +are refused with the reason when the program is not stopped — there is no +restart stack to walk from a running one. `test/programs/break.flan` errors +twice and the test takes a *different* restart each time, so a loop that always +resumed the same way could not pass. + ### Conditions — step 2: `restart-case` and `invoke-restart` `spec-conditions.md` §3 to §6: the transfer. A handler runs where the signal diff --git a/lib/emit.ml b/lib/emit.ml index 80b7c1f..c80950e 100644 --- a/lib/emit.ml +++ b/lib/emit.ml @@ -633,6 +633,18 @@ and emit_restart_case f ty clauses body = ins f "%s = getelementptr inbounds %%restart, ptr %s, i32 0, i32 1" nid slot; ins f "store i32 %d, ptr %s" c.Tast.rname_id nid; + (* The name itself, beside the hash. A hash is all that matching + needs, but a break loop has to *show* someone their choices, and + nothing at run time can turn a hash back into a name. *) + let sid, slen = string_bytes f.md c.Tast.rname in + let np = fresh f in + ins f "%s = getelementptr inbounds %%restart, ptr %s, i32 0, i32 2" + np slot; + ins f "store ptr %s, ptr %s" sid np; + let nl = fresh f in + ins f "%s = getelementptr inbounds %%restart, ptr %s, i32 0, i32 3" + nl slot; + ins f "store i64 %d, ptr %s" slen nl; ins f "call void @flan_restart_push(ptr %s)" slot; slot) clauses @@ -1139,7 +1151,7 @@ let header = {|; Generated by flan. The layout is C's: no object headers anywher ; target field, because the frame's own address *is* the target — which makes ; a transfer's aim exact, and makes re-entering a restart-case work with ; nothing extra, since each activation allocates its own. -%restart = type { ptr, i32 } +%restart = type { ptr, i32, ptr, i64 } declare void @flan_rt_init(i32, ptr) declare void @flan_argv(ptr) diff --git a/runtime/flan_rt.c b/runtime/flan_rt.c index 01e8397..cf932af 100644 --- a/runtime/flan_rt.c +++ b/runtime/flan_rt.c @@ -78,6 +78,11 @@ void flan_signal(uint32_t type_id, void *condition, void *xfer) { typedef struct flan_restart { struct flan_restart *prev; uint32_t name_id; + /* The name as written, beside the hash that matching uses. Matching never + * needs it; a break loop does, because it has to show someone their choices + * and nothing at run time can turn a hash back into a name. */ + const uint8_t *name; + int64_t namelen; } flan_restart; static flan_restart *restarts; @@ -89,6 +94,22 @@ void flan_restart_push(flan_restart *r) { void flan_restart_pop(flan_restart *r) { restarts = r->prev; } +/* What is on offer, innermost first — spec-conditions.md §4's walk, without + * committing to anything. This is [compute-restarts]' data; today its only + * caller is the break loop. */ +int32_t flan_restart_count(void) { + int32_t n = 0; + for (flan_restart *r = restarts; r != NULL; r = r->prev) n++; + return n; +} + +const uint8_t *flan_restart_name(int32_t i, int64_t *len) { + for (flan_restart *r = restarts; r != NULL; r = r->prev) + if (i-- == 0) { *len = r->namelen; return r->name; } + *len = 0; + return NULL; +} + void *flan_find_restart(uint32_t name_id) { for (flan_restart *r = restarts; r != NULL; r = r->prev) if (r->name_id == name_id) return r; @@ -204,10 +225,51 @@ _Noreturn void flan_bounds_fail(const uint8_t *loc, int64_t loclen, * * [flan_signal] is not reused with a flag because the two differ in what they * do when the walk ends, which is the whole of §1 against §2. */ +/* The dev-build break loop, spec-conditions.md §2. A hook rather than a direct + * call because the loop lives in the *agent*, which is an optional package, and + * this file is the release runtime — it must not depend on something a program + * may not have imported. A program with no agent leaves this NULL and dies the + * way it always did. + * + * The hook may resume by writing a restart into the transfer channel, which is + * the same channel an invoke-restart writes and reaches the same guard. So + * choosing a restart from the break loop and choosing one from a handler are + * the same act, lowered the same way. */ +void (*flan_break_hook)(const uint8_t *name, int64_t namelen, void *condition, + void *xfer); + +/* Must agree with Check.type_id, byte for byte, or a name typed at the break + * loop matches nothing. FNV-1a over the name, 32 bits. */ +static uint32_t flan_name_id(const uint8_t *s, int64_t n) { + uint32_t h = 0x811c9dc5u; + for (int64_t i = 0; i < n; i++) { + h ^= (uint32_t)s[i]; + h *= 0x01000193u; + } + return h; +} + +/* What the break loop calls to resume: look a restart up by the name someone + * typed and aim the channel at it. 0 if no frame offers it, and then the loop + * says so rather than resuming into nothing. */ +int32_t flan_break_resume(const uint8_t *name, int64_t namelen, void *xfer) { + void *r = flan_find_restart(flan_name_id(name, namelen)); + if (r == NULL) return 0; + *(void **)xfer = r; + return 1; +} + void flan_error(uint32_t type_id, void *condition, void *xfer, const uint8_t *name, int64_t namelen) { flan_signal(type_id, condition, xfer); if (*(void **)xfer != NULL) return; + /* Nothing handled it. In a dev build that is a place to stand, not the end + * of the program — which is the whole of §2 and the reason it is worth + * having. */ + if (flan_break_hook != NULL) { + flan_break_hook(name, namelen, condition, xfer); + if (*(void **)xfer != NULL) return; + } fflush(stdout); fprintf(stderr, "unhandled %.*s\n", (int)namelen, (const char *)name); rt_die(); diff --git a/test/programs/break.flan b/test/programs/break.flan new file mode 100644 index 0000000..d56ca52 --- /dev/null +++ b/test/programs/break.flan @@ -0,0 +1,21 @@ +;;;; The dev-build break loop — spec-conditions.md §2. +;;;; +;;;; An unhandled error used to kill the program. Here it stops instead, on the +;;;; frame that erred with nothing unwound, and waits for someone to pick a +;;;; restart. Two of them, and the run takes a different one each time, so a +;;;; break loop that always resumed the same way could not pass. +(import agent "vendor:agent") + +(defstruct Missing [id i32]) + +(defn fetch [n i32] i32 + (restart-case + (do (error (Missing {:id n})) 0) + (use-placeholder [] -1) + (retry [] 7))) + +(defn main [] i32 + (agent/start "/tmp/flan-break.sock") + (print-i64 (i64 (fetch 1))) (newline) + (print-i64 (i64 (fetch 2))) (newline) + 0) diff --git a/test/test_agent.ml b/test/test_agent.ml index b751d06..3072a99 100644 --- a/test/test_agent.ml +++ b/test/test_agent.ml @@ -163,8 +163,97 @@ let () = "1\n1000\n1007\n" end; + (* ── The break loop, spec-conditions.md §2 ──────────────────────── *) + + (* The claim is that an unhandled [error] stops rather than dying, and can + be resumed into a restart chosen from outside. A program that died would + exit 134 with no output; one that stopped and was never resumed would + hang and be killed by the timeout. Only a resume produces both numbers, + and they differ, so a loop that always took the same restart fails. *) + let bsock = tmp "break.sock" and bout = tmp "break.out" in + (try Sys.remove bsock with Sys_error _ -> ()); + let bt, bl = Session.create ~file:"programs/break.flan" in + let bexe = tmp "break" in + ignore + (Build.executable ~opts:dev ~csrcs:bl.Load.csrcs ~lflags:bl.Load.lflags + bt.Session.host ~out:bexe); + let bfd = Unix.openfile bout [ Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC ] 0o600 in + let env = + Array.append (Unix.environment ()) [| "FLAN_AGENT_SOCKET=" ^ bsock |] + in + let bpid = + Unix.create_process_env bexe [| bexe |] env Unix.stdin bfd bfd + in + Unix.close bfd; + if not (await (fun () -> Sys.file_exists bsock)) then + fail "the broken program never listened" + else begin + (* It has to be *stopped* before the restarts are knowable: the listener + refuses to walk a stack the game thread is still running on. *) + let listed = ref "" in + if not + (await (fun () -> + listed := send bsock "restarts"; + !listed <> "" && not (String.length !listed >= 3 + && String.sub !listed 0 3 = "err"))) + then fail "the program never reached the break loop: %S" !listed + else begin + (* Innermost first, and both on offer. *) + if !listed <> "retry\nuse-placeholder\n.\n" then + fail "restarts on offer\n got: %S\n wanted: %S" !listed + "retry\nuse-placeholder\n.\n"; + (* A name nothing offers is refused *here*, before the reply. Answering + ok and discovering it on the game thread would report success for + something that cannot happen. *) + let bad = send bsock "restart nonesuch" in + if not (String.length bad >= 3 && String.sub bad 0 3 = "err") then + fail "a restart nobody offers was accepted: %S" bad; + ignore (send bsock "restart retry"); + (* Wait for the *result* of that choice before making the next one. + Asking whether it is stopped is not enough: it is still stopped in + the first break until the resume lands, and a second choice sent + then would be taken by the first one — which passes the listing + check and then hangs, because the second break never gets an + answer. The printed 7 is the only proof the first resume happened. *) + let printed () = + let t = In_channel.with_open_bin bout In_channel.input_all in + List.exists (String.equal "7") (String.split_on_char '\n' t) + in + if not (await printed) then + fail "the first restart never produced its value" + else if not (await (fun () -> send bsock "restarts" + = "retry\nuse-placeholder\n.\n")) + then fail "the program never stopped a second time" + else ignore (send bsock "restart use-placeholder") + end + end; + let bstatus = ref (Unix.WEXITED 0) in + let reaped = + await ~ms:5000 (fun () -> + match Unix.waitpid [ Unix.WNOHANG ] bpid with + | 0, _ -> false + | _, s -> bstatus := s; true) + in + if not reaped then begin + (try Unix.kill bpid Sys.sigkill with Unix.Unix_error _ -> ()); + fail "the program never resumed out of the break loop" + end + else begin + let text = In_channel.with_open_bin bout In_channel.input_all in + let want = "7\n-1\n" in + let got = + String.concat "\n" + (List.filter + (fun l -> l <> "" && not (String.length l >= 5 && String.sub l 0 5 = "flan:") + && not (String.length l >= 2 && String.sub l 0 2 = " ")) + (String.split_on_char '\n' text)) + in + if !bstatus <> Unix.WEXITED 0 || got ^ "\n" <> want then + fail "break loop transcript\n got: %S\n wanted: %S" got want + end; + List.iter (fun f -> try Sys.remove f with Sys_error _ -> ()) - [ exe; so1; so2; sock; out ]; + [ exe; so1; so2; sock; out; bsock; bout; bexe ]; if !failures = 0 then print_endline "agent: all tests passed" else begin Printf.printf "\n%d failure(s)\n" !failures; diff --git a/vendor/agent/flan_agent.c b/vendor/agent/flan_agent.c index d1f15ff..d5351e4 100644 --- a/vendor/agent/flan_agent.c +++ b/vendor/agent/flan_agent.c @@ -77,6 +77,86 @@ static void publish(job j) { } /* Returns how many modules were installed. Call it between frames. */ +/* ── The break loop, spec-conditions.md §2 ──────────────────────────── */ +/* Where "a crash kills the program" stops being true. An unhandled error runs + * this instead of rt_die(), on the frame that erred, with nothing unwound — so + * the condition and every restart between here and the top are still live. + * + * It is the *poll* loop, run from here instead of from the frame boundary, and + * that is the whole design. An expression evaluated while stopped is a module + * the loader thread queues and the game thread runs; if this loop did not + * drain that queue, C-x C-e would hang exactly when it is most wanted. + * + * Installing while stopped is deliberately allowed. The rule that a redefined + * function must not be swapped while it is on the stack is about *mid-frame + * consistency* — half a frame of old code and half of new — and there is no + * frame in progress here. The old body on the stack keeps running; a retry + * restart calls through the cell and reaches the new one. That is the fix-it- + * and-retry loop, and refusing the install would remove the point. */ + +/* The runtime's end of it. flan_rt.c holds the hook and the lookup; the loop + * itself is here, because it needs the socket and the install queue and the + * runtime must not depend on an optional package for either. */ +extern void (*flan_break_hook)(const uint8_t *name, int64_t namelen, + void *condition, void *xfer); +extern int32_t flan_break_resume(const uint8_t *name, int64_t namelen, + void *xfer); +extern int32_t flan_restart_count(void); +extern const uint8_t *flan_restart_name(int32_t i, int64_t *len); + +/* What the listener thread hands the stopped game thread. One slot, because + * only one thread is ever stopped. */ +static _Atomic int broken; /* the game thread is in the loop */ +static char chosen[128]; +static _Atomic int chosen_ready; +static _Atomic int aborting; + +int32_t flan_agent_poll(void); + +static void break_loop(const uint8_t *name, int64_t namelen, void *condition, + void *xfer) { + struct timespec step = { 0, 2000000 }; /* 2ms */ + (void)condition; + fflush(stdout); + fprintf(stderr, "\nflan: unhandled %.*s — stopped, not dead.\n", + (int)namelen, (const char *)name); + { + int32_t n = flan_restart_count(); + if (n == 0) + fprintf(stderr, " no restarts are active; abort, or fix and reload\n"); + for (int32_t i = 0; i < n; i++) { + int64_t len = 0; + const uint8_t *nm = flan_restart_name(i, &len); + if (nm != NULL) + fprintf(stderr, " restart: %.*s\n", (int)len, (const char *)nm); + } + } + fflush(stderr); + atomic_store(&broken, 1); + for (;;) { + flan_agent_poll(); + if (atomic_load(&aborting)) { + fflush(stdout); + fprintf(stderr, "flan: aborted at the break loop\n"); + exit(134); + } + if (atomic_load(&chosen_ready)) { + int32_t ok = + flan_break_resume((const uint8_t *)chosen, (int64_t)strlen(chosen), xfer); + atomic_store(&chosen_ready, 0); + if (ok) { + fprintf(stderr, "flan: resuming at restart %s\n", chosen); + fflush(stderr); + atomic_store(&broken, 0); + return; + } + fprintf(stderr, "flan: no restart named %s is active\n", chosen); + fflush(stderr); + } + nanosleep(&step, NULL); + } +} + int32_t flan_agent_poll(void) { unsigned t = atomic_load_explicit(&tail, memory_order_relaxed); unsigned h = atomic_load_explicit(&head, memory_order_acquire); @@ -142,6 +222,56 @@ static void serve(int fd) { * expression evaluated, with the counter that says whether it is a new * one. The daemon polls this rather than the agent holding a connection * open across a frame boundary it does not control. */ + /* Only while stopped: what is on offer, and which one to take. Both are + * refused when the program is running, by name, rather than silently + * doing nothing — there is no restart stack to walk from here. */ + if (strcmp(line, "restarts") == 0) { + if (!atomic_load(&broken)) { reply(fd, "err not stopped\n"); return; } + int32_t n = flan_restart_count(); + for (int32_t i = 0; i < n; i++) { + int64_t len = 0; + const uint8_t *nm = flan_restart_name(i, &len); + if (nm != NULL) { + send(fd, nm, (size_t)len, MSG_NOSIGNAL); + reply(fd, "\n"); + } + } + reply(fd, ".\n"); + return; + } + if (strncmp(line, "restart ", 8) == 0) { + if (!atomic_load(&broken)) { reply(fd, "err not stopped\n"); return; } + size_t k = strlen(line + 8); + if (k == 0 || k >= sizeof chosen) { reply(fd, "err bad restart name\n"); return; } + /* Checked here, against the stack the stopped thread is holding still, + * rather than accepted and found wrong after the reply has gone. "ok" + * has to mean the program will resume. */ + { + int32_t n = flan_restart_count(), found = 0; + for (int32_t i = 0; i < n && !found; i++) { + int64_t len = 0; + const uint8_t *nm = flan_restart_name(i, &len); + found = nm != NULL && (size_t)len == k && memcmp(nm, line + 8, k) == 0; + } + if (!found) { + reply(fd, "err no restart named "); + reply(fd, line + 8); + reply(fd, " is active\n"); + return; + } + } + memcpy(chosen, line + 8, k + 1); + /* Published last, so the game thread never reads a half-written name. */ + atomic_store(&chosen_ready, 1); + reply(fd, "ok\n"); + return; + } + if (strcmp(line, "abort") == 0) { + if (!atomic_load(&broken)) { reply(fd, "err not stopped\n"); return; } + reply(fd, "ok\n"); + atomic_store(&aborting, 1); + return; + } if (strcmp(line, "result") == 0) { uint64_t gen = 0, len = 0; const char *v = flan_dev_result_get(&gen, &len); @@ -215,5 +345,9 @@ int32_t flan_agent_start(const uint8_t *path, int64_t len) { if (bind(listen_fd, (struct sockaddr *)&addr, sizeof addr) < 0) return -1; if (listen(listen_fd, 4) < 0) return -1; if (pthread_create(&listener, NULL, accept_loop, NULL) != 0) return -1; + /* From here an unhandled error stops rather than dying. Installed with the + * socket and not before it: without a listener there is nobody to ask what + * to do, and stopping forever is worse than the abort it replaces. */ + flan_break_hook = break_loop; return 0; }