JosephFerano/flan
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

A render job carries the stop it was built under, and the agent will not run it without one

Browse Source
This commit is contained in:
Joseph Ferano 2026-09-18 12:45:46 +07:00
parent 9d73900f3e
commit 8b48744864
4 changed files with 294 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
docs/BUGS-2026-09-18.md
View File

@ -104,9 +104,24 @@ territory; fix or record, the lane's call.
narrowed buffers.
- **`flan-connect` + `flan-dev-quit` kill two sessions** (`flan-dev.el:721`): quit sends
`close` down the current connection and kills the daemon it started for another program.
- **`reg at` TOCTOU** (`dev.ml:1488`): the stopped-only gate is checked three round trips
before the render thunk runs; a `restart` in between lets the thunk chase freed memory
with the gate's blessing.
- ~~**`reg at` TOCTOU**~~ — FIXED. The gate was checked three round trips before the
render thunk ran, and nothing held the break across the ~300ms build, so a `restart`
in between let the thunk chase freed memory with the gate's blessing. The sound fix is
agent-side: the render job now carries the condition it was built under. `inspect` by
address delivers its module as `stopped-only <path>`, the job header keeps the flag,
and `flan_agent_poll` drops such a job — counted, handle closed, nothing installed and
nothing called — when `depth` is 0 at the moment it is claimed. That read is sound
rather than narrower: only the game thread polls and only the game thread raises
`depth`, so `depth > 0` seen inside a poll means *this* thread is parked in the break
loop and cannot be running a frame. The daemon reads the `refusals` count either side
of the delivery and surfaces the agent's own sentence — "the program resumed while this
inspection was being built — stop it again and re-ask" — instead of the timeout's
wrong-cause "is it calling (agent/poll)?". `reg at`'s depth gate stays as the front
door. `locals`, `globals` and `inspect` by slot are deliberately *not* stopped-only:
they hold no address, re-deriving the frame through `snap_top` or binding a global by
name at thunk-run time, so they carry no blessing that can expire. Pinned in
`test_agent.ml`, where a stopped-only job delivered to a program that is definitively
running is dropped while the eval module beside it installs — one install, not two.
- ~~**defenum values never range-checked to i32**~~ — FIXED. Every resolved member value,
explicit or autoincremented, is range-checked against i32 in the parser before the collision scan,
so the scan compares the numbers the program will actually have. Out of range is refused

123
lib/dev.ml
View File

@ -158,6 +158,39 @@ let request t line =
in the new. *)
let deliver t path = String.trim (request t path)
(* The same, for a module that may only run from a break.
The word goes in front of the path rather than into the module, which is
what keeps this change out of both emitters and out of the [.so]'s ABI. The
agent's own note says why it belongs on the request: what is stopped-only is
the *question*, not the code. *)
let deliver_stopped_only t path = deliver t ("stopped-only " ^ path)
(* 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.
Both come from the agent because the sentence lives there, once. Reading it
here and keeping a second copy of the words would be the same refusal in two
places, drifting apart the first time either is reworded — and this one is
the sentence somebody reads in the minibuffer when their inspection comes
back empty, so the wording is the whole of its value.
[None] where the program cannot be reached or answers something else: a
count that could not be read is not a count that did not move, and the
caller treats it as "no evidence" rather than as zero. *)
let refusals t : (int * string) option =
match request t "refusals" with
| exception Unix.Unix_error _ -> None
| text ->
(match String.index_opt text '\n' with
| None -> None
| Some i ->
(match int_of_string_opt (String.trim (String.sub text 0 i)) with
| None -> None
| Some n ->
Some (n, String.trim (String.sub text (i + 1)
(String.length text - i - 1)))))
(* Read back the value of the last expression evaluated, with the counter that
says whether it is a new one. The thunk runs on the game thread whenever the
program next reaches a frame boundary, which is not a moment the compiler
@ -1135,15 +1168,58 @@ let backtrace_op t =
nobody sees until the one path that lost it is the one being used.
[tag] only names the [.so] on disk, which is what someone reads when they
go looking at [t.dir] to find out which verb produced what. *)
let run_render_thunk t ~tag ~(c : Session.change) : (string, string) result =
go looking at [t.dir] to find out which verb produced what.
── [stopped_only], and why only one of the three wants it ────────────
Everything between the gate and the thunk takes time the gate does not
cover. The verb checks that the program is stopped, the agent checks it
again, and then a module is *built* — a third of a second of llc and a
linker — and delivered, and waited on for up to five seconds. A [restart]
arriving anywhere in there resumes the game thread, and the thunk runs at
the next frame boundary instead of from the break. The wait below does not
even notice: it keeps waiting while liveness is [Live], and a resumed
program is the liveliest thing there is.
What that costs depends entirely on what the thunk holds.
[inspect] by *address* holds a number. [Dev.render_addr] bakes the address
into the module as an integer literal, because the registry's blessing —
"something live is there, and it is a [Foo]" — was given at build time by a
table that the running program is exactly the thing that changes. Run that
thunk after a resume and it dereferences an address whose blessing expired,
possibly into storage the program has since freed. So it is delivered
stopped-only, and the agent drops it rather than running it.
[locals], [inspect] by *slot* and [globals] hold no address at all, and that
is the line. A local is reached through [flan/dev-slot], which is
[flan_agent_frame_slot], which asks [snap_top] for the frame *when the thunk
runs* — and [snap_top] is empty once the break that pushed it has been
resumed past. A global is reached by name: [Emit.redefinition] leaves it
[external], the dynamic linker binds it to the program's own storage, and
that storage has existed since the process started. Neither carries a
permission that can go stale between the asking and the running, because
neither was given one. Tagging them stopped-only would refuse work that is
sound, which is the other way to lose an answer. *)
let run_render_thunk ?(stopped_only = false) t ~tag ~(c : Session.change)
: (string, string) result =
let before = match result t with Some (g, _) -> g | None -> 0L in
(* Read *before* the build, not before the wait: the resume this is watching
for can land while llc is still running, and the job it kills is this one.
[None] when the program cannot say, in which case nothing below compares
against it — a missing count is no evidence either way. *)
let refused_before = if stopped_only then refusals t else None in
let resumed () =
match (refused_before, if stopped_only then refusals t else None) with
| Some (before, _), Some (now, why) when now > before -> Some why
| _ -> None
in
t.n <- t.n + 1;
let out = Filename.concat t.dir (Printf.sprintf "%s%d.so" tag t.n) in
match build_module c ~debug:t.session.Session.debug ~out with
| exception Failure m -> Error m
| _ ->
(match deliver t out with
(match (if stopped_only then deliver_stopped_only t out else deliver t out) with
| exception Unix.Unix_error (e, _, _) ->
Error ("cannot reach the program: " ^ Unix.error_message e)
| "ok" ->
@ -1158,21 +1234,31 @@ let run_render_thunk t ~tag ~(c : Session.change) : (string, string) result =
than anywhere. *)
drain t;
match result t with
| Some (g, v) when Int64.compare g before > 0 -> Some v
| _ when ms <= 0 -> None
| Some (g, v) when Int64.compare g before > 0 -> Ok v
(* Asked every tick and not once at the end, because the five seconds
are the point: a dropped job produces no value, so without this the
answer would be the timeout's sentence — "is it calling
(agent/poll)?" — which names the wrong cause and sends the reader
to look at a loop that was polling perfectly well. *)
| _ ->
ignore (Unix.select [] [] [] 0.005);
(* [Live] for the reason [eval_expr]'s own wait gives: a thunk
needs a frame boundary, and neither a gone program nor a parked
one is going to reach one. *)
if liveness t = Live then wait (ms - 5) else None
match resumed () with
| Some why -> Error why
| None ->
let gave_up =
Error
"the program did not reach a frame boundary; is it calling \
(agent/poll)?"
in
if ms <= 0 then gave_up
else begin
ignore (Unix.select [] [] [] 0.005);
(* [Live] for the reason [eval_expr]'s own wait gives: a thunk
needs a frame boundary, and neither a gone program nor a
parked one is going to reach one. *)
if liveness t = Live then wait (ms - 5) else gave_up
end
in
(match wait 5000 with
| Some v -> Ok v
| None ->
Error
"the program did not reach a frame boundary; is it calling \
(agent/poll)?")
wait 5000
| reply -> Error ("the program refused the module: " ^ reply))
(* The frame checks, which every verb that reads a *frame* has to make and
@ -1687,7 +1773,10 @@ let inspect_addr t ~addr ~want_type =
| Error m -> error m
| exception Failure m -> error m
| Ok c ->
(match run_render_thunk t ~tag:"a" ~c with
(* Stopped-only, and the only one of the three that is. The
gate above was read three round trips and a build ago;
this is what holds it. See [run_render_thunk]. *)
(match run_render_thunk ~stopped_only:true t ~tag:"a" ~c with
| Error m -> error m
| Ok v ->
ok

54
test/test_agent.ml
View File

@ -383,6 +383,32 @@ let () =
let ec = Session.eval_expr qt ("\"" ^ long ^ "\"") in
let eso = tmp "queue-eval.so" in
ignore (Build.shared ~opts:dev ~ir:ec.Session.ir ~out:eso ());
(* ── A stopped-only job that reaches a running program ────────
The [reg at] TOCTOU, pinned at the one place it can be pinned
deterministically. In the daemon it is a race: [inspect] by
address checks that the program is stopped, spends a third of a
second building a thunk with that address baked in, and a
[restart] landing in the window resumes the game thread before
the thunk runs. Reproducing *that* means winning a race against
llc. What the fix actually turns on does not need the race at
all — a job tagged stopped-only, arriving at a frame boundary
with no break in force, must be dropped — and this program is
never stopped, so "no break in force" is not something to arrange.
It is also the A/B, in one program at one moment, which is why it
sits beside the eval module rather than in a block of its own.
Two jobs are queued back to back: the same [tick] module the ring
above installed sixty-four times, now sent stopped-only, and the
eval module sent plainly. The program polls once and prints how
many it installed. One. Without the flag it is two, and the two
deliveries differ in nothing but the word in front of the path. *)
let refusals_before = send qsock "refusals" in
let s = send qsock ("stopped-only " ^ qso) in
if s <> "ok\n" then
fail "a stopped-only module was not queued: %S" s;
let r = send qsock eso in
if r <> "ok\n" then fail "the eval module was not queued: %S" r;
ignore (Unix.write wfd (Bytes.of_string "\n") 0 1);
@ -412,6 +438,34 @@ let () =
fail "the result is not the value that was rendered: %S"
(head 8)
end;
(* And the program says so, because a job that vanishes quietly is
the same lie the ring's dropped-oldest was. The count is what the
daemon compares either side of a delivery; the sentence is what it
puts in front of the person, and it lives here so that there is
one copy of it. *)
let refusals_after = send qsock "refusals" in
let count r =
match String.index_opt r '\n' with
| None -> None
| Some i -> int_of_string_opt (String.sub r 0 i)
in
(match (count refusals_before, count refusals_after) with
| Some b, Some a when a = b + 1 -> ()
| _ ->
fail "a dropped stopped-only job was not counted: %S then %S"
refusals_before refusals_after);
let wanted =
"the program resumed while this inspection was being built — \
stop it again and re-ask"
in
(match String.index_opt refusals_after '\n' with
| Some i
when String.trim
(String.sub refusals_after (i + 1)
(String.length refusals_after - i - 1))
= wanted -> ()
| _ ->
fail "the refusal does not say why: %S" refusals_after);
(try Sys.remove eso with Sys_error _ -> ())
end;
Unix.close wfd;

118
vendor/agent/flan_agent.c vendored
View File

@ -141,7 +141,66 @@ int flan_dev_reg_overflowed(void);
* ran a thunk and left nothing behind. Everything else is kept mapped forever:
* a cell holds an address inside a module's text, and unloading it would leave
* every call site pointing at unmapped memory. */
typedef struct { install_fn install; call_fn call; void *handle; } job;
/* [stopped_only] is the second half of a promise the *daemon* made and cannot
* keep on its own. A module built to render what is at a raw address has that
* address baked into it as an integer literal: the registry blessed the
* address as live at one moment, and the thunk dereferences it at another,
* three round trips and a ~300ms build later. Nothing in between held the
* break. A [restart] arriving mid-build resumes the game thread, and this ring
* would then install and run the thunk at the very next frame boundary —
* mid-frame, against an address the program may have freed since.
*
* So the job carries the condition it was built under, and [flan_agent_poll]
* re-asks it at the moment of truth instead of trusting the answer the sender
* got. The gate lives *here* and nowhere else on purpose: a second check on
* the listener thread, at delivery, would only shrink the window — [depth] can
* flip between that check and this one — and two places emitting the same
* refusal is how two places stop agreeing, which is the note the struct
* comment further down already makes about layouts.
*
* What makes the check in [flan_agent_poll] sound rather than narrower: only
* the game thread polls (agent.flan's [poll-raw] at a frame boundary, and the
* break loop below, which is the same thread parked), and only the game thread
* raises [depth]. So [depth > 0] read from inside a poll means *this* thread
* is inside the break loop right now — it cannot be running a frame at the
* same time — and the stop the daemon asked under is still the stop in force.
* A false negative is possible in one direction only and is the harmless one:
* a break entered after the read refuses a job that would have been legal, and
* refusing is right anyway, because that is a different stop from the one the
* registry answered under.
*
* [handle] is set only for a module that declared itself transient — one that
* ran a thunk and left nothing behind. Everything else is kept mapped forever:
* a cell holds an address inside a module's text, and unloading it would leave
* every call site pointing at unmapped memory. */
typedef struct {
install_fn install;
call_fn call;
void *handle;
int stopped_only;
} job;
/* Said once, in one place, and shipped to the daemon over [refusals] rather
* than written down again at the other end. A refusal is a sentence naming
* what actually happened, and the thing that actually happened is not "the
* gate failed" — it is that the program the person was inspecting started
* running again while the inspector was being compiled. */
static const char *RESUMED =
"the program resumed while this inspection was being built — stop it again "
"and re-ask";
/* How many stopped-only jobs have been dropped, ever. A count and not a flag:
* the daemon reads it before it delivers and again while it waits, and what it
* wants to know is whether one happened *in between*, which a flag somebody
* else could have cleared cannot say.
*
* It is the whole of the channel because the sentence is fixed. One residual,
* left alone deliberately: two inspections in flight at once would let either
* one's refusal surface to the other's waiter. The result buffer they both
* write into has exactly that property already — flan_dev.c says so where the
* seqlock is — and fixing it here without fixing it there would be half a
* repair wearing the whole one's clothes. */
static atomic_ullong refused_while_running;
static job queue[QUEUE];
static atomic_uint head; /* written by the listener */
@ -601,6 +660,24 @@ int32_t flan_agent_poll(void) {
if (t == h) return n;
job j = queue[t % QUEUE];
atomic_store_explicit(&tail, t + 1, memory_order_relaxed);
/* The gate the [job] comment argues for, asked at the only moment whose
* answer is worth anything. Claimed first and then dropped, rather than
* left in the ring for a later stop: the address it was built around was
* blessed by a stop that has already ended, and a job that waits for the
* *next* break would run against a blessing older still.
*
* The handle is closed for the reason the listener closes one it refuses
* for having no installer: a transient module nothing ever points into is
* unreachable the moment this returns, and dlopen refcounts by path, so
* dropping the handle on the floor here would leave a count that re-asking
* the same inspection bumps and nothing brings down. [n] is not bumped —
* nothing was installed, and [n] is what a caller polls to find out that
* something was. */
if (j.stopped_only && atomic_load(&depth) <= 0) {
atomic_fetch_add(&refused_while_running, 1);
if (j.handle != NULL) { dlclose(j.handle); }
continue;
}
if (j.install != NULL) { j.install(); n++; }
/* After the install, so a thunk sees the bodies its own module published.
*
@ -1028,6 +1105,24 @@ static void handle_line(char *line, sink *o) {
free(nb); free(vb);
return;
}
/* How many stopped-only jobs the game thread has dropped for arriving at a
* frame boundary instead of at a break, and the sentence to say about it.
* Answered while running and while stopped alike — it is a count of things
* that have already happened, not a claim about the program's state now.
*
* The count leads and the text follows, because the daemon needs the count
* to decide and the text only to speak. It reads this before it delivers a
* stopped-only module and again while it waits for the value, and a number
* that moved in between is the answer. */
if (strcmp(line, "refusals") == 0) {
char hdr[32];
int k = snprintf(hdr, sizeof hdr, "%llu\n",
(unsigned long long)atomic_load(&refused_while_running));
if (k > 0) emit(o, hdr, (size_t)k);
reply(o, RESUMED);
reply(o, "\n");
return;
}
if (strcmp(line, "result") == 0) {
uint64_t gen = 0, len = 0;
uint64_t cap = flan_dev_result_cap();
@ -1212,6 +1307,23 @@ static void handle_line(char *line, sink *o) {
free(counts); free(bytes); free(typelens); free(types);
return;
}
/* A module that may only run from a break, and the word for it goes in front
* of the path rather than inside the module. The alternative was a fourth
* exported symbol beside [flan_reload_transient], which would have put the
* decision in both emitters and in the .so's ABI — for a fact that is not
* about the module's *contents* at all. What is stopped-only is the
* *question*, not the code: the same rendering machinery, rooted at a frame
* slot instead of a raw address, is perfectly safe to run while the program
* runs. So it travels with the request. See [job].
*
* Everything after the prefix is the ordinary path, gate included, so a
* stopped-only module with no installer is still refused for having no
* installer. */
int stopped_only = 0;
if (strncmp(line, "stopped-only ", 13) == 0) {
stopped_only = 1;
line += 13;
}
/* Before the dlopen, not after it: a module there is no room to queue is
* one there is no point relocating, and refusing here means no handle is
* taken for it at all. Only one producer runs at a time, so room seen now is
@ -1261,7 +1373,9 @@ static void handle_line(char *line, sink *o) {
* dlopen and only this thread consumes it, so this cannot fail; it is
* asserted rather than assumed because a silent [publish] that did nothing
* is the failure being fixed. */
if (!publish((job){ f, c, transient == NULL ? NULL : h }))
if (!publish((job){ .install = f, .call = c,
.handle = transient == NULL ? NULL : h,
.stopped_only = stopped_only }))
fprintf(stderr, "flan: reload queue full after it was checked\n");
return;
}