diff --git a/TODO.org b/TODO.org index aa50db50..bc1164ef 100644 --- a/TODO.org +++ b/TODO.org @@ -1584,14 +1584,6 @@ was delivered" is a generation number rather than a name, so evaluating from inside a break into a thunk that stops on the same condition class is settled by comparing two integers. -** NEXT Whose break it is, which no counter answers -Decided 2026-09-25: fix it. A stop records whether the thread that stopped was running the evaluation's thunk or the program's own code, so the sentence is decided by the frame and not by the generation counter. -A game loop that signals during the build or the wait bumps the generation exactly -as a thunk would. The machine-readable fields stay right; what is wrong is the -sentence. The per-frame program-or-eval label is computed by the daemon from -ownership, not from anything in the frame, so this is not the shadow-stack gap it -was once written down as. Not queued — the window is narrow. - ** DONE The first evaluation no longer stalls behind the agent socket The accept loop used to sit behind a ten-second wait for the agent socket, so a program that binds its socket late — or not at all — looked ready and answered diff --git a/lib/dev.ml b/lib/dev.ml index ccbe8084..5d9f3a17 100644 --- a/lib/dev.ml +++ b/lib/dev.ml @@ -268,10 +268,28 @@ let deliver_at_stop t ~gen path = [None] where the program cannot be reached or answers something else, and the caller treats that the way it treats a missing refusal count: as no evidence, not as zero. Zero is a fact — it means running. *) -let stop_gen t : int option = +let stop_reply t = match request t "stop" with | exception Unix.Unix_error _ -> None - | text -> int_of_string_opt (String.trim text) + | text -> + (match String.split_on_char ' ' (String.trim text) with + | g :: rest -> + Option.map (fun g -> (g, rest)) (int_of_string_opt g) + | [] -> None) + +let stop_gen t : int option = Option.map fst (stop_reply t) + +(* The same stop with whose code it stopped in: [Some true] when the thread + was running an evaluated thunk, [Some false] when it was in the program's + own code, [None] from an agent that does not say. *) +let stop_owner t : (int * bool option) option = + Option.map + (fun (g, rest) -> + (g, match rest with + | [ "eval" ] -> Some true + | [ "program" ] -> Some false + | _ -> None)) + (stop_reply t) (* How many stopped-only modules the program has thrown away for reaching the game thread while it was running, and the sentence the agent says about it. @@ -1225,17 +1243,9 @@ let eval_expr t ~code ~origin ~pause = the reason [stop_gen]'s note gives at its definition. The name is kept beside it as the fallback for an agent that cannot answer the verb. - As early as it usefully can be, and still not early enough to be - exact: [build_module] below takes a couple of hundred milliseconds, - and a game loop that signals *on its own* during them — or mid-wait, - while the thunk is still perfectly fine — bumps the generation too. - The reply then says "the expression stopped on X" about an expression - that had not run. Its machine-readable half stays right, so the editor - opens the break the program is actually in; only the sentence is - wrong, and no counter closes this one, because the program's break and - the thunk's are the same kind of event. Separating them wants the - per-frame origin the backtrace carries, which is LLVM-only. - TODO.org, "Whose break it is, which no counter answers" has it. *) + A game loop that signals *on its own* while this is in flight bumps + the generation too, so a fresh stop is not yet the thunk's. The stop + itself says whose it is — see [settled] below. *) let entered = state t in let entered_gen = stop_gen t in t.n <- t.n + 1; @@ -1323,12 +1333,17 @@ let eval_expr t ~code ~origin ~pause = re-stop between them could pair a stale name with a fresh generation; it cannot manufacture one, since the generation only climbs when a break really was entered. *) + (* A fresh stop in the program's own code is not an answer: the + thunk has not run, and the break loop that stop entered polls + the ring, so the thunk runs inside it and its value arrives on + a later tick. *) let settled now = match now with | Stopped c -> let fresh = - match stop_gen t, entered_gen with - | Some g, Some g0 -> g > g0 + match stop_owner t, entered_gen with + | Some (_, Some false), _ -> false + | Some (g, _), Some g0 -> g > g0 | _ -> (match entered with Stopped c0 -> c0 <> c | _ -> true) in diff --git a/test/programs/dev-own-break.flan b/test/programs/dev-own-break.flan new file mode 100644 index 00000000..a7de2a59 --- /dev/null +++ b/test/programs/dev-own-break.flan @@ -0,0 +1,22 @@ +;;;; A program that stops on its own while an evaluation is in flight. +;;;; +;;;; Setting [go] from the editor starts it: the loop sees it, sleeps without +;;;; polling for longer than a module takes to build, and then signals. An +;;;; expression evaluated just after [go] is therefore waiting in the ring +;;;; when the program's own break is entered, and runs inside that break's +;;;; loop. The stop is the program's and the value is the expression's. +(import agent "vendor:agent") + +(declare-c usleep [us i32] i32 "usleep") + +(defstruct Late []) + +(defonce go i64) + +(defn main [] i32 + (while (= go 0) + (agent/wait 5)) + (usleep 1500000) + (restart-case + (do (error (Late {})) 0) + (carry-on [] 0))) diff --git a/test/test_dev.ml b/test/test_dev.ml index d5446668..dcbff377 100644 --- a/test/test_dev.ml +++ b/test/test_dev.ml @@ -8712,6 +8712,71 @@ let () = hook_block ~llvm:false; hook_block ~llvm:true; + (* ── Whose break it is ─────────────────────────────────────────── *) + + (* The program stops on its own while an evaluation is in flight: [go] + makes it sleep for longer than a module takes to build and then + signal, without polling in between. The stop is fresh, as a thunk's + would be, and it is not the expression's; the expression runs inside + the program's break loop and its value is the answer. On the default + backend, because the stop's owner is the agent's and not the + backend's. *) + let osock = tmp "ownbreak.sock" and oout = tmp "ownbreak.out" in + (try Sys.remove osock with Sys_error _ -> ()); + let ofd = Unix.openfile oout [ Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC ] 0o600 in + let opid = + Unix.create_process flan + [| flan; "dev"; "programs/dev-own-break.flan"; "-s"; osock |] + Unix.stdin ofd Unix.stderr + in + Unix.close ofd; + if not (listening ~pid:opid osock) then begin + fail "the own-break daemon %s" !listen_why; + (try Unix.kill opid Sys.sigkill with Unix.Unix_error _ -> ()) + end + else begin + let c = connect osock in + let said r = + Option.value ~default:(status r) (Wire.string_field r "message") + in + let ev code = + request c + (Printf.sprintf + "(:op \"eval-expr\" :code %s :file \"programs/dev-own-break.flan\")" + (Wire.quote code)) + in + let r = ev "(do (set go 1) 0)" in + if status r <> "ok" then fail "own break: setting go: %s" (said r) + else begin + (* The sleep keeps the thunk running inside the program's break for + many of the daemon's ticks, so a wait that took any fresh stop for + the thunk's would answer before the value exists. *) + let r = ev "(do (usleep 300000) 42)" in + if status r <> "ok" + || Wire.string_field r "value" <> Some "42" then + fail "own break: an expression in flight when the program stopped \ + on its own answered %s %S (value %S)" + (status r) (said r) + (Option.value ~default:"" (Wire.string_field r "value")); + (match Wire.field r "condition" with + | Some { Form.v = Form.Str "Late"; _ } -> () + | _ -> fail "own break: the reply does not carry the program's stop") + end; + ignore (aborted c); + (try Unix.close c with Unix.Unix_error _ -> ()); + if not + (await ~ms:10000 (fun () -> + match Unix.waitpid [ Unix.WNOHANG ] opid with + | 0, _ -> false + | _ -> true)) + then begin + fail "own break: the daemon did not end on abort"; + (try Unix.kill opid Sys.sigkill with Unix.Unix_error _ -> ()); + (try ignore (Unix.waitpid [] opid) with Unix.Unix_error _ -> ()) + end + end; + List.iter (fun f -> try Sys.remove f with Sys_error _ -> ()) [ osock; oout ]; + List.iter (fun f -> try Sys.remove f with Sys_error _ -> ()) [ sock; out; bsock; bout ]; Test_support.report ~label:"dev" () diff --git a/vendor/agent/flan_agent.c b/vendor/agent/flan_agent.c index efe49f02..5cc5c027 100644 --- a/vendor/agent/flan_agent.c +++ b/vendor/agent/flan_agent.c @@ -437,6 +437,15 @@ static const uint8_t abandon_report[] = * saved and restored around the call like [eval_boundary]. */ static sigjmp_buf *eval_escape; +/* Whether the game thread is inside an evaluated thunk's call, at any depth, + * rather than in the program's own code. A break records it, and it is what + * says whose break that is: a game loop that signals on its own while an + * evaluation is in flight stops exactly as a thunk would, and the stop + * counter cannot tell the two apart. Not [eval_boundary], which a class + * migration clears inside a thunk, nor [frame_floor], which it sets outside + * one. Game thread only, saved and restored around the call. */ +static int in_thunk; + /* What the chains looked like when the evaluation was called, weak for the * reason the frame walk below is: the runtime is linked into every program * that links this, but not every build carries the dev and dyn halves. */ @@ -505,6 +514,7 @@ static int migrate_call(void *fn, uint64_t instance, uint64_t added, void flan_agent_run_reset(void) { eval_boundary = NULL; eval_escape = NULL; + in_thunk = 0; restart_floor = 0; frame_floor = -1; } @@ -567,6 +577,7 @@ static _Atomic int aborting; typedef struct { int32_t gen; /* never reused, never 0 */ + int32_t in_eval; /* stopped inside a thunk */ /* Whether *any* restart on this list can be taken, which is a property of * the break and not of the restarts. [reachable] answers a different * question — that one is per restart, and it is about the thunk boundary. @@ -773,6 +784,7 @@ static int snap_push(int resumable, void *cond) { snapshot *s = &snaps[d]; int32_t n = flan_restart_count(); s->gen = ++snap_gen; + s->in_eval = in_thunk; s->resumable = resumable; s->cond = cond; s->sitelen = 0; @@ -1310,6 +1322,7 @@ int32_t flan_agent_poll(void) { * signal handler, and the jump leaves the handler. */ sigjmp_buf escape; sigjmp_buf *oescape = eval_escape; + int othunk = in_thunk; void *mh = NULL, *mr = NULL, *mf = NULL; int32_t md = 0; int64_t mroots = 0; @@ -1318,6 +1331,7 @@ int32_t flan_agent_poll(void) { if (flan_dyn_root_mark) mroots = flan_dyn_root_mark(); uint64_t mctx[2] = { 0, 0 }; if (flan_context_save) flan_context_save(mctx); + in_thunk = 1; if (sigsetjmp(escape, 1) == 0) { eval_escape = &escape; j.call(); @@ -1328,6 +1342,7 @@ int32_t flan_agent_poll(void) { if (flan_context_load) flan_context_load(mctx); } eval_escape = oescape; + in_thunk = othunk; /* Popped whichever way the thunk left — returning with a value, or * unwinding past this frame because someone abandoned it. */ flan_restart_pop_c(eval_boundary); @@ -1903,10 +1918,15 @@ static void handle_line(char *line, sink *o) { * of those have readers in flight and a reply format is a thing two ends * agree on. Answered while running as well, for [status]'s reason: an * editor polls this without knowing the state already. */ + /* After the number, whose code stopped: "eval" when the thread was inside + * an evaluated thunk, "program" when it was in the program's own code. */ if (strcmp(line, "stop") == 0) { snapshot *s = (atomic_load(&depth) > 0) ? snap_top() : NULL; char hdr[32]; - int k = snprintf(hdr, sizeof hdr, "%d\n", s == NULL ? 0 : s->gen); + int k = s == NULL + ? snprintf(hdr, sizeof hdr, "0\n") + : snprintf(hdr, sizeof hdr, "%d %s\n", s->gen, + s->in_eval ? "eval" : "program"); if (k > 0) emit(o, hdr, (size_t)k); return; }