Four runtime defects, and the two buffers that now have evidence
This commit is contained in:
commit
e2bafec373
42
BUILT.md
42
BUILT.md
@ -384,7 +384,7 @@ slice parameter.
|
||||
**`flan_dev.c` is compiled into every build, not only a dev one.** Nothing in a release build calls into it — the
|
||||
compiler only emits a registry lookup for a name the host was not built with, which cannot arise without cells — but the
|
||||
agent package's C refers to it, and a package's C sources are collected whatever `main` does. Leaving it out of release
|
||||
builds made `flan build sand.flan` fail at the link with an undefined `flan_dev_result_get`, which reads as a compiler
|
||||
builds made `flan build sand.flan` fail at the link with an undefined `flan_dev_result_read`, which reads as a compiler
|
||||
bug rather than as a missing flag. The table is BSS, so the cost is address space and not binary size; `-rdynamic` and
|
||||
the cells are still what `--dev` means. `test_agent.ml` links the agent program both ways for this reason.
|
||||
|
||||
@ -469,8 +469,13 @@ still null. Not race-testable, so it is asserted on the emitted `flan_reload_ins
|
||||
- **`flan_dev_global` refuses a size change.** The running process has already laid that memory out; handing back the
|
||||
old allocation for a differently shaped type means the new body reads fields at the wrong offsets and nothing says so.
|
||||
This is the layout-drift rule's first enforcement point. Retyping a var needs a restart.
|
||||
- **Nothing is ever `dlclose`d.** A cell holds an address inside a module's text; unloading it leaves every call site
|
||||
pointing at unmapped memory. That is a constraint on the agent too.
|
||||
- **Nothing that published anything is ever `dlclose`d.** A cell holds an address inside a module's text; unloading it
|
||||
leaves every call site pointing at unmapped memory. That is a constraint on the agent too. The rule is about being
|
||||
*pointed into*, which is why it has exactly two exceptions and they are not exceptions to the reasoning: a transient
|
||||
thunk, which takes no registry slot and has returned; and a module the agent refuses before queueing it — no installer,
|
||||
or no room in the ring — which published nothing and which nothing can name. What was leaked in the second case 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, and the one reference that could was dropped on the floor.
|
||||
- **The registry never moves.** A module holds a cell's address for as long as it is loaded, so the table is fixed
|
||||
capacity with a loud failure rather than growable.
|
||||
|
||||
@ -499,6 +504,19 @@ redefined function is on the stack, so it happens on the game thread, at the top
|
||||
The two are connected by a single-producer/single-consumer ring and two atomics; the game thread never blocks on the
|
||||
loader.
|
||||
|
||||
**A full ring is refused, at the sender, before the `dlopen`.** Of the three honest answers this is the only one that
|
||||
reaches the person who asked: dropping loses a reload the sender was told was `ok`, which is the same lie more quietly,
|
||||
and blocking stalls the accept loop — it serves connections inline, so a program that had stopped polling would also
|
||||
stop answering `status` and `abort`, leaving the dev loop with no way to reach a program that had stopped listening to
|
||||
it. The check is separate from the store because there is exactly one producer: room, once seen, cannot be taken away,
|
||||
since the consumer only ever makes more of it. Sixty-four is a lot of reloads between two frames and the refusal says
|
||||
what to do about it — call `agent/poll`.
|
||||
|
||||
**The way out of the break loop is `_exit`, not `exit`.** `exit` 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, which is the failure the break loop exists to replace. The streams are flushed by hand at each call site, and 134
|
||||
stays because that is what a trap exits with.
|
||||
|
||||
`wait` exists for tests. A test that races the frame rate fails on a loaded machine, so `test/programs/agent.flan` waits
|
||||
for the reload instead of sleeping past it. It takes **two** reloads, which is the daemon's actual loop: the first
|
||||
introduces a global the process was never built with, the second only reads it, and the second can only answer 1007 if
|
||||
@ -765,8 +783,22 @@ decision's bill, and it is why the printer set is small rather than universal.
|
||||
|
||||
It does not go through stdout. Stdout belongs to the program, it is in the hot path for anything that prints, and a
|
||||
dev-only feature must not put a branch in it — so `flan_rt.c` is untouched and the value is read back over the agent's
|
||||
socket. The read is safe without a handshake because `flan_dev_result` bumps a generation counter last; the daemon waits
|
||||
for it to move rather than assuming the program has reached a frame boundary.
|
||||
socket. The read is safe without a handshake because the counter is a **seqlock**, and it had to be made into a real
|
||||
one: the first version bumped the generation last and handed back the buffer itself, which says a new value has arrived
|
||||
and says nothing about whether the bytes the agent then wrote to a socket were that value — the game thread is free to
|
||||
be a hundred bytes into the next one by then. A seqlock cannot validate a read that finishes after it returns, so the
|
||||
bare pointer was the bug rather than the ordering. `flan_dev_result_read` copies into the caller's buffer and checks
|
||||
the counter either side of the copy; the counter is odd for exactly as long as a value is being written, and a reader
|
||||
that loses the race reports the last *complete* generation and no bytes, so a daemon polling for a new value keeps
|
||||
polling rather than being shown half of one. The count handed out is the number of complete values, so the daemon's
|
||||
"has it moved" still means what it meant. Marking the counter odd needs a release *fence* and not a release store — a
|
||||
release store orders what precedes it, so the writes to the buffer would be free to become visible ahead of it, which
|
||||
is the original bug with more ceremony. And the odd mark is *set* rather than incremented, because a `begin` with no
|
||||
`end` is reachable: the thunk calls `begin` before it evaluates anything, so an expression that signals is stopped
|
||||
inside that window, and a restart taken from the break transfers past the thunk and `end` never runs. Incrementing
|
||||
would leave the counter odd for the life of the process and every later read reporting "in progress"; setting the bit
|
||||
means the next evaluation repairs it. The daemon waits for the count to move rather than assuming the program has
|
||||
reached a frame boundary.
|
||||
|
||||
**This renderer is most of `println`**, which is worth knowing before anyone schedules it. plan.org describes a
|
||||
compiler-provided, type-directed intrinsic that selects or emits a structural printer per concrete instantiation, prints
|
||||
|
||||
49
NEXT.md
49
NEXT.md
@ -89,11 +89,13 @@ it *would* have written.
|
||||
which is a compiler feature of the same shape the bounds checks already have, or they belong to the checker. Not
|
||||
decided. `test_sanitize` pins the current answer with a control that must *not* report, so a future clang changing
|
||||
this is a test failure rather than a discovery.
|
||||
2. **Four named buffers got no evidence at all.** The 4K result cap, the dev registry overflow guard,
|
||||
`SNAP_MAX`/`SNAP_NAMES` and `condition_name[128]` are on the daemon and agent paths, which need a socket and are not
|
||||
in the corpus. Their guards were read and are correct; that is reading, not testing. `escaped[ESCAPE_MAX]` is the one
|
||||
that *is* covered, because `println.flan` drives a 1100-character string through it on purpose — 1019 bytes out
|
||||
against a worst case of 1021 into 1024. `scratch[SCRATCH]` never sees more than 20 characters of 64.
|
||||
2. **Two of the four named buffers now have evidence; two still do not.** The 4K result cap and `condition_name[128]`
|
||||
are driven over the agent's socket from `test_agent.ml` — a 5000-byte value comes back as 4096 ending in the
|
||||
ellipsis, a 198-character condition class comes back from `status` as 127. The **dev registry overflow guard** and
|
||||
`SNAP_MAX`/`SNAP_NAMES` are still read rather than tested: four thousand interned names and sixty-five nested
|
||||
`restart-case`s are a lot of program for a clamp each. `escaped[ESCAPE_MAX]` is covered because `println.flan`
|
||||
drives a 1100-character string through it on purpose — 1019 bytes out against a worst case of 1021 into 1024.
|
||||
`scratch[SCRATCH]` never sees more than 20 characters of 64.
|
||||
3. **Valgrind over the headless corpus, not done.** ASan does not see uninitialised reads, which is where `zeroed` and
|
||||
struct padding live. MSan is out: it needs every dependency instrumented and raylib settles that.
|
||||
|
||||
@ -538,7 +540,7 @@ expander last, on 6's unions.
|
||||
no evidence of what it meant. The agent copies the list on entering `break_loop` — names into its own buffer, frames
|
||||
as the addresses a transfer carries — one snapshot per nested break, and every verb answers from it. Caps are
|
||||
`SNAP_MAX` 64 restarts and `SNAP_NAMES` 4096 bytes; past either, the listing says how many it did not show. Neither
|
||||
cap has a test, same blind spot as the 4K result cap below.
|
||||
cap has a test; the 4K result cap that shared that blind spot now does.
|
||||
|
||||
- **A snapshot generation has no test, and the window is a race.** A choice is validated against the snapshot on top
|
||||
when the request arrives and resolved against the snapshot on top when the game thread next looks. Between those,
|
||||
@ -550,13 +552,31 @@ expander last, on 6's unions.
|
||||
window means landing a request inside a two-millisecond poll from outside the process. It wants a hook the test can
|
||||
drive, not a sleep.
|
||||
|
||||
- **The job ring has no fullness check**, and the comment describing its overflow is wrong. `publish` never consults
|
||||
`tail`; past `QUEUE` entries it overwrites the slot the consumer is reading, and `job` is 24 non-atomic bytes.
|
||||
Reachable from a program that goes a long time between `agent/poll` calls.
|
||||
- **`flan_dev_result_get` is not the seqlock its comment claims** — it reads the generation first, then a non-atomic
|
||||
length, then returns a bare pointer the caller sends later.
|
||||
- Smaller: `exit(134)` from the break loop with the listener inside `dlopen`; a `dlopen` handle leaked when a module
|
||||
has no installer.
|
||||
- ~~The job ring has no fullness check.~~ **Fixed by refusing, at the sender.** Dropping loses a reload the sender was
|
||||
told was ok; blocking stalls the accept loop, which serves connections inline, so a program that had stopped polling
|
||||
would also stop answering `status` and `abort`. The refusal happens before the `dlopen`, so a module there is no room
|
||||
for is never relocated and no handle is taken for it. `programs/agent-queue.flan` blocks on stdin so the window is
|
||||
held open by the test rather than by a timer: 64 queued, the 65th refused with a reason, 64 installed when it finally
|
||||
polls.
|
||||
|
||||
- ~~`flan_dev_result_get` is not the seqlock its comment claims.~~ **Fixed by making it one**, rather than by writing
|
||||
the honest comment — what it guaranteed was nothing, and the daemon has no other way to read a result. The counter is
|
||||
odd while a value is being written, `flan_dev_result_read` copies into the caller's buffer and checks the counter
|
||||
either side of the copy, and a reader that loses the race reports the last complete generation and no bytes. The
|
||||
count handed out is the number of complete values, so `lib/dev.ml`'s "has it moved" still means what it meant. The
|
||||
race itself has no test, for the same reason the snapshot generation above has none.
|
||||
|
||||
- ~~Smaller: `exit(134)` from the break loop with the listener inside `dlopen`; a `dlopen` handle leaked when a module
|
||||
has no installer.~~ **Both fixed.** `exit` runs the atexit chain and the ELF destructors, which want the loader lock
|
||||
the listener may be holding — a program asked to abort would hang instead of dying; `_exit`, with the streams flushed
|
||||
by hand. The leak was the handle *value* and not the mapping: a module with no installer published nothing, so
|
||||
nothing can point into it, and it is closed. The deadlock is read rather than tested; the exit status is tested.
|
||||
- **`rt_die` in `flan_rt.c` still calls `exit(134)`**, which is the shape just fixed in the break loop: a trap on the
|
||||
game thread runs the atexit chain and the ELF destructors, which want the loader lock the agent's listener thread may
|
||||
be holding inside `dlopen`, so a program that should die could hang. Found while fixing the break loop and not fixed
|
||||
with it — `rt_die` is the non-dev path too, where there is no listener and nothing to deadlock against, so whether it
|
||||
should be `_exit` unconditionally or only under `--dev` is a decision rather than a typo.
|
||||
|
||||
- **`(A {:x 1})` on a union variant says "unknown struct A"** rather than the union refusal `check_struct` plainly
|
||||
intends — `env` has no table of variant names. A diagnostics bug, not a backend death.
|
||||
|
||||
@ -569,7 +589,8 @@ Sixty mutations, nineteen left the whole suite green. The severe cluster is clos
|
||||
function a valid program calls, so the build fails to link.
|
||||
- `flan_dev_global`'s size-change guard — the layout-drift check, with no test that retypes a global across a reload.
|
||||
- A local shadowing an imported name is qualified anyway.
|
||||
- The 4K result cap and the registry overflow guard have **no coverage at all**, rather than a missing assertion.
|
||||
- The registry overflow guard has **no coverage at all**, rather than a missing assertion. The 4K result cap that used
|
||||
to sit beside it here is driven over the agent's socket now.
|
||||
- The reader accepts an unknown string escape; `+5` stops being a number.
|
||||
- And a warning: a reader mutation makes the suite **hang** rather than fail. A green run is not the only outcome to
|
||||
plan for in CI.
|
||||
|
||||
@ -123,7 +123,23 @@ void *flan_dev_global(const char *name, uint64_t size, const void *init) {
|
||||
*
|
||||
* [generation] is what makes the read safe without a handshake. The thunk runs
|
||||
* on the game thread at a frame boundary, whenever that happens to be; the
|
||||
* daemon waits for the counter to move rather than guessing it has. */
|
||||
* daemon waits for the counter to move rather than guessing it has.
|
||||
*
|
||||
* It is a *seqlock*, and it has to be a real one, because the reader is the
|
||||
* agent's listener thread and the writer is the game thread and neither waits
|
||||
* for the other. The counter is odd for exactly as long as a value is being
|
||||
* written, so a reader that sees an odd count, or a different count either
|
||||
* side of its copy, has read a value that was being overwritten underneath it
|
||||
* and reads again. A count of 2k means k complete values; the count the
|
||||
* outside world is given is that k, so that the daemon's "has it moved" keeps
|
||||
* meaning "is there a new value".
|
||||
*
|
||||
* The copy is what makes it safe, and the API is shaped around that: a reader
|
||||
* gets *bytes of its own*, not a pointer into [result]. The pointer version of
|
||||
* this was the bug — it read the generation, then a length, then handed back
|
||||
* the buffer itself, and the caller sent it down a socket some time later
|
||||
* while the game thread was free to be a hundred bytes into the next value.
|
||||
* A seqlock cannot validate a read that happens after it returns. */
|
||||
|
||||
#define RESULT_MAX 4096
|
||||
static char result[RESULT_MAX];
|
||||
@ -132,6 +148,30 @@ static int result_full;
|
||||
static uint64_t generation;
|
||||
|
||||
void flan_dev_result_begin(void) {
|
||||
/* Odd first, and only then the reset: the counter has to say "in progress"
|
||||
* before the buffer stops being the value it used to be.
|
||||
*
|
||||
* The fence is the half of that a release *store* cannot do. A release store
|
||||
* orders what comes before it, not what comes after, so the writes below —
|
||||
* and every memcpy in [emit] — would be free to become visible ahead of the
|
||||
* odd count, and a reader could see an even count either side of a copy it
|
||||
* made while the buffer was being overwritten. Which is the bug this
|
||||
* replaced, with more ceremony. So: mark it relaxed, fence, then write.
|
||||
*
|
||||
* Setting the low bit rather than incrementing, because a [begin] with no
|
||||
* [end] is reachable and must not poison the counter for the life of the
|
||||
* process. A render thunk that signals is stopped inside this window, and a
|
||||
* restart taken from that break transfers past the thunk — [end] never runs.
|
||||
* Repairing it here costs nothing in the ordinary case (2k, 2k+1, 2k+2) and
|
||||
* means an abandoned write is over as soon as the next evaluation starts,
|
||||
* rather than leaving every later read reporting "in progress" forever.
|
||||
*
|
||||
* What it does not fix, because the buffer cannot: an evaluation that runs
|
||||
* while another is stopped mid-render shares this one buffer, so the inner
|
||||
* value is the one that survives and the outer thunk, if it is ever resumed,
|
||||
* appends to it. That was true before the counter was a seqlock. */
|
||||
__atomic_store_n(&generation, generation | 1, __ATOMIC_RELAXED);
|
||||
__atomic_thread_fence(__ATOMIC_RELEASE);
|
||||
result_len = 0;
|
||||
result_full = 0;
|
||||
}
|
||||
@ -208,12 +248,52 @@ void flan_dev_result_end(void) {
|
||||
memcpy(result + result_len, ell, k);
|
||||
result_len += k;
|
||||
}
|
||||
/* Last, so a reader that sees the new generation sees the whole value. */
|
||||
__atomic_store_n(&generation, generation + 1, __ATOMIC_RELEASE);
|
||||
/* Last, and back to even, so a reader that sees the new generation sees the
|
||||
* whole value. [| 1] first for the same reason [begin] sets rather than
|
||||
* increments: this must land on an even count whatever state an abandoned
|
||||
* write left behind. */
|
||||
__atomic_store_n(&generation, (generation | 1) + 1, __ATOMIC_RELEASE);
|
||||
}
|
||||
|
||||
const char *flan_dev_result_get(uint64_t *gen, uint64_t *len) {
|
||||
*gen = __atomic_load_n(&generation, __ATOMIC_ACQUIRE);
|
||||
*len = (uint64_t)result_len;
|
||||
return result;
|
||||
/* Copy the current value out, with the counter that says which one it is.
|
||||
*
|
||||
* Returns 1 having copied a value that was complete for the whole of the copy,
|
||||
* 0 if the game thread was in the middle of writing one — in which case [gen]
|
||||
* is the last *complete* value's number and [len] is 0, so a caller polling
|
||||
* for a new one keeps polling instead of being handed half of it. Spinning
|
||||
* here is bounded: the writer is a render thunk between frames, not a loop,
|
||||
* and the reader is the listener thread, which has nothing better to do.
|
||||
*
|
||||
* [cap] is the caller's buffer. A value longer than it is truncated, which is
|
||||
* the only failure this can have and is a clamp rather than an overrun; the
|
||||
* agent sizes its buffer at RESULT_MAX so it does not arise. */
|
||||
/* What a caller's buffer has to be for the copy never to be truncated. The
|
||||
* bound is declared in one place and asked for rather than written down twice:
|
||||
* the agent's buffer and this one agreeing is the whole of "never truncated",
|
||||
* and two literals in two files is how that stops being true. */
|
||||
uint64_t flan_dev_result_cap(void) { return RESULT_MAX; }
|
||||
|
||||
int flan_dev_result_read(char *dst, uint64_t cap, uint64_t *gen,
|
||||
uint64_t *len) {
|
||||
for (int attempt = 0; attempt < 64; attempt++) {
|
||||
uint64_t g1 = __atomic_load_n(&generation, __ATOMIC_ACQUIRE);
|
||||
if (g1 & 1) continue; /* a write is in progress */
|
||||
size_t n = __atomic_load_n(&result_len, __ATOMIC_RELAXED);
|
||||
if (n > RESULT_MAX) n = RESULT_MAX; /* a torn read cannot overrun */
|
||||
if ((uint64_t)n > cap) n = (size_t)cap;
|
||||
memcpy(dst, result, n);
|
||||
/* The copy must be ordered before the second read of the counter, or the
|
||||
* check is of a copy the compiler was free to make afterwards. */
|
||||
__atomic_thread_fence(__ATOMIC_ACQUIRE);
|
||||
if (__atomic_load_n(&generation, __ATOMIC_ACQUIRE) == g1) {
|
||||
*gen = g1 / 2;
|
||||
*len = (uint64_t)n;
|
||||
return 1;
|
||||
}
|
||||
}
|
||||
/* Integer division is the same answer either side of a write in progress:
|
||||
* during value k the counter is 2k-1 and k-1 are complete. */
|
||||
*gen = __atomic_load_n(&generation, __ATOMIC_ACQUIRE) / 2;
|
||||
*len = 0;
|
||||
return 0;
|
||||
}
|
||||
|
||||
@ -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.
|
||||
|
||||
16
test/noinstall.c
Normal file
16
test/noinstall.c
Normal file
@ -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);
|
||||
}
|
||||
15
test/programs/agent-longname.flan
Normal file
15
test/programs/agent-longname.flan
Normal file
@ -0,0 +1,15 @@
|
||||
;;;; The agent's condition_name[128], which had no coverage at all: it is on
|
||||
;;;; the socket path, so nothing in the sanitizer corpus can reach it.
|
||||
;;;;
|
||||
;;;; The condition class here is 200 characters, so the copy into that buffer
|
||||
;;;; has to clamp, and [status] answers with the 127 that fit. The name is ugly
|
||||
;;;; on purpose — its length is the whole of the test.
|
||||
(import agent "vendor:agent")
|
||||
|
||||
(defstruct MissingYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY [id i32])
|
||||
|
||||
(defn main [] i32
|
||||
;; The path is overridden by FLAN_AGENT_SOCKET; a program has to name one.
|
||||
(agent/start "/tmp/flan-longname.sock")
|
||||
(error (MissingYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY {:id 1}))
|
||||
0)
|
||||
43
test/programs/agent-queue.flan
Normal file
43
test/programs/agent-queue.flan
Normal file
@ -0,0 +1,43 @@
|
||||
;;;; 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 "")
|
||||
;; Round two: one evaluated expression, whose rendering is longer
|
||||
;; than the 4K the dev runtime will hold. The program has to still be
|
||||
;; here afterwards for the value to be read back off the socket, so
|
||||
;; it waits a third time and leaves on end of file.
|
||||
(stdin-byte)
|
||||
(print (agent/poll)) (println "")
|
||||
(stdin-byte)
|
||||
0)))))
|
||||
@ -285,8 +285,218 @@ 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
|
||||
(* Close-on-exec, or the child inherits the write end and its own stdin
|
||||
never reaches end of file: the program would sit in its last read
|
||||
waiting for a byte only it could send. The read end is dup'd onto fd 0
|
||||
by [create_process], which clears the flag on the copy. *)
|
||||
let rfd, wfd = Unix.pipe ~cloexec:true () 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);
|
||||
if not (await (fun () -> has "64")) then
|
||||
fail "the ring never drained: %S" (qtext ())
|
||||
else begin
|
||||
(* ── The 4K result cap ───────────────────────────────────────
|
||||
|
||||
NEXT.md names this buffer twice as having no coverage at all,
|
||||
because it is on the agent's path and so needs a socket. There is
|
||||
a socket here. The value is a 5000-byte string literal, which is
|
||||
longer than anything [emit] will keep, so what comes back is the
|
||||
clamp and the ellipsis [result_end] puts there to say it clamped
|
||||
— and it comes back through the seqlock's copy rather than off a
|
||||
borrowed pointer. *)
|
||||
let long = String.make 5000 'x' in
|
||||
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 ());
|
||||
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);
|
||||
if not (await (fun () -> has "1")) then
|
||||
fail "the eval thunk never ran: %S" (qtext ())
|
||||
else begin
|
||||
let reply = send qsock "result" in
|
||||
match String.index_opt reply '\n' with
|
||||
| None -> fail "no result header: %S" reply
|
||||
| Some i ->
|
||||
let hdr = String.sub reply 0 i in
|
||||
let body =
|
||||
String.sub reply (i + 1) (String.length reply - i - 1)
|
||||
in
|
||||
(* Exactly the cap, ellipsis included: [result_end] makes room
|
||||
for it rather than assuming there is any. *)
|
||||
if String.length body <> 4096 then
|
||||
fail "the 4K result cap: %d bytes back, header %S"
|
||||
(String.length body) hdr;
|
||||
let tail n = if String.length body < n then body
|
||||
else String.sub body (String.length body - n) n in
|
||||
let head n = if String.length body < n then body
|
||||
else String.sub body 0 n in
|
||||
if tail 3 <> "..." then
|
||||
fail "a clamped result did not say so: %S" (tail 8);
|
||||
if head 2 <> "\"x" then
|
||||
fail "the result is not the value that was rendered: %S"
|
||||
(head 8)
|
||||
end;
|
||||
(try Sys.remove eso with Sys_error _ -> ())
|
||||
end;
|
||||
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\n1" then
|
||||
fail "job ring\n got: %S\n wanted: %S" got
|
||||
"ready\nunloaded\n64\n1"
|
||||
end
|
||||
end
|
||||
end;
|
||||
|
||||
(* ── condition_name[128] ────────────────────────────────────────── *)
|
||||
|
||||
(* The other named buffer with no coverage at all, and it needs a socket
|
||||
for the same reason: nothing but [status] ever reads it. The condition
|
||||
class is 198 characters, so what comes back is the 127 that fit and a
|
||||
terminator — a clamp, not an overrun, and now measured rather than
|
||||
read. Aborting out of it also pins that the break loop's way out is
|
||||
still exit status 134 now that it takes it with _exit. *)
|
||||
let lsock = tmp "long.sock" and lout = tmp "long.out" in
|
||||
(try Sys.remove lsock with Sys_error _ -> ());
|
||||
let lt, ll = Session.create ~file:"programs/agent-longname.flan" () in
|
||||
let lexe = tmp "long" in
|
||||
ignore
|
||||
(Build.executable ~opts:dev ~csrcs:ll.Load.csrcs ~lflags:ll.Load.lflags
|
||||
lt.Session.host ~out:lexe);
|
||||
let lfd = Unix.openfile lout [ Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC ] 0o600 in
|
||||
let lenv =
|
||||
Array.append (Unix.environment ()) [| "FLAN_AGENT_SOCKET=" ^ lsock |]
|
||||
in
|
||||
let lpid = Unix.create_process_env lexe [| lexe |] lenv Unix.stdin lfd lfd in
|
||||
Unix.close lfd;
|
||||
let stopped = ref "" in
|
||||
if not
|
||||
(await (fun () ->
|
||||
Sys.file_exists lsock
|
||||
&& (stopped := send lsock "status";
|
||||
String.length !stopped > 8
|
||||
&& String.sub !stopped 0 8 = "stopped ")))
|
||||
then begin
|
||||
fail "the long-named condition never stopped: %S" !stopped;
|
||||
(try Unix.kill lpid Sys.sigkill with Unix.Unix_error _ -> ())
|
||||
end
|
||||
else begin
|
||||
let got = String.trim (String.sub !stopped 8 (String.length !stopped - 8)) in
|
||||
if String.length got <> 127 then
|
||||
fail "condition_name clamps at 127: got %d characters"
|
||||
(String.length got);
|
||||
if String.length got >= 7 && String.sub got 0 7 <> "Missing" then
|
||||
fail "the clamped name is not the condition's: %S" got;
|
||||
ignore (send lsock "abort");
|
||||
let lstatus = ref (Unix.WEXITED 0) in
|
||||
let reaped =
|
||||
await ~ms:5000 (fun () ->
|
||||
match Unix.waitpid [ Unix.WNOHANG ] lpid with
|
||||
| 0, _ -> false
|
||||
| _, s -> lstatus := s; true)
|
||||
in
|
||||
if not reaped then begin
|
||||
(try Unix.kill lpid Sys.sigkill with Unix.Unix_error _ -> ());
|
||||
fail "abort did not end the program"
|
||||
end
|
||||
else if !lstatus <> Unix.WEXITED 134 then
|
||||
fail "abort left status %s, wanted exit 134"
|
||||
(match !lstatus with
|
||||
| Unix.WEXITED c -> Printf.sprintf "exit %d" c
|
||||
| Unix.WSIGNALED c -> Printf.sprintf "signal %d" c
|
||||
| Unix.WSTOPPED c -> Printf.sprintf "stopped %d" c)
|
||||
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; lexe; lsock; lout ];
|
||||
if !failures = 0 then print_endline "agent: all tests passed"
|
||||
else begin
|
||||
Printf.printf "\n%d failure(s)\n" !failures;
|
||||
|
||||
120
vendor/agent/flan_agent.c
vendored
120
vendor/agent/flan_agent.c
vendored
@ -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
|
||||
@ -47,12 +50,39 @@ typedef void (*install_fn)(void);
|
||||
* is consistent. */
|
||||
typedef void (*call_fn)(void);
|
||||
|
||||
const char *flan_dev_result_get(uint64_t *gen, uint64_t *len);
|
||||
/* The value of the last evaluated expression, copied out under the seqlock in
|
||||
* runtime/flan_dev.c rather than borrowed. The buffer below is RESULT_MAX, so
|
||||
* the copy is never truncated; a read that loses the race reports the last
|
||||
* complete generation and no bytes, which leaves the daemon polling rather
|
||||
* than showing it half a value. */
|
||||
#define RESULT_MAX 4096
|
||||
int flan_dev_result_read(char *dst, uint64_t cap, uint64_t *gen,
|
||||
uint64_t *len);
|
||||
uint64_t flan_dev_result_cap(void);
|
||||
|
||||
/* 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:
|
||||
@ -68,12 +98,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. */
|
||||
@ -240,6 +284,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 */
|
||||
@ -257,7 +317,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();
|
||||
@ -296,14 +356,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
|
||||
@ -568,7 +628,16 @@ static void serve(int fd) {
|
||||
}
|
||||
if (strcmp(line, "result") == 0) {
|
||||
uint64_t gen = 0, len = 0;
|
||||
const char *v = flan_dev_result_get(&gen, &len);
|
||||
char v[RESULT_MAX];
|
||||
/* The one thing that could make the copy truncate, checked where it
|
||||
* would happen rather than trusted to two files holding the same
|
||||
* number. */
|
||||
if (flan_dev_result_cap() > sizeof v) {
|
||||
reply(fd, "err the agent's result buffer is smaller than the "
|
||||
"runtime's\n");
|
||||
return;
|
||||
}
|
||||
flan_dev_result_read(v, sizeof v, &gen, &len);
|
||||
char hdr[64];
|
||||
int k = snprintf(hdr, sizeof hdr, "%llu %llu\n", (unsigned long long)gen,
|
||||
(unsigned long long)len);
|
||||
@ -578,6 +647,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 ");
|
||||
@ -586,7 +664,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. */
|
||||
@ -598,8 +688,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;
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user