The agent binds before main, so a dev program need not start it at all
A constructor in the agent package binds FLAN_AGENT_SOCKET when it is set, which is the daemon and nothing else — both shapes set it, before the fork in --two-process and before the exec in the merged build. So a program under [flan dev] that calls (agent/poll) and has no (agent/start) in it takes redefinitions anyway, and one that does call start meets an agent that is already listening and gets a no-op. The window this closes was the complaint in DISCUSS.org: a program that opens a window before starting its agent leaves the daemon waiting on a socket that does not exist yet. Bound here, the socket exists before main whatever the program does afterwards — so test_dev.ml's late-agent row asserts the negation of what it used to. The delivery sent during the sleep no longer carries "the program has not called (agent/start ...) yet", because that is no longer true of it; what is still late, and still asserted, is the poll that installs it. It reaches exactly as far as the linker does. Reach prunes a package nothing calls into, so a program that mentions the agent nowhere does not link this file and has no constructor to run: auto-start is for a program that polls and has dropped its start call, not for one that says nothing about the agent at all. That limit and the release-build residual are in FIX.org, along with the daemon branch that can no longer be reached. agent-nostart.flan is the pin, and its two numbers are the honest ones: 1 before anything could arrive, 1000 after the wait, because a listener bound before main is still not an install.
This commit is contained in:
parent
e9c0e96a9b
commit
77d6e43b09
72
FIX.org
72
FIX.org
@ -2442,3 +2442,75 @@ failures were that row and the sixth was ~dev-trap-free-all~, so what is racy
|
||||
is ~trap_park~ itself and every row that calls it — which is exactly what the
|
||||
mechanism described there predicts. Per the sweep policy the ~@x86~ and
|
||||
~@sanitize~ sweeps were not run here.
|
||||
|
||||
* (agent/start) lost its argument, 2026-09-20
|
||||
Four notes from the lane that made the socket path optional and bound it
|
||||
before main. Three of them are about ground this lane deliberately did not
|
||||
take; the fourth is a line the author can delete at leisure.
|
||||
|
||||
** sand.flan can drop its socket path
|
||||
=(agent/start "/tmp/flan-sand.sock")= at sand.flan:125 still works and always
|
||||
will — the explicit form is not going anywhere. But the path was only ever a
|
||||
value nothing read under =flan dev=, because the daemon overrides it through
|
||||
FLAN_AGENT_SOCKET, and the zero-argument =(agent/start)= now does the right
|
||||
thing in both places: the daemon's socket when there is one, and an announced
|
||||
=/tmp/flan-agent-<pid>-<clock>.sock= when there is not. Changing that line is
|
||||
a one-word edit whenever the author feels like it; this lane does not touch
|
||||
sand.flan.
|
||||
|
||||
With the constructor below, sand.flan could delete the call outright — it
|
||||
calls =(agent/poll)= in its frame loop, which is the condition. That is a
|
||||
bigger claim than a shortened line and is worth making deliberately.
|
||||
|
||||
** Auto-start reaches as far as the linker does, and no further
|
||||
=vendor/agent/flan_agent.c='s =auto_start= constructor binds FLAN_AGENT_SOCKET
|
||||
before main, so a program under =flan dev= needs no =(agent/start)= at all.
|
||||
What it cannot do is reach a program that never mentions the agent: =Reach=
|
||||
prunes a package nothing calls into, so an executable with no =(import agent
|
||||
...)= — or one that imports it and calls nothing — does not link the file the
|
||||
constructor is in. There is nothing to run.
|
||||
|
||||
So the true scope is: *a program that calls =(agent/poll)= or =(agent/wait)=
|
||||
and has dropped its start call*. That is the ceremony the feature was asked to
|
||||
remove, and it is removed. Full invisibility — a dev build that links the
|
||||
agent because it is a dev build, whether or not the source says so — needs the
|
||||
package force-linked from Load/Build, which are files this lane did not own
|
||||
and a decision about what =--dev= means rather than about the agent.
|
||||
|
||||
The constructor is also not =--dev=-only, because nothing in
|
||||
=vendor/agent/flan_agent.c= can tell the two builds apart: the dev runtime is
|
||||
linked either way and there is no weak symbol to ask. A *release* binary that
|
||||
links the agent and is run with FLAN_AGENT_SOCKET set in its environment
|
||||
therefore binds a listener it would not have bound before. Only =flan dev=
|
||||
sets that variable and it never runs release builds, so this is a sentence
|
||||
about the shape of the gate rather than an observed problem — but it is the
|
||||
one behavioural difference outside the dev loop and it should be said.
|
||||
|
||||
** The daemon's "has not called (agent/start ...)" note is now unreachable
|
||||
=install_note= (lib/dev.ml:789) and the =describe= branch at lib/dev.ml:1085
|
||||
say, of a RUNNING program whose socket is not bound, that a redefinition
|
||||
installs at its next =(agent/poll)= and not at all if there is none. For a
|
||||
program that links the agent that cannot happen any more: the constructor
|
||||
binds before main, so by the time any editor can ask, =agent_bound= is true.
|
||||
|
||||
The sentence is still reached — and still right — for a program that does not
|
||||
link the agent at all, which is =programs/dev-noagent.flan= and the row in
|
||||
test_dev.ml that drives it. So nothing here is dead; what is dead is the
|
||||
*late* case, which is what test_dev.ml's late-agent row used to assert and now
|
||||
asserts the negation of. Retiring the branch is the author's call over a lane
|
||||
that merged days ago, not this one's.
|
||||
|
||||
** The agent socket under the daemon is still the temp directory's problem
|
||||
=start_on= now stashes the path it bound and unlinks it three ways: an atexit
|
||||
for an ordinary exit, and by hand in =die_now= and =orphan_die=, which both
|
||||
leave by =_exit= and skip the atexit chain deliberately. That covers a program
|
||||
run on its own, a program aborted out of the break loop, and an orphan whose
|
||||
daemon died.
|
||||
|
||||
It does not cover an ordinary =flan dev= session ending, and cannot: the
|
||||
two-process daemon kills its child with SIGTERM and the merged session leaves
|
||||
by =Unix._exit 0=, neither of which runs an atexit. The socket sits in
|
||||
=/tmp/flan-dev-<pid>/= and goes when that directory goes — which is the item
|
||||
above, "The daemon leaves its temp directory behind", still open. No separate
|
||||
fix is wanted here; the session-end cleanup that item asks for takes the
|
||||
socket with it.
|
||||
|
||||
27
test/programs/agent-nostart.flan
Normal file
27
test/programs/agent-nostart.flan
Normal file
@ -0,0 +1,27 @@
|
||||
;;;; A program with no (agent/start) in it at all, which still takes a
|
||||
;;;; redefinition.
|
||||
;;;;
|
||||
;;;; The listener comes from the package's constructor, which binds
|
||||
;;;; FLAN_AGENT_SOCKET when the daemon has set it — so under [flan dev] there
|
||||
;;;; is nothing to call and nothing to forget. Installing is still the
|
||||
;;;; program's own decision and still happens where it says: [wait] is the
|
||||
;;;; headless spelling of the poll at the top of a frame, and the second number
|
||||
;;;; below cannot appear before it.
|
||||
;;;;
|
||||
;;;; It does import the agent and it does call into it. That is not an accident
|
||||
;;;; of how the fixture was written: [Reach] prunes a package nothing calls, so
|
||||
;;;; an executable that mentions the agent nowhere does not link the
|
||||
;;;; constructor either. See flan_agent.c's [auto_start].
|
||||
(import agent "vendor:agent")
|
||||
|
||||
(defvar ticks i64)
|
||||
|
||||
(defn tick [] i64
|
||||
(set ticks (+ ticks 1))
|
||||
ticks)
|
||||
|
||||
(defn main [] i32
|
||||
(print (tick)) (println "")
|
||||
(while (= (agent/wait 100) 0) 0)
|
||||
(print (tick)) (println "")
|
||||
0)
|
||||
@ -12,6 +12,12 @@
|
||||
;;;; Two claims are checked against it in test_dev.ml: the session answers
|
||||
;;;; while the sleep is still running, and a redefinition sent during the
|
||||
;;;; sleep is installed once the program is up.
|
||||
;;;;
|
||||
;;;; The socket is no longer late, only the call is. The package's constructor
|
||||
;;;; binds it before main when a daemon has said where, so the start below
|
||||
;;;; arrives at an agent that is already listening and is a no-op answering the
|
||||
;;;; same socket — which is what makes this fixture the double-start case under
|
||||
;;;; a real daemon as well.
|
||||
(import agent "vendor:agent")
|
||||
|
||||
(defvar frames i64)
|
||||
|
||||
@ -269,6 +269,74 @@ let () =
|
||||
List.iter (fun f -> try Sys.remove f with Sys_error _ -> ())
|
||||
[ aexe; aso; aout; aerr ];
|
||||
|
||||
(* ── No (agent/start) at all ────────────────────────────────────── *)
|
||||
|
||||
(* The ceremony gone rather than shortened: this program says nothing about
|
||||
the agent except that it installs what has arrived. The listener is the
|
||||
package's constructor, and the condition it runs on is
|
||||
FLAN_AGENT_SOCKET — which is exactly what the daemon sets, in both
|
||||
shapes, before the program starts. Setting it here is therefore not a
|
||||
test fixture standing in for the daemon; it is the same variable set the
|
||||
same way, which is why this does not need a daemon to be the claim.
|
||||
|
||||
The two numbers are the split the agent exists for, and they are what
|
||||
keeps this honest: a listener bound before main is still not an install.
|
||||
1 is the body the program was built with, printed before anything could
|
||||
have arrived; 1000 is a body that did not exist then, and it appears
|
||||
only after the [wait] — the program's own frame boundary — which is the
|
||||
half no constructor can do for it.
|
||||
|
||||
[Reach] is the limit, and the fixture's header says so: the package is
|
||||
linked because the program calls into it. A program that called nothing
|
||||
would not have this constructor in it to run. *)
|
||||
let nsock = tmp "nostart.sock" and nout = tmp "nostart.out" in
|
||||
(try Sys.remove nsock with Sys_error _ -> ());
|
||||
let nt, nl = Session.create ~file:"programs/agent-nostart.flan" () in
|
||||
let nexe = tmp "nostart" in
|
||||
ignore
|
||||
(Build.executable ~opts:dev ~csrcs:nl.Load.csrcs ~lflags:nl.Load.lflags
|
||||
nt.Session.host ~out:nexe);
|
||||
let nso = tmp "nostart-tick.so" in
|
||||
let nc = Session.eval nt "(defn tick [] i64 1000)" in
|
||||
ignore (Build.shared ~opts:dev ~ir:nc.Session.ir ~out:nso ());
|
||||
let nenv =
|
||||
Array.append (Unix.environment ()) [| "FLAN_AGENT_SOCKET=" ^ nsock |]
|
||||
in
|
||||
let nfd =
|
||||
Unix.openfile nout [ Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC ] 0o600
|
||||
in
|
||||
let npid = Unix.create_process_env nexe [| nexe |] nenv Unix.stdin nfd nfd in
|
||||
Unix.close nfd;
|
||||
if not (await (fun () -> Sys.file_exists nsock)) then begin
|
||||
fail "a program with no (agent/start) never bound %s" nsock;
|
||||
(try Unix.kill npid Sys.sigkill with Unix.Unix_error _ -> ())
|
||||
end
|
||||
else begin
|
||||
let r = send nsock nso in
|
||||
if r <> "ok\n" then
|
||||
fail "the constructor's socket refused a module: %S" r;
|
||||
let nstatus = ref (Unix.WEXITED 0) in
|
||||
let reaped =
|
||||
await ~ms:5000 (fun () ->
|
||||
match Unix.waitpid [ Unix.WNOHANG ] npid with
|
||||
| 0, _ -> false
|
||||
| _, s -> nstatus := s; true)
|
||||
in
|
||||
if not reaped then begin
|
||||
(try Unix.kill npid Sys.sigkill with Unix.Unix_error _ -> ());
|
||||
fail "the redefinition never reached a program that never started an \
|
||||
agent"
|
||||
end
|
||||
else begin
|
||||
let text = In_channel.with_open_bin nout In_channel.input_all in
|
||||
if !nstatus <> Unix.WEXITED 0 || text <> "1\n1000\n" then
|
||||
fail "no (agent/start)\n got: %S\n wanted: %S" text
|
||||
"1\n1000\n"
|
||||
end
|
||||
end;
|
||||
List.iter (fun f -> try Sys.remove f with Sys_error _ -> ())
|
||||
[ nexe; nso; nsock; nout ];
|
||||
|
||||
(* ── The break loop, spec-conditions.md §2 ──────────────────────── *)
|
||||
|
||||
(* The claim is that an unhandled [error] stops rather than dying, and can
|
||||
|
||||
@ -5110,13 +5110,28 @@ let () =
|
||||
|
||||
(* ── A program that starts its agent late ──────────────────────────
|
||||
The shape a real one has: sand.flan opens a window and starts the
|
||||
agent afterwards, so the socket appears seconds into the run. The
|
||||
merged session used to wait up to ten seconds for it *before* running
|
||||
its accept loop, which charged those seconds to the first thing the
|
||||
editor asked. Two claims, and the second is what stops the first from
|
||||
being bought with a lie: the session answers during the delay, and a
|
||||
redefinition sent during it is really installed once the program is
|
||||
up. *)
|
||||
agent afterwards, so the program's own [(agent/start ...)] is seconds
|
||||
into the run. The merged session used to wait up to ten seconds for the
|
||||
socket *before* running its accept loop, which charged those seconds to
|
||||
the first thing the editor asked. Two claims, and the second is what
|
||||
stops the first from being bought with a lie: the session answers during
|
||||
the delay, and a redefinition sent during it is really installed once
|
||||
the program is polling.
|
||||
|
||||
The socket itself is no longer late. The agent package's constructor
|
||||
binds FLAN_AGENT_SOCKET before main (vendor/agent/flan_agent.c,
|
||||
[auto_start]), so under a daemon it is there from the first instant
|
||||
whatever the program does afterwards — which is what this fixture's
|
||||
sleep was a stand-in for, and the window DISCUSS.org complained about,
|
||||
closed. What is still late is the *poll*, and that is the half that
|
||||
matters: a module queued now installs when the program reaches its loop
|
||||
and not before.
|
||||
|
||||
So this is also where the double start is pinned against a real daemon.
|
||||
The fixture keeps its explicit [(agent/start ...)] three seconds in,
|
||||
arriving at an agent that is already listening, and the delivery below
|
||||
is installed on the socket the constructor bound — one listener, not
|
||||
two, or the module would be queued on a ring nothing drains. *)
|
||||
let lsock = tmp "lateagent.sock" and lout = tmp "lateagent.out" in
|
||||
(try Sys.remove lsock with Sys_error _ -> ());
|
||||
let lfd =
|
||||
@ -5150,12 +5165,20 @@ let () =
|
||||
fail
|
||||
"the first editor request waited %.1fs on a program whose agent \
|
||||
starts late; the accept loop is gated on the agent again" ldt;
|
||||
(* And the delivery, also inside the delay. It is taken — in one process
|
||||
the agent's ring is reachable whether or not the program has bound a
|
||||
socket — and the note says what "queued" means for a program that has
|
||||
not got to its poll yet. Asserted because the alternative was the
|
||||
reply this whole area exists to prevent: an "ok" that reads as
|
||||
installed. *)
|
||||
(* And the delivery, also inside the delay. It is taken, and it carries
|
||||
no note about [(agent/start ...)]: that sentence is for a program
|
||||
whose socket is not bound, and the constructor bound this one before
|
||||
main. [install_note] answering nothing here is therefore the evidence
|
||||
that the socket is up — the assertion is on the absence because the
|
||||
absence is the claim.
|
||||
|
||||
It used to be the presence. The program's own start call is still
|
||||
three seconds away, so this is the same moment it always was; what
|
||||
changed is that the moment is no longer one in which the program
|
||||
cannot be reached. lib/dev.ml's branch still says the true thing for
|
||||
a program that does not link the agent at all (the agentless row
|
||||
above), and FIX.org records that a dev program which links it can no
|
||||
longer get there. *)
|
||||
let r =
|
||||
request lc
|
||||
"(:op \"eval\" :code \"(defn step [] i64 9)\" :file \
|
||||
@ -5165,10 +5188,10 @@ let () =
|
||||
fail "a redefinition sent before (agent/start ...): %s" (said r)
|
||||
else begin
|
||||
let note = Option.value ~default:"" (Wire.string_field r "note") in
|
||||
if not (contains_sub note "(agent/start ...)") then
|
||||
if contains_sub note "(agent/start ...)" then
|
||||
fail
|
||||
"a redefinition delivered before the program's agent was up said \
|
||||
nothing about it: %S" note
|
||||
"the agent socket was not bound before main, so a delivery was \
|
||||
told to wait for a call the program had not made: %S" note
|
||||
end;
|
||||
(* The claim the note makes, checked against the program rather than
|
||||
against the reply: once the sleep is over and the program is polling,
|
||||
|
||||
52
vendor/agent/flan_agent.c
vendored
52
vendor/agent/flan_agent.c
vendored
@ -593,10 +593,18 @@ static char bound_sock[sizeof(((struct sockaddr_un *)0)->sun_path)];
|
||||
* break-loop abort die_now, by hand, because it takes _exit
|
||||
* orphaned child orphan_die, by hand, for the same reason
|
||||
*
|
||||
* Under [flan dev] the socket also sits inside the daemon's temp directory,
|
||||
* but that directory is not removed today (FIX.org, "The daemon leaves its
|
||||
* temp directory behind"), so this is the only thing that takes the socket
|
||||
* away. Outside the daemon the path is under /tmp and nothing else would. */
|
||||
* Outside a daemon that is the whole story, and the path is under /tmp where
|
||||
* nothing else would ever reclaim it.
|
||||
*
|
||||
* Under [flan dev] none of the three is how a session usually ends, and this
|
||||
* is worth being exact about rather than claiming cover it does not give: the
|
||||
* two-process daemon kills its child with SIGTERM, whose default disposition
|
||||
* runs no atexit, and the merged session leaves by [Unix._exit 0]. So the
|
||||
* socket there is left in the daemon's temp directory — which is itself never
|
||||
* removed today. FIX.org, "The daemon leaves its temp directory behind": the
|
||||
* session-end cleanup that item asks for takes the socket with it, and until
|
||||
* it lands the socket outlives the session. Nothing below can fix that from
|
||||
* here; a program that is killed does not get to tidy up. */
|
||||
static void unlink_bound_sock(void) {
|
||||
if (bound_sock[0] != '\0') unlink(bound_sock);
|
||||
}
|
||||
@ -1880,3 +1888,39 @@ int32_t flan_agent_start_auto(void) {
|
||||
}
|
||||
return r < 0 ? -1 : 0;
|
||||
}
|
||||
|
||||
/* And the call itself, gone. A program under [flan dev] that imports this
|
||||
* package gets the listener before main, without asking.
|
||||
*
|
||||
* FLAN_AGENT_SOCKET is the whole condition, and it is the right one: the
|
||||
* daemon sets it in both shapes — before the fork in --two-process, before the
|
||||
* exec in the merged build — and nothing else on a machine sets it. So an
|
||||
* ordinary run of an ordinary program falls straight through here and this
|
||||
* costs it one getenv. (Not FLAN_DEV_PARENT, which is deliberately unset in
|
||||
* the merged build; gating on it would quietly skip half the daemon.)
|
||||
*
|
||||
* WHAT THIS DOES NOT REACH, because it is a fact about linking rather than a
|
||||
* choice made here: [Reach] prunes a package nothing calls into, and an
|
||||
* executable that never mentions the agent does not link this file at all — so
|
||||
* there is no constructor in it to run. Auto-start covers a program that calls
|
||||
* [(agent/poll)] and has dropped its [(agent/start)], which is the ceremony
|
||||
* this was asked to remove. A program that wants a dev loop while calling
|
||||
* nothing at all would need the package force-linked into every --dev build,
|
||||
* which is a decision for Load and Build and not for this file.
|
||||
*
|
||||
* The window it closes is real and was costing seconds: a program that opened
|
||||
* a window before its [(agent/start)] left the daemon waiting on a socket that
|
||||
* did not exist yet (DISCUSS.org). Bound here, it exists before main.
|
||||
*
|
||||
* A pthread from a constructor is fine — it is this executable's own init, not
|
||||
* a dlopen, so nothing holds the loader lock against it — and the thread it
|
||||
* starts does nothing until something connects. The narrow hazard is a request
|
||||
* arriving before [flan_rt_init]: the verbs read runtime state, and a person
|
||||
* with nc and very quick fingers could ask before there is any. The daemon
|
||||
* cannot, in either shape — it has an editor to hear from first, and a module
|
||||
* to compile after that. */
|
||||
__attribute__((constructor)) static void auto_start(void) {
|
||||
const char *env = getenv("FLAN_AGENT_SOCKET");
|
||||
if (env == NULL || env[0] == '\0') return;
|
||||
(void)start_on(env);
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user