The job ring never looked at tail, and the comment described a drop it never did

publish() wrote queue[head % QUEUE] without consulting tail, so the 65th module
queued between two agent/poll calls landed on the slot the game thread was
reading — twenty-four bytes of function pointers copied field by field with no
atomic near them, so the consumer could take half of one job and half of
another and call it. The comment claimed the overflow dropped the oldest
request; nothing did that.

A full ring is refused now, at the sender, before the dlopen. Dropping loses a
reload the sender was told was ok, which is the same lie more quietly; blocking
stalls the accept loop, which serves connections inline, so a program that had
stopped polling would also stop answering status and abort — the dev loop would
have no way to reach a program that had stopped listening to it. The check is
separate from the store because there is one producer: room, once seen, cannot
be taken away.

Two smaller defects in the same file:

A module with no flan_reload_install was refused and its handle dropped on the
floor. Not an exception to "nothing is ever dlclosed" — that rule is about a
module something points into, and this one installed nothing, so no cell names
it. What leaked was the handle value rather than the mapping: dlopen refcounts
by path, so re-sending the same bad file raised a count nothing could lower.

exit(134) from the break loop runs the atexit chain and the ELF destructors,
which want the loader lock the listener thread may be holding inside dlopen. A
program asked to abort would hang instead of dying. _exit, with the streams
flushed by hand at each call site. The deadlock itself is read rather than
tested; what the tests pin is that the exit status is still 134.

programs/agent-queue.flan blocks on stdin so the window is held open by the
test rather than by a timer: it takes 64 modules, refuses the 65th with a
reason, and installs 64 when it finally polls. noinstall.c's destructor prints
while the program is still running, which is the only way to see the close — at
exit the loader runs every destructor whether anything was closed or not. Both
halves fail on the old code.

test/dune

@@ -25,6 +25,9 @@
   (glob_files programs/*.flan)
   ; The reload primitive's host: a C main that dlopens what Build.shared made.
   (file reload_host.c)
+  ; A shared object that is not a redefinition module, for the agent's refusal
+  ; path. Its destructor is what proves the handle was closed rather than lost.
+  (file noinstall.c)
   ; test_dev runs the compiler itself: flan dev launches and owns a program.
   (file %{workspace_root}/bin/main.exe)
   ; The Emacs client, which test_emacs drives against a real daemon.

test/noinstall.c

@@ -0,0 +1,16 @@
+/* A shared object that is not a redefinition module: it loads, and it has no
+ * flan_reload_install for the agent to find.
+ *
+ * The destructor is the observation. The agent used to drop the handle on the
+ * floor when it refused a module like this one — the mapping stayed, the
+ * reference count went up, and the one handle that could have brought it down
+ * was gone. Now the module is closed, because nothing was installed from it
+ * and so nothing can point into it, and this line appears while the program is
+ * still running. Waiting for the program to exit would prove nothing: the
+ * loader runs every destructor at exit whether anything was closed or not. */
+#include <stdio.h>
+
+__attribute__((destructor)) static void unloaded(void) {
+  printf("unloaded\n");
+  fflush(stdout);
+}

test/programs/agent-queue.flan

@@ -0,0 +1,36 @@
+;;;; The job ring between the listener thread and the game thread, and what it
+;;;; does when it is full.
+;;;;
+;;;; The window this needs is "the listener has queued modules the game thread
+;;;; has not looked at yet", and that window has to be held open by something
+;;;; other than a timer — how long sixty-five connections take on a loaded
+;;;; machine is exactly the kind of race a test must not be. So the program
+;;;; blocks on stdin: the test fills the ring, checks what the agent said, and
+;;;; only then writes the byte that lets the program poll.
+(import agent "vendor:agent")
+
+;;; libc's, declared straight: no aggregate crosses the boundary, so there is
+;;; nothing for a shim to do.
+(declare stdin-byte [] i32 "getchar")
+
+(defvar ticks i64)
+
+(defn tick [] i64
+  (set ticks (+ ticks 1))
+  ticks)
+
+(defn main [args [string]] i32
+  (if (< (len args) 2)
+    (do (println "usage: agent-queue <socket>") 2)
+    (do
+      (if (< (agent/start (at args 1)) 0)
+        (do (println "cannot listen") 1)
+        (do
+          (println "ready")
+          (stdin-byte)
+          ;; How many the ring actually held. Every module queued is installed
+          ;; here, so this number is the count of slots that survived — which
+          ;; is the whole claim: a ring that overwrote the slot it was reading
+          ;; would answer with something else.
+          (print (agent/poll)) (println "")
+          0)))))

test/test_agent.ml

@@ -285,8 +285,108 @@ let () =
         fail "break loop transcript\n        got: %S\n     wanted: %S" got want
     end;
 
+    (* ── The job ring, and what a full one does ─────────────────────── *)
+
+    (* Two claims, one program. A module the agent refuses because it carries
+       no installer is *closed* rather than leaked; and a ring with no room
+       refuses the module instead of overwriting the slot the game thread is
+       reading.
+
+       The program blocks on stdin until the test has finished filling the
+       ring, so neither claim is a race against how fast sixty-five
+       connections are served. *)
+    let qsock = tmp "queue.sock" and qout = tmp "queue.out" in
+    (try Sys.remove qsock with Sys_error _ -> ());
+    let qt, ql = Session.create ~file:"programs/agent-queue.flan" () in
+    let qexe = tmp "queue" in
+    ignore
+      (Build.executable ~opts:dev ~csrcs:ql.Load.csrcs ~lflags:ql.Load.lflags
+         qt.Session.host ~out:qexe);
+    (* One module, sent many times. dlopen keys on the path, so this is the
+       same relocation over and over — what is being counted is publishes, and
+       building sixty-five of them would measure llc instead. *)
+    let qso = tmp "queue-tick.so" in
+    let qc = Session.eval qt "(defn tick [] i64 (set ticks (+ ticks 1)) ticks)" in
+    ignore (Build.shared ~opts:dev ~ir:qc.Session.ir ~out:qso ());
+    let noinstall = tmp "noinstall.so" in
+    let cc =
+      Printf.sprintf "clang -shared -fPIC -o %s noinstall.c 2>/dev/null"
+        (Filename.quote noinstall)
+    in
+    if Sys.command cc <> 0 then fail "could not build noinstall.so"
+    else begin
+      let rfd, wfd = Unix.pipe () in
+      let qfd =
+        Unix.openfile qout [ Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC ] 0o600
+      in
+      let qpid = Unix.create_process qexe [| qexe; qsock |] rfd qfd qfd in
+      Unix.close qfd;
+      Unix.close rfd;
+      let qtext () = In_channel.with_open_bin qout In_channel.input_all in
+      let has needle =
+        let t = qtext () in
+        List.exists (String.equal needle) (String.split_on_char '\n' t)
+      in
+      if not (await (fun () -> Sys.file_exists qsock && has "ready")) then begin
+        fail "the queue program never bound its socket";
+        (try Unix.kill qpid Sys.sigkill with Unix.Unix_error _ -> ())
+      end
+      else begin
+        (* A module with no installer. The refusal was always there; what is
+           new is that the handle is closed, and the destructor saying so
+           *while the program is still running* is the only way to see it —
+           at exit the loader would run it either way. *)
+        let r = send qsock noinstall in
+        if r <> "err no flan_reload_install\n" then
+          fail "a module with no installer: %S" r;
+        if not (await (fun () -> has "unloaded")) then
+          fail "the refused module was not closed: %S" (qtext ());
+
+        (* QUEUE slots, then one more. The one more is refused, at the sender,
+           with a reason — the old code took it, wrote it over slot 0, and
+           said ok. *)
+        let queue_size = 64 in
+        let bad = ref "" in
+        for _ = 1 to queue_size do
+          let r = send qsock qso in
+          if r <> "ok\n" && !bad = "" then bad := r
+        done;
+        if !bad <> "" then fail "a module that fitted was refused: %S" !bad;
+        let full = send qsock qso in
+        if full <> "err reload queue full; the program is not calling agent/poll\n"
+        then fail "a full ring did not refuse: %S" full;
+
+        (* Let it poll. Every slot the ring kept is installed here, so the
+           number is how many survived — 64, not 65 and not some torn count. *)
+        ignore (Unix.write wfd (Bytes.of_string "\n") 0 1);
+        Unix.close wfd;
+        let qstatus = ref (Unix.WEXITED 0) in
+        let reaped =
+          await ~ms:5000 (fun () ->
+            match Unix.waitpid [ Unix.WNOHANG ] qpid with
+            | 0, _ -> false
+            | _, s -> qstatus := s; true)
+        in
+        if not reaped then begin
+          (try Unix.kill qpid Sys.sigkill with Unix.Unix_error _ -> ());
+          fail "the queue program never finished"
+        end
+        else begin
+          let got =
+            String.concat "\n"
+              (List.filter (fun l -> l <> "")
+                 (String.split_on_char '\n' (qtext ())))
+          in
+          if !qstatus <> Unix.WEXITED 0 || got <> "ready\nunloaded\n64" then
+            fail "job ring\n        got: %S\n     wanted: %S" got
+              "ready\nunloaded\n64"
+        end
+      end
+    end;
+
     List.iter (fun f -> try Sys.remove f with Sys_error _ -> ())
-      [ exe; so1; so2; sock; out; bsock; bout; bexe ];
+      [ exe; so1; so2; sock; out; bsock; bout; bexe; qexe; qso; qsock; qout;
+        noinstall ];
     if !failures = 0 then print_endline "agent: all tests passed"
     else begin
       Printf.printf "\n%d failure(s)\n" !failures;

vendor/agent/flan_agent.c

@@ -17,8 +17,11 @@
  * that is when the swap becomes visible. Nothing else in the program needs to
  * know the agent exists.
  *
- * Nothing is ever dlclosed: a cell holds an address inside a module's text,
- * and unloading it would leave every call site pointing at unmapped memory.
+ * Nothing that published anything is ever dlclosed: a cell holds an address
+ * inside a module's text, and unloading it would leave every call site
+ * pointing at unmapped memory. The two modules that are closed are the ones
+ * nothing can point into — a transient thunk, which installs no bodies, and a
+ * module refused before it was queued.
  *
  * This is not a protocol. One line per request, the path to load, and a one
  * line answer. The daemon and its nREPL are a separate program that will speak
@@ -57,8 +60,27 @@ int flan_dev_result_read(char *dst, uint64_t cap, uint64_t *gen, uint64_t *len);
 
 /* A ring the listener writes and the game thread reads. One producer, one
  * consumer, so two atomics and no lock — the game thread must never block on
- * the loader. Overflow drops the oldest request rather than stalling; a dev
- * loop that queues 64 reloads between two frames has a bigger problem. */
+ * the loader.
+ *
+ * A full ring is *refused*, at the sender, with an error. The previous comment
+ * here claimed it dropped the oldest request, and nothing did that: [publish]
+ * never read [tail], so the 65th module overwrote the slot the game thread was
+ * reading — twenty-four bytes of function pointers, copied field by field with
+ * no atomic anywhere near them, so the consumer could take half of one job and
+ * half of another and call it. Silent corruption of the one thing in this file
+ * that gets *called*.
+ *
+ * Of the three honest answers, refusing is the only one that reaches the
+ * person who asked. Dropping loses a reload the sender was told was ok — the
+ * same lie in quieter clothes. Blocking stalls the accept loop, which serves
+ * connections inline, so a program that has stopped polling would also stop
+ * answering [status] and [abort]: the dev loop would have no way to say
+ * anything to a program that had stopped listening to it. A refusal is a line
+ * the daemon can show, and the fix is to call agent/poll.
+ *
+ * The check is safe to make separately from the store because there is exactly
+ * one producer — this thread — so room, once seen, cannot be taken away by
+ * anyone: the consumer only ever makes more of it. */
 #define QUEUE 64
 /* [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:
@@ -74,12 +96,26 @@ static int listen_fd = -1;
 static pthread_t listener;
 static atomic_int started;
 
-static void publish(job j) {
+/* Room for one more. Unsigned subtraction, so the answer survives [head] and
+ * [tail] wrapping; only their difference means anything. */
+static int queue_room(void) {
   unsigned h = atomic_load_explicit(&head, memory_order_relaxed);
+  unsigned t = atomic_load_explicit(&tail, memory_order_acquire);
+  return (h - t) < QUEUE;
+}
+
+/* 0 if the ring is full, having published nothing. Checked here as well as at
+ * the caller, because a producer that forgot would otherwise reintroduce
+ * exactly the overwrite this replaced. */
+static int publish(job j) {
+  unsigned h = atomic_load_explicit(&head, memory_order_relaxed);
+  unsigned t = atomic_load_explicit(&tail, memory_order_acquire);
+  if (h - t >= QUEUE) return 0;
   queue[h % QUEUE] = j;
   /* Release: the store to the slot must be visible before the index that
    * advertises it. */
   atomic_store_explicit(&head, h + 1, memory_order_release);
+  return 1;
 }
 
 /* Returns how many modules were installed. Call it between frames. */
@@ -246,6 +282,22 @@ static char        condition_name[128];
 
 int32_t flan_agent_poll(void);
 
+/* Every way out of the break loop that is not a resume. [_exit] and not
+ * [exit], because this runs on the game thread while the listener thread may
+ * be inside [dlopen] holding the loader lock — and [exit] runs the atexit
+ * chain and the ELF destructors, which want that same lock. A program asked to
+ * abort would hang instead of dying, which is the failure mode the break loop
+ * exists to replace. Nothing here needs an orderly teardown: the streams are
+ * flushed by hand above every call.
+ *
+ * 134 is kept because that is what a trap exits with; see rt_die in
+ * flan_rt.c. */
+static _Noreturn void die_now(void) {
+  fflush(stdout);
+  fflush(stderr);
+  _exit(134);
+}
+
 static void break_loop(const uint8_t *name, int64_t namelen, void *condition,
                        void *xfer) {
   struct timespec step = { 0, 2000000 };   /* 2ms */
@@ -263,7 +315,7 @@ static void break_loop(const uint8_t *name, int64_t namelen, void *condition,
     fprintf(stderr, "flan: %d nested break loops - giving up rather than "
                     "spinning\n", BREAK_MAX);
     fflush(stderr);
-    exit(134);
+    die_now();
   }
   {
     snapshot *s = snap_top();
@@ -302,14 +354,14 @@ static void break_loop(const uint8_t *name, int64_t namelen, void *condition,
             "flan: %d nested break loops — giving up rather than spinning\n",
             BREAK_MAX);
     fflush(stderr);
-    exit(134);
+    die_now();
   }
   for (;;) {
     flan_agent_poll();
     if (atomic_load(&aborting)) {
       fflush(stdout);
       fprintf(stderr, "flan: aborted at the break loop\n");
-      exit(134);
+      die_now();
     }
     if (atomic_load(&chosen_ready)) {
       /* Claimed into a local and the flag cleared *before* the attempt. The
@@ -585,6 +637,15 @@ static void serve(int fd) {
       }
       return;
     }
+    /* 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 this thread produces, so room seen now is room
+     * still there at [publish] below. */
+    if (!queue_room()) {
+      reply(fd, "err reload queue full; the program is not calling "
+                "agent/poll\n");
+      return;
+    }
     void *h = dlopen(line, RTLD_NOW | RTLD_LOCAL);
     if (h == NULL) {
       reply(fd, "err ");
@@ -593,7 +654,19 @@ static void serve(int fd) {
       return;
     }
     install_fn f = (install_fn)(uintptr_t)dlsym(h, "flan_reload_install");
-    if (f == NULL) { reply(fd, "err no flan_reload_install\n"); return; }
+    if (f == NULL) {
+      /* Closed, and this is not an exception to "nothing is ever dlclosed".
+       * That rule is about a module something *points into* — a cell holding
+       * an address in its text. This one published nothing: no installer ran,
+       * so no cell names it, and it is unreachable the moment this function
+       * returns. What leaked before was the handle value rather than the
+       * mapping — dlopen refcounts by path, so re-sending the same bad file
+       * bumped a count nothing could ever bring down, and the one reference
+       * that could was dropped on the floor here. */
+      dlclose(h);
+      reply(fd, "err no flan_reload_install\n");
+      return;
+    }
     /* Optional: only an expression evaluation has one. */
     call_fn c = (call_fn)(uintptr_t)dlsym(h, "flan_reload_call");
     /* And only one that leaves nothing behind may be unloaded. */
@@ -605,8 +678,12 @@ static void serve(int fd) {
      * sender as a reset, not as an answer. */
     reply(fd, "ok\n");
     /* "queued", not "installed": the store happens on the game thread, at a
-     * time this thread does not get to choose. */
-    publish((job){ f, c, transient == NULL ? NULL : h });
+     * time this thread does not get to choose. The room was checked before the
+     * 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 }))
+      fprintf(stderr, "flan: reload queue full after it was checked\n");
     return;
   }
 }