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

Merge branch 'worktree-agent-a1b7f33fe11893c24' into dev-loop

Browse Source 
# Conflicts:
#	FIX.org
This commit is contained in:
Joseph Ferano 2026-09-21 12:12:36 +07:00
commit ef2ac41172
4 changed files with 270 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

105
FIX.org
View File

@ -5713,3 +5713,108 @@ inside @checks, where CI hides the same thing. Both fixed here. @valgrind,
LLVM-only and there is nothing to track.
- The reload path, which was already outside @sanitize: a redefinition module
is built by llc and ld, so nothing instruments it.
* An accepted re-run is running, 2026-09-21
Reported as an intermittent failure of test_dev's re-run loops under load:
"after three re-runs the program never printed \"counter 44\"", with the
transcript stopping at counter 43 — three requests, two runs — and, less
often, the second request refused outright with "rerun: the program is already
running".
Two faces, one cause, and the cause is a state that lagged a decision.
[flan_merged_rerun] took the request under the lock and left [program_state]
as it found it: PROGRAM_PARKED. The flip to PROGRAM_RUNNING happened on the
parked thread, when it got round to waking — and [describe]'s [:parked] reads
that same state. So a caller that asks for a re-run and then waits for the
program to park again was liable to be answered by the park it had just ended.
The wait fell through immediately, the next request went out while the first
run had not started, and one of two things happened: [program_asked] was still
1, so both requests were answered ok and a single run came of them; or the
thread had woken in between, and the second was refused as "already running".
test/test_dev.ml does exactly this shape in four places, three of them in a
loop, which is why it was the tests that found it.
The window was documented. The comment over [flan_merged_park] said two
re-runs arriving in it are both answered ok for one run, called it a
microsecond wide before the state flip moved in front of the flush and "as
wide as a flush" after, and traded it against a refusal that was simply false.
What it did not account for is the drain that later went in front of the park's
exit: the leaving round polls the agent's ring — a dlopen and a module install,
or a thunk that stops in the break loop — before it breaks. The window was no
longer a flush. It was however long the next thing the program had to do took.
Fixed by making the state the decision rather than a report of it. The store
is in [flan_merged_rerun], under the lock that accepted the request: accepted
means running, and there is no moment in which a committed re-run reads as
parked. The park's own store on the way back into main stays, now a no-op, as
the one place that knows the thread really is on its way.
Considered and rejected: teaching the tests to wait on something else — a line
of the new run's output, a counter — rather than on the park. That is the
right fix only if the daemon's contract really is "parked until the thread
wakes", and it is not: the C refuses a second re-run precisely because the
first is committed, so the two answers disagreed about the same fact. The
contract wanted stating, not working around.
Also considered: keeping the state and refusing a second request while
[program_asked] is set. It closes the collapse and leaves the refusal — the
wait for a park is still answered by the stale one, and the next request is
still refused. Only the state carries both.
Nothing depends on PARKED lasting until the wakeup. [Program.state] has three
readers: [Dev.liveness], which is every op's guard; [describe]'s [:parked];
and [flan_merged_wake], which declines a running program because a running
program polls its own ring. The last is the only one whose answer changes in
the window, and the decline is right there too — the round the thread leaves
on drains the ring before main is re-entered, and what misses that drain is
picked up at the new run's first frame boundary, exactly as for any running
program. No test asserts [:parked t] after an accepted re-run; the one that
asserts it against a thunk stopped in the park asks before any re-run.
There is no path where the request is taken and the run does not happen. The
only way out of the park's loop is the [program_asked] test, and the thread is
either in the wait or in the agent poll that wait runs. What can delay it is a
thunk that stopped in the break loop, which holds the thread inside the poll
until somebody resumes it. The run is still committed — that is why [rerun]
accepts it and says so in a note — and for the duration the state says running
while no frame is executing. Deliberate: [:stopped] is what reports a break
loop, [:parked] reports a park, and this is neither. [Dev.rerun] reads the
liveness and the break for that note *before* it calls [Program.rerun], since
after the call the answer is running by construction.
The two-process daemon has no such window. Its program is a child process with
no park at all: [flan_program_rerun] finds the weak symbol null and answers
"this session's program is a process of its own", and liveness comes from
[waitpid] — a child that finishes is Gone, not parked.
Measured rather than argued, on the loop at test/test_dev.ml:5817 driven
standalone against programs/dev-rerun.flan — park, three re-runs each waited
for by [describe], then the transcript. Unloaded: 3 failures in 28 runs, both
faces among them. Under two busy-loop burners, this machine's load average
running 8 to 16: 9 failures in 25 runs. After the fix, same harness and the
same two burners at the same load: 0 in 50.
One test is strengthened rather than served by this. test_dev.ml:763 asserts
that a re-run asked for while the program is running is refused as "already
running" — and that assertion was itself racy: it fires after the run it
re-ran has been waited for, so the run could have finished and parked, and the
refusal it demands would not have come. With the flip at the acceptance the
RUNNING state starts earlier and covers more of that gap. It is a narrower
window, not a closed one; if that row ever flakes, the answer is to make the
test ask while a run is demonstrably in progress, not to widen the state
further.
Three sentences went stale the moment the state moved, and were rewritten with
it — the same class of defect as the one being fixed, since each was correct
only while a committed re-run still read as parked. [flan_merged_park]'s note
that the program stays PARKED across a poll, now true of every round but the
one it leaves on. [Program.rerun]'s refusal, which told a reader to close a
window or let the program finish when what holds it is a break loop: it takes
what the caller already knows and says resume or abort instead. And the
five-second timeout in [eval_expr] and [run_render_thunk], which reached its
break-loop sentence through the [Parked] arm and would otherwise have asked
whether a game loop calls [(agent/poll)] about a program stopped between runs.
Related but separate, and not this: test_reload's "the registry read under a
writer" is a reader racing a writer over the allocation registry, about one run
in five in isolation, and shares nothing with this but the word intermittent.

17
docs/BUILT.md
View File

@ -1458,6 +1458,23 @@ semantics and it is what was asked for. A clean slate is one evaluation away; a
**A re-run while the program is running is refused, not queued.** The test and the signal are under one mutex, so the
window between them does not exist, and two `main`s writing the same globals at once never starts.
**An accepted re-run is running, from the moment it is accepted.** `flan_merged_rerun` stores `PROGRAM_RUNNING` itself,
under the lock that took the request, rather than leaving the state as it found it for the parked thread to update when
it wakes. That matters to anyone who asks for a re-run and then waits for the program to park again — which is what an
editor does, and what the tests do. While the state lagged the decision, the wait was answered by the park the re-run
had just ended: a second re-run then went out before the first run had started, and the two of them produced one run
between them, or arrived a moment later and were told the program was already running. One state that lagged, two
symptoms. The park's own store of `PROGRAM_RUNNING` on the way back into `main` is now a no-op that documents where the
thread has got to.
The case an editor author has to know about is `:parked nil :stopped t` with no frames running. An expression evaluated
against a park can stop on a condition, which leaves the program's one thread inside the break loop; a re-run asked for
then is accepted — it is the next thing that thread will do — so the state says running while nothing is executing.
That is not a lie about a park: `:parked` reports a park, `:stopped` reports a break loop, and this is a break loop
with a run waiting behind it. `rerun`'s reply says so in its `:note`, a second `rerun` is refused with the resume-or-abort
sentence rather than the close-the-window one, and an evaluation that times out there names the break too. The run
starts when the break is resumed or aborted.
`close`ing stdout went with this change, and it had to. That was how the compiler learned the program was done — the
pipe read EOF, exactly as the two-process daemon learns it from a dead child — but a pipe delivers EOF *once*, so the
signal and the program's output were the same resource: spending it left the second run with nowhere to print. The

150
lib/dev.ml
View File

@ -1263,6 +1263,23 @@ let eval_expr t ~code ~origin ~pause =
program is parked, so nothing is competing with it: the \
thunk is most likely stopped on a condition inside the \
break loop, which restart or abort answers"
(* The same cause without the park to name it. An earlier
evaluation can stop on something and still be sitting in the
break loop when this one arrives — [`Broke] above sees only
stops entered after the request — and the thread is then in
no state to run anything. That used to be reached through
the [Parked] arm, which is where such a thread was; it is
not any more once a re-run has been accepted, because taking
one ends the park while the break goes on holding the
thread. Sending that reader to look for [(agent/poll)] in a
game loop would be sending them away from the break buffer
their editor has open. *)
else if parked_break t then
error
"the expression produced no value in five seconds, because \
the program is stopped at an earlier break and runs \
nothing until that ends. Take a restart or abort in the \
break buffer, and evaluate this again"
(* And a third cause, which is the one a session now reaches
early enough to hit: the program has not bound its agent
socket, so it is still ahead of its own [(agent/start ...)]
@ -1825,7 +1842,10 @@ let run_render_thunk ?(stopped_only = false) ?at_stop t ~tag
thing not to say here. A parked thread has nothing competing
with it, so what it is doing is holding a break — the thunk
asked for a stopped stack and the stop has been let go of. *)
let gave_up =
(* A function and not a value, because the middle arm asks the
agent and this is the body of a five-millisecond tick. Only
the tick that gives up needs the answer. *)
let gave_up () =
if liveness t = Parked then
Error
"the inspection produced nothing in five seconds. The \
@ -1833,12 +1853,27 @@ let run_render_thunk ?(stopped_only = false) ?at_stop t ~tag
the break it was built against has most likely been \
resumed since, and a stopped-only job is dropped rather \
than run against a resumed program"
(* Still at a break, and not parked — a program stopped during
a run, or one whose park has ended because a re-run was
accepted while the break went on holding the thread. The
break loop is the poll loop, so the job had somewhere to
run; what it did not have is the stop it was built against,
which is the same story the parked sentence tells. Naming
the game loop here would send the reader past the break
buffer that is holding their program. *)
else if parked_break t then
Error
"the inspection produced nothing in five seconds. The \
program is stopped at a break, and a stopped-only job is \
dropped rather than run once the stop it was built against \
has been left behind. Ask again from the break buffer as \
it stands now"
else
Error
"the program did not reach a frame boundary; is it calling \
(agent/poll)?"
in
if ms <= 0 then gave_up
if ms <= 0 then gave_up ()
else begin
ignore (Unix.select [] [] [] 0.005);
(* Not [Gone], for the reason [eval_expr]'s own wait now gives:
@ -1846,7 +1881,7 @@ let run_render_thunk ?(stopped_only = false) ?at_stop t ~tag
frame boundary, and a render job asked for at a break the
park is holding is running in that break's own poll. Gone is
the only state no amount of waiting recovers from. *)
if liveness t <> Gone then wait (ms - 5) else gave_up
if liveness t <> Gone then wait (ms - 5) else gave_up ()
end
in
wait 5000
@ -3074,12 +3109,35 @@ let abort t =
The state is asked twice — here, to say something useful about [Gone], and
again inside [Program.rerun], which is the answer that counts. The C does
its test and its signal under one lock, so the window between them that this
check cannot see is a window that does not exist there. *)
check cannot see is a window that does not exist there.
ACCEPTED MEANS RUNNING, and an editor can rely on it. The C stores the new
state as it takes the request rather than when the parked thread gets round
to waking, so a session that asks for a re-run and then waits for the next
park is waiting for the park this run ends with — never for the one it just
left. Before that, the two were indistinguishable for as long as it took the
thread to wake: a wait for "parked" was answered immediately by the old
park, the next request went out while the first run had not started, and the
pair of them produced one run — or, a moment later, a refusal saying the
program was already running. Both faces are gone with the lag. *)
let rerun t =
match liveness t with
| Gone -> error gone
| Live | Parked ->
(match Program.rerun () with
(* Read before the request and not after it. Taking a re-run is what ends
the park — the C stores the new state as it accepts — so by the time
there is an [Ok] to describe, this session's program reads as running
whatever it was doing a moment ago, and the note below would have lost
the one case it exists for.
[at_break] is asked of the agent whichever state the program is in,
because both answers need it and neither can be inferred from the other.
Accepted and stopped is the note below; refused and stopped is a refusal
whose advice would otherwise be about a window to close — see
[Program.rerun]. *)
let at_break = parked_break t in
let stopped_in_park = liveness t = Parked && at_break in
(match Program.rerun ~stopped:at_break () with
| Ok () ->
(* The park this session was explaining is over, so the park after it
gets the explanation again — see [install_note]. Cleared here as
@ -3096,7 +3154,7 @@ let rerun t =
here, unlike the running case the C refuses: there is no second main
about to start, only one waiting for the thread to be free. *)
let note =
if liveness t = Parked && parked_break t then
if stopped_in_park then
"accepted; an expression evaluated against the park is stopped in \
the break loop, so main starts once that is resumed or aborted"
else
@ -4397,11 +4455,28 @@ static void flan_merged_exit(int32_t status) {
* waiting. Nothing is lost in it: [flan_merged_rerun] sets [program_asked] and
* signals under the same lock, a signal delivered to nobody is discarded, and
* the [while] below reads the flag rather than the wakeup — so a request that
* lands in the window is taken and the wait falls straight through. The one
* visible cost is that two re-runs arriving in that window are both answered
* "ok" for a single run. That race existed before and was a microsecond wide;
* it is now as wide as a flush, which is the right trade against a refusal
* that was simply false.
* lands in the window is taken and the wait falls straight through.
*
* THAT WINDOW USED TO HAVE A COST, and it was paid by whoever asked twice.
* A taken re-run left the state PARKED until this thread got round to waking,
* so the window was one in which the program was going to run and every
* question about it still answered "parked" — as wide as a flush, and wider
* still once the drain below went in front of the exit, because a poll that
* loads a module is in it too. An editor that asks for a re-run and then waits
* for the next park was therefore liable to be answered by the park it had
* just ended: the wait fell through on the *old* park, the next request landed
* in the same window, and the two of them produced one run between them —
* unless the thread woke in between, in which case the second was refused as
* "already running". One cause, two faces, and both are a state that lagged
* the decision.
*
* So the decision is the state now: [flan_merged_rerun] stores
* PROGRAM_RUNNING itself, under the same lock as the test that accepted the
* request. Nothing between the acceptance and the thread's return into main
* can be mistaken for a park, because there is no longer a moment in which an
* accepted re-run reads as parked. The store below is what that leaves: the
* ordinary way out of this loop, and a no-op on the state by the time it
* runs.
*
* ── THE SECOND THING THE PARK SERVICES ───────────────────────────────
*
@ -4421,10 +4496,18 @@ static void flan_merged_exit(int32_t status) {
* So the wait has two flags and not one, and the difference between them is
* what the thread does next: [program_asked] leaves the park and
* [program_poll] waits again. What they no longer differ about is the ring,
* which is drained on the way round either way — see the loop. The program
* stays PROGRAM_PARKED across the whole of a poll — a thunk is not a run, and
* an editor that saw [:parked nil] for the duration of a C-x C-e would show
* the program as live for a moment that has no frames in it.
* which is drained on the way round either way — see the loop. A poll the
* program is only visiting leaves it PROGRAM_PARKED throughout — a thunk is
* not a run, and an editor that saw [:parked nil] for the duration of a C-x
* C-e would show the program as live for a moment that has no frames in it.
*
* The round it leaves on is the exception, and it is one by the same rule
* rather than against it. That round begins with a re-run already taken, and a
* taken re-run is a run: [flan_merged_rerun] has stored PROGRAM_RUNNING
* before this thread is even awake. So the drain in front of the exit happens
* with the program reading as running, which is what it is — on its way into
* main, installing what was queued for that run. The state follows the
* decision either way; only the decision differs.
*
* A re-run is tested first, so a stream of evaluations cannot starve one. The
* poll flag is cleared BEFORE the lock is dropped, which is what makes a
@ -4490,6 +4573,10 @@ static void flan_merged_park(void) {
if (leaving) break;
}
program_asked = 0;
/* Already PROGRAM_RUNNING — [flan_merged_rerun] stored it when it took the
* request. Stored again because this is the only place that knows the thread
* is really on its way back into main, and a store that is a no-op is
* cheaper than a reader that has to know which of the two did it. */
program_state = PROGRAM_RUNNING;
pthread_mutex_unlock(&program_lock);
}
@ -4506,16 +4593,32 @@ static void flan_merged_park(void) {
* between the finished run's longjmp and the park is either seen as running
* (refused, and the program parks a moment later) or seen as parked (taken).
* The park flips the state before it flushes, so the second is now the usual
* answer rather than the lucky one, and what that widens is written out
* there. Queueing it instead would mean a second main starting the
* instant the first finished, which is never what somebody pressing a key
* meant. */
* answer rather than the lucky one. Queueing it instead would mean a second
* main starting the instant the first finished, which is never what somebody
* pressing a key meant.
*
* TAKING THE REQUEST IS WHAT ENDS THE PARK, so this says so rather than
* leaving it to be said later. The state is stored here, under the lock that
* accepted the request, and not on the parked thread when it happens to wake:
* the park is over the moment a run is committed to, and everything anyone can
* ask between the two is answered by that. Leaving it to the wakeup left a
* window in which an accepted re-run still read as parked — which is the whole
* of the race written out over [flan_merged_park].
*
* The thread has no say in it and needs none: it is asleep on a condition
* variable inside this park or in the agent poll that park runs, and both
* paths end at the [program_asked] test. Nothing between here and main can
* decline. The one thing that can delay it is a thunk that stopped in the
* break loop, which holds the thread inside the poll until somebody resumes
* it — the run is still committed, still the next thing this thread does, and
* the editor is told so by [rerun]'s own note rather than by the state. */
int flan_merged_rerun(void) {
int rc;
pthread_mutex_lock(&program_lock);
if (program_state != PROGRAM_PARKED) rc = 1;
else {
program_asked = 1;
program_state = PROGRAM_RUNNING;
pthread_cond_signal(&program_wake);
rc = 0;
}
@ -4533,7 +4636,14 @@ int flan_merged_rerun(void) {
*
* A running program is refused rather than woken, because there is nothing to
* wake: its game thread reaches a frame boundary on its own and polls there.
* The caller treats that as "no wake was needed", not as a failure. */
* The caller treats that as "no wake was needed", not as a failure.
*
* A program with a re-run already taken reads as running here, and the refusal
* is right for the same reason: the parked thread is on its way out of the
* park, and the round it leaves on drains the ring before it re-enters main.
* A delivery that gets there in time is picked up by that drain and the rest
* at the new run's first frame boundary, which is what a running program does
* with a delivery anyway. */
int flan_merged_wake(void) {
int rc;
pthread_mutex_lock(&program_lock);

20
lib/program.ml
View File

@ -71,10 +71,26 @@ let wake () = ignore (raw_wake ())
The refusal is the program's to make, not this end's. Checking the state
here and signalling after would leave a gap for the program to finish or
start in; the C does both under one lock, so a request is either taken or
told the program is running, and never both. *)
let rerun () =
told the program is running, and never both.
[stopped] is the one thing the refusal cannot see for itself, and without it
the advice is wrong in the case that needs it most. The C knows the program
is not available; it does not know that the thread is sitting in a break
loop, which is a thing the agent reports and this end reads. "Close its
window, or let it finish" is what to do about a program that is running; it
is no use at all to somebody whose evaluation stopped on a breakpoint, or
who has already asked for a re-run that is waiting on that same break. Both
of those are ended by resuming or aborting, so that is what those are told.
The caller passes what it already knows — see [Dev.rerun]. *)
let rerun ?(stopped = false) () =
match raw_rerun () with
| 0 -> Ok ()
| 1 when stopped ->
Error
"the program is stopped at a break, so main cannot be started or \
restarted until that ends: resume it or abort it, and the program goes \
on from there. If a re-run was already accepted, it is waiting on the \
same thing and needs no second request"
| 1 ->
Error
"the program is already running; a re-run starts main again, and two \