diff --git a/BUILT.md b/BUILT.md index 3d41ea2..75d8366 100644 --- a/BUILT.md +++ b/BUILT.md @@ -994,6 +994,96 @@ It waits for the program to bind before accepting an evaluation — one arriving like a compiler bug — and it accepts with a timeout so that a program which has exited takes the daemon with it rather than leaving an editor waiting on a socket nobody is serving. +### One process — `flan dev` is the program, and the compiler is a thread in it + +`flan dev` builds **one binary** that is the compiled Flan program *and* the whole OCaml compiler, and `exec`s it. +There is no child: `/proc//exe` on the pid an editor launched points at the program the daemon built, and +`test/test_dev.ml` checks exactly that rather than taking the claim on trust. + +Who owns what: + +| thread | what runs there | +|---|---| +| main | the program. C's `main` holds it — macOS needs a window on the main thread | +| a pthread | `caml_startup`, then `Flan.Dev.merged_setup` and `merged_serve`: the compiler and the editor socket | +| a pthread | `flan_agent.c`'s accept loop, as it always was | + +The program keeps `main()` and the compiler comes up beside it, rather than the other way round, because that is the +shape DISCUSS.md §11 landed on: **the agent is already a server inside the program**, so the compiler moves *into* the +program's process. It is SLIME's model — you start the image, it serves, the editor connects. + +`--two-process` is the escape hatch, for a machine where the compiler object cannot be built (no `ocamlfind`, no +`flan.cmxa` beside the binary). It has its own test and it stays. + +**The editor socket and its wire protocol did not move.** Emacs cannot tell the difference, which is what made the merge +testable: the whole existing suite is the check. + +Two rules hold in one address space and did not before: + +- **The game thread must never call into OCaml.** A native thread has no safe points, so the collector can never stop +it — which is precisely why a frame is never paused, and precisely what one convenient direct call would undo. Requests +reach the compiler by being left somewhere and picked up, never by a call out of the frame loop. +- **No OCaml `value` goes into Flan storage** without `caml_register_global_root`. That is how "the GC does not touch +the arenas" stops being true. + +A Flan `main` does not return — `Emit` ends it with `flan_exit` and an `unreachable` — so in one process that call would +take the compiler down with a program that merely *finished*. `flan_rt.c` has a hook, null in every other build, that +the merged entry point uses to flush, close stdout and park. The compiler learns the program is done the way the daemon +did: the pipe reads EOF. + +#### The internal socket is gone — the transport, measured + +The merge deliberately deleted nothing, so that it could be tested against the shape it replaced. This is the first +thing to go: **the unix socket between the compiler and the program**. + +It was a connect, a write and a read that looped back into the *same address space*. `vendor/agent/flan_agent.c`'s +verb table is now `handle_line(line, sink *)`, where a `sink` is either an fd or a growing buffer, and there are two +callers: `serve()`, which reads a line off a connection as before, and `flan_agent_request()`, which the compiler +thread calls directly. **One verb table, not two** — every answer is assembled from several pieces (a header, a name, a +newline), so the seam had to be the writing rather than the handlers, or they would have been duplicated and would +drift. `Dev.deliver`, `Dev.result` and `Dev.ask` were three copies of the same socket dance; they are now three lines +over one `Dev.request`. + +**Which path is taken is decided by the linker, not by a flag.** `flan_agent_request` is declared *weak* in +`lib/dynload_stubs.c`, so it is null in every binary that links no `flan_agent.o` — the `flan` launcher, `flan reload`, +the two-process daemon, every test — and `Agent.request` answers `None` there, which is what makes `Dev` fall back to +the socket. A flag could disagree with reality; this cannot. + +Three things had to be right: + +- **The ring still has one producer at a time.** `queue_room` checks for space separately from `publish`'s store, which +was safe only because the accept loop was the sole producer and was single-threaded. The compiler thread can now ask +while the accept loop is serving, so `handle_line` runs under a mutex. It is held across the `dlopen`, which is what +the accept loop already did to itself by serving connections inline. +- **A delivery is still only *queued*.** The direct call publishes to the ring exactly as the listener did; the install +is one store per function on the game thread at a frame boundary. Installing on the spot would be a frame running half +in the old code and half in the new, and it would be an easy thing to do by accident here. +- **The OCaml runtime system is released across the call.** A delivery is a `dlopen` — milliseconds of relocation and +the loader lock — and holding OCaml's lock through it stalls every other OCaml thread. DISCUSS.md §14's third cost, in +the one place this change creates it. + +**What it is worth, measured** (this machine, warm caches, a one-`defn` redefinition over the editor socket, median of +12): + +| | before | after | +|---|---|---| +| redefinition, end to end | 23.2ms | 22.0ms | +| ...of which the build (`:ms`) | 20.4ms | 19.0ms | +| a `break` round trip (editor socket + one agent ask) | 0.069ms | 0.019ms | + +**The transport was about 50µs of a 23ms redefinition, and removing it does not move that number.** That is the finding, +and it is worth more than a speedup would have been: the end-to-end column moved by less than its own run-to-run spread, +and `--two-process` measures 21.6ms — marginally *faster* than the merged build did before this change. **The merge's +prize was never latency.** It is that the compiler and the program now share an address space, which is what makes the +items below deletable at all and what unblocks reading the stopped frame's memory directly. Anyone reaching for an +in-process JIT on the strength of "transport is slow" should read this row first: code generation is 19 of the 22 +milliseconds. + +The test that pins it is a deletion, because a reply cannot say which way it came: `test_dev.ml` **unlinks the agent's +socket file** once the merged program has bound it, and then runs every evaluation in the file. Unlinking a bound unix +socket does not disturb the listener, it makes new connects fail — so if the deliveries still install, nothing +connected. The agent still binds it, for `--two-process` and for a person at a raw socket. + ### The Emacs client `emacs/flan-mode.el` derives from `prog-mode` with `lisp-mode`'s syntax table, which is most of the work: Flan is diff --git a/NEXT.md b/NEXT.md index bdfa208..f68de04 100644 --- a/NEXT.md +++ b/NEXT.md @@ -523,15 +523,28 @@ acquisitions against releases at that boundary and report what is still held at annotation, nothing running at a distance — it does not change how code is written, it reports when something was forgotten. -## Before the batch below: read `DISCUSS.md`'s "NEXT SESSION STARTS HERE" +## ~~Before the batch below: read `DISCUSS.md`'s "NEXT SESSION STARTS HERE"~~ — **answered, and being unwound** -An architectural question was raised at the end of 2026-09-12 and agreed as the next thing to investigate: **putting -the compiler inside the running program's process, instead of the two separate processes there are today.** It may -reopen several things recorded here as settled — the watch design, the 4K result cap and its seqlock, the snapshot -machinery, the render-thunk-per-inspection design for locals and globals, and whether an in-process JIT or a -hand-written backend is needed at all. +The architectural question raised at the end of 2026-09-12 — **putting the compiler inside the running program's +process** — was researched (`DISCUSS.md` §14), then built: `flan dev` is one binary and one process, and the editor +socket did not move. See "One process" in [`BUILT.md`](BUILT.md). -It is research first, not building. The batch below stays valid and none of it is blocked by the question. +What it reopened is now being deleted one piece at a time, each with its own green run: + +1. ~~**The internal socket, the line protocol, and `Dev.deliver`/`result`/`ask`.**~~ **Done** — a delivery is a direct + call into the agent's verb table. Measured: the transport was ~50µs of a 23ms redefinition, so the end-to-end + number did not move. Code generation is 19 of the 22 milliseconds, which is the number any backend argument has to + start from. +2. **The 4K `RESULT_MAX` cap** in `runtime/flan_dev.c`. It exists to size a transport buffer. **The seqlock does not + go with it** — the game thread still writes and the compiler thread still reads, so that race is real in one + process too. +3. **`flan_agent.c`'s snapshot copying and generation stamping.** The compiler can read the stopped frame's memory + directly. +4. **The render-thunk-per-inspection design for locals and globals.** A redesign rather than a deletion, and its own + lane: it is what unblocks "the inspector can retain a value". + +Still reopened and still undecided: the watch design (push was chosen partly because polling costs a compile), and +whether an in-process JIT or a hand-written backend is needed at all. ## ~~Queued: a second tier of the standard library, after macros~~ — **landed** diff --git a/lib/agent.ml b/lib/agent.ml new file mode 100644 index 0000000..30b9319 --- /dev/null +++ b/lib/agent.ml @@ -0,0 +1,23 @@ +(** The agent, as the compiler reaches it when both are in one process. + + [flan dev] builds one binary that is the compiled program and holds this + compiler; the agent's listener thread and this one are threads in it. So a + request that used to be a connect, a write and a read on a unix socket + looping back into this address space is a call into + [vendor/agent/flan_agent.c] instead. + + [None] means there is no agent in this process — the [flan] binary that + builds the merged program, [flan reload], the two-process daemon, every + test. The answer comes from the linker rather than from a flag: the symbol + is weak in [dynload_stubs.c] and is null in a binary that links no + [flan_agent.o]. [Dev] falls back to the socket on [None], which is what + keeps [--two-process] working unchanged. + + What does *not* change is where a delivery lands. The agent still only + publishes to its ring; the game thread picks the job up when it next + reaches a frame boundary. The rule that keeps the collector away from the + frame thread — the game thread never calls into OCaml, requests are left + somewhere and picked up — is the same rule with the same mechanism. This is + the compiler thread calling C, never the other way. *) + +external request : string -> string option = "flan_agent_direct" diff --git a/lib/dev.ml b/lib/dev.ml index bba36e0..4be873b 100644 --- a/lib/dev.ml +++ b/lib/dev.ml @@ -94,43 +94,29 @@ let await ?(ms = 5000) f = in go ms -(* ── Delivery ──────────────────────────────────────────────────────── *) +(* ── Asking the agent ──────────────────────────────────────────────── *) -(* The agent answers "ok" when it has queued a module, and anything else is a - refusal with a reason. Reporting that back rather than swallowing it is what - keeps a failed delivery from looking like a successful evaluation — the - whole class of bug this socket makes possible. *) -let deliver t path = +(* One line out, one line back. The agent is not a protocol and must not become + one, and every verb below goes through here. + + Two ways to ask, and the caller cannot tell them apart. In a merged build + the agent is in this process and the answer is a function call — the socket + would be a connect, a write and a read looping back into this same address + space, which is the transport the merge exists to remove. In + [--two-process] there is no agent here, so it is the socket, exactly as + before. + + Which one is not a flag: [Agent.request] is [None] when the linker resolved + a weak symbol to null, so it is [None] in precisely the binaries that have + no agent to call. A flag could disagree with reality; this cannot. *) +let over_socket t line = let s = Unix.socket Unix.PF_UNIX Unix.SOCK_STREAM 0 in Fun.protect ~finally:(fun () -> try Unix.close s with Unix.Unix_error _ -> ()) (fun () -> Unix.connect s (Unix.ADDR_UNIX t.agent); - let msg = path ^ "\n" in + let msg = line ^ "\n" in ignore (Unix.write_substring s msg 0 (String.length msg)); - let b = Bytes.create 1024 in - let buf = Buffer.create 64 in - let rec drain () = - match Unix.read s b 0 1024 with - | 0 -> () - | n -> Buffer.add_subbytes buf b 0 n; drain () - | exception Unix.Unix_error _ -> () - in - drain (); - String.trim (Buffer.contents buf)) - -(* Read back the value of the last expression evaluated, with the counter that - says whether it is a new one. The thunk runs on the game thread whenever the - program next reaches a frame boundary, which is not a moment the daemon gets - to know about, so this waits for the counter to move rather than assuming it - has. *) -let result t = - let s = Unix.socket Unix.PF_UNIX Unix.SOCK_STREAM 0 in - Fun.protect - ~finally:(fun () -> try Unix.close s with Unix.Unix_error _ -> ()) - (fun () -> - Unix.connect s (Unix.ADDR_UNIX t.agent); - ignore (Unix.write_substring s "result\n" 0 7); let b = Bytes.create 4096 in let buf = Buffer.create 256 in let rec drain () = @@ -140,47 +126,53 @@ let result t = | exception Unix.Unix_error _ -> () in drain (); - let text = Buffer.contents buf in - match String.index_opt text '\n' with - | None -> None - | Some i -> - let header = String.sub text 0 i in - let body = String.sub text (i + 1) (String.length text - i - 1) in - (match String.split_on_char ' ' header with - | [ g; _ ] -> - (match Int64.of_string_opt g with - | Some g -> Some (g, body) - | None -> None) - | _ -> None)) + Buffer.contents buf) + +let request t line = + match Agent.request line with + | Some answer -> answer + | None -> over_socket t line + +(* ── Delivery ──────────────────────────────────────────────────────── *) + +(* The agent answers "ok" when it has queued a module, and anything else is a + refusal with a reason. Reporting that back rather than swallowing it is what + keeps a failed delivery from looking like a successful evaluation — the + whole class of bug this hand-off makes possible. + + "Queued", still, and not "installed", in one process as in two: the store + happens on the game thread at a frame boundary, and a direct call that + installed on the spot would be a frame running half in the old code and half + in the new. *) +let deliver t path = String.trim (request t path) + +(* Read back the value of the last expression evaluated, with the counter that + says whether it is a new one. The thunk runs on the game thread whenever the + program next reaches a frame boundary, which is not a moment the compiler + gets to know about, so this waits for the counter to move rather than + assuming it has. *) +let result t = + let text = request t "result" in + match String.index_opt text '\n' with + | None -> None + | Some i -> + let header = String.sub text 0 i in + let body = String.sub text (i + 1) (String.length text - i - 1) in + (match String.split_on_char ' ' header with + | [ g; _ ] -> + (match Int64.of_string_opt g with + | Some g -> Some (g, body) + | None -> None) + | _ -> None) (* ── The break state ───────────────────────────────────────────────── *) (* Everything above is about changing a *running* program. This is the other half: an unhandled [error] does not kill a dev build, it stops the game - thread on the frame that erred and waits. The agent's socket is where that - shows, and the daemon is the only thing holding that socket — so an editor - asks here or not at all. - - One line out, one line back, exactly like [result]: the agent is not a - protocol and must not become one. *) -let ask t verb = - let s = Unix.socket Unix.PF_UNIX Unix.SOCK_STREAM 0 in - Fun.protect - ~finally:(fun () -> try Unix.close s with Unix.Unix_error _ -> ()) - (fun () -> - Unix.connect s (Unix.ADDR_UNIX t.agent); - let msg = verb ^ "\n" in - ignore (Unix.write_substring s msg 0 (String.length msg)); - let b = Bytes.create 4096 in - let buf = Buffer.create 128 in - let rec drain () = - match Unix.read s b 0 4096 with - | 0 -> () - | n -> Buffer.add_subbytes buf b 0 n; drain () - | exception Unix.Unix_error _ -> () - in - drain (); - Buffer.contents buf) + thread on the frame that erred and waits. The agent is where that shows, and + the session is the only thing holding it — so an editor asks here or not at + all. *) +let ask t verb = request t verb type state = | Running diff --git a/lib/dynload_stubs.c b/lib/dynload_stubs.c index a86d7eb..306a49e 100644 --- a/lib/dynload_stubs.c +++ b/lib/dynload_stubs.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -146,3 +147,52 @@ CAMLprim value flan_peek_bytes(value p, value off, value n) { (size_t)Long_val(n)); CAMLreturn(s); } + +/* ── The agent, when it is in this same process ─────────────────────── */ + +/* [flan dev] builds one binary that is the compiled program and holds this + * compiler, so a request to the agent need not leave the address space. The + * two ends still meet at one line of text and one answer — that is + * vendor/agent/flan_agent.c's [handle_line], and this is a call to it. + * + * Weak, because the same [flan] binary that builds a merged program does not + * itself contain an agent: the launcher, [flan reload], the two-process daemon + * and every test link no flan_agent.o, and there the symbol is null. So "is + * there an agent in this process" is answered by the linker rather than by a + * flag that could disagree with reality, and [None] here is what makes + * lib/dev.ml fall back to the socket. + * + * The runtime system is released across the call. A delivery does a [dlopen], + * which is milliseconds of relocation and the loader lock, and holding OCaml's + * lock through it stalls every other OCaml thread for no reason — DISCUSS.md + * §14's third cost, in the one place this lane creates it. Nothing the agent + * does touches an OCaml value, so there is nothing to keep alive across it. */ +extern char *flan_agent_request(const char *line, uint64_t *len) + __attribute__((weak)); +extern void flan_agent_request_free(char *p) __attribute__((weak)); + +CAMLprim value flan_agent_direct(value line) { + CAMLparam1(line); + CAMLlocal2(s, r); + char *out; + uint64_t n = 0; + if (flan_agent_request == NULL) CAMLreturn(Val_int(0)); /* None */ + { + /* Copied out first: [String_val] points into the OCaml heap, which may + move once the runtime system is released. */ + size_t k = caml_string_length(line); + char *copy = malloc(k + 1); + if (copy == NULL) caml_failwith("out of memory asking the agent"); + memcpy(copy, String_val(line), k); + copy[k] = '\0'; + caml_release_runtime_system(); + out = flan_agent_request(copy, &n); + caml_acquire_runtime_system(); + free(copy); + } + s = caml_alloc_initialized_string((mlsize_t)n, out == NULL ? "" : out); + if (out != NULL && flan_agent_request_free != NULL) + flan_agent_request_free(out); + r = caml_alloc_some(s); + CAMLreturn(r); +} diff --git a/test/test_dev.ml b/test/test_dev.ml index 6b6049a..f021796 100644 --- a/test/test_dev.ml +++ b/test/test_dev.ml @@ -105,6 +105,29 @@ let () = (string_of_int pid) link want | exception Unix.Unix_error _ -> () end; + (* And that the compiler is not talking to the program over a socket any + more. In one process a delivery is a call into the agent's own verb + table, so the *path* it used to connect on is now needed by nothing — + removing it is therefore the decisive test, and the only one available + from out here: a reply cannot say which way it came. + + Unlinking a bound unix socket does not disturb the listener; it makes + new connects fail with ENOENT. So if every evaluation below still + installs, nothing connected. The agent's own socket stays bound for + [--two-process] and for a person at a raw socket, which is why it is + still created at all. + + The path is the same one [start_merged] computes, and the /proc check + above has already established that this pid is the program. *) + let agent_sock = + Filename.concat + (Filename.concat (Filename.get_temp_dir_name ()) + (Printf.sprintf "flan-dev-%d" pid)) + "agent.sock" + in + if not (Sys.file_exists agent_sock) then + fail "the merged program never bound %s" agent_sock + else (try Unix.unlink agent_sock with Unix.Unix_error _ -> ()); (* The daemon owns the program's lifetime and kills it on [close], so every step waits for the program to have got there. "ok" from an eval means the module was queued, not that it has been installed. Output diff --git a/vendor/agent/flan_agent.c b/vendor/agent/flan_agent.c index d8000be..60721af 100644 --- a/vendor/agent/flan_agent.c +++ b/vendor/agent/flan_agent.c @@ -26,6 +26,15 @@ * 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 * to a socket, not something this file grows into. + * + * The compiler is no longer a separate *process*, though. [flan dev] builds one + * binary that is the program and holds the whole compiler, so a request from it + * is a call to [flan_agent_request] below and not a connect on a socket that + * loops back into this address space. It is the same verb table either way — + * [handle_line] writes into a [sink] that is an fd or a buffer — and the same + * hand-off: a module is published to the ring, and the game thread installs it + * at a frame boundary. The socket stays for [--two-process] and for a person + * with nc. */ #include @@ -81,8 +90,9 @@ uint64_t flan_dev_result_cap(void); * 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. */ + * one producer at a time — [request_lock] is what keeps that true now that the + * compiler can call in as well as connect — 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: @@ -571,20 +581,357 @@ int32_t flan_agent_wait(int32_t ms) { return flan_agent_poll(); } +/* Where an answer goes. Two kinds, and the verb table below cannot tell them + * apart: a socket, which is how the two-process daemon asks, and a buffer, + * which is how the compiler asks when it is a thread in this same process. + * + * One sink rather than two copies of the verb table. Every answer here is + * assembled from several pieces — a header, a name, a newline — so the seam + * had to be the writing and not the handler, or the handlers would have had + * to be duplicated and would drift. */ +typedef struct { + int fd; /* -1 when this is a buffer */ + char *buf; + size_t len; + size_t cap; +} sink; + /* MSG_NOSIGNAL rather than write(2). A reply goes out in more than one piece, * and a sender that has read enough and closed leaves the rest of it writing * into a closed socket — which is SIGPIPE, whose default action would kill the * program the agent is embedded in. Suppressing it per call rather than * installing a handler, because the disposition belongs to the program and not * to us. */ -static void reply(int fd, const char *s) { - size_t n = strlen(s); - while (n > 0) { - ssize_t k = send(fd, s, n, MSG_NOSIGNAL); - if (k <= 0) return; - s += k; - n -= (size_t)k; +static void emit(sink *o, const void *p, size_t n) { + if (o->fd >= 0) { + const char *s = (const char *)p; + while (n > 0) { + ssize_t k = send(o->fd, s, n, MSG_NOSIGNAL); + if (k <= 0) return; + s += k; + n -= (size_t)k; + } + return; } + if (o->len + n + 1 > o->cap) { + size_t want = o->cap ? o->cap : 1024; + while (want < o->len + n + 1) want *= 2; + char *g = realloc(o->buf, want); + /* Nothing useful to do with a failed realloc here: the caller reads + * [len], so a short answer is what it sees, and that is the same shape as + * a socket the peer closed early. */ + if (g == NULL) return; + o->buf = g; + o->cap = want; + } + memcpy(o->buf + o->len, p, n); + o->len += n; + o->buf[o->len] = '\0'; +} + +static void reply(sink *o, const char *s) { emit(o, s, strlen(s)); } + +/* One line, one answer, one module. This is the whole of what the agent is + * asked, and it is reached two ways: from the socket below, and — in a build + * where the compiler is a thread in this same process — by being called. The + * two share this function rather than a protocol. + * + * What does not change with the caller is where the work lands. [dlopen] runs + * here, on whichever thread asked; the install is only ever *published* to the + * ring, and the game thread picks it up at a frame boundary. A direct call + * that installed on the spot would be a frame running half in the old code and + * half in the new. */ +/* One request at a time, whoever asks. Until the compiler could call in, the + * accept loop was the only caller and was single-threaded, and the ring's + * "exactly one producer" was true by construction. It is not any more: a + * direct call runs on the compiler thread while the accept loop can be serving + * the same verbs. This lock puts that back rather than weakening the ring — + * [publish]'s room check is separate from its store, and two producers + * interleaving there is the silent overwrite the ring comment above describes. + * + * It is held across the [dlopen], which is milliseconds. That is what the + * accept loop already did to itself by serving connections inline, so no + * caller waits longer than it did before. The game thread never takes it. */ +static pthread_mutex_t request_lock = PTHREAD_MUTEX_INITIALIZER; + +static void handle_line(char *line, sink *o) { + /* The one verb that is not a module: read back the value of the last + * expression evaluated, with the counter that says whether it is a new + * one. The daemon polls this rather than the agent holding a connection + * open across a frame boundary it does not control. */ + /* Only while stopped: what is on offer, and which one to take. Both are + * refused when the program is running, by name, rather than silently + * doing nothing — there is no restart stack to walk from here. */ + /* Whether the program is stopped, and on what. Answered while it is + * running too — "running" is an answer, not a refusal — because this is + * the one question an editor asks without knowing the state already, and + * refusing it would leave nothing to poll. */ + if (strcmp(line, "status") == 0) { + if ((atomic_load(&depth) > 0)) { + reply(o, "stopped "); + reply(o, condition_name); + reply(o, "\n"); + } else + reply(o, "running\n"); + return; + } + /* One line per restart, innermost first: the index it is taken by, a flag + * for whether it can be taken at all, and the name. The index leads + * because it is the identity - two frames can offer [retry] and only one + * of them is the one meant, which is the whole reason this is not a list + * of names any more. Read from the snapshot, never from the live stack. */ + if (strcmp(line, "restarts") == 0) { + if (!(atomic_load(&depth) > 0)) { reply(o, "err not stopped\n"); return; } + snapshot *s = snap_top(); + if (s == NULL) { reply(o, "err no restart snapshot\n"); return; } + for (int32_t i = 0; i < s->n; i++) { + char hdr[32]; + int k = snprintf(hdr, sizeof hdr, "%d %c ", i, + s->reachable[i] ? '+' : '-'); + if (k > 0) emit(o, hdr, (size_t)k); + emit(o, s->names + s->off[i], (size_t)s->len[i]); + reply(o, "\n"); + } + reply(o, ".\n"); + return; + } + /* Where the stopped thread is. One line per frame, innermost first: + * the index, whether the frame is the program's or the evaluation's, how + * many slots it has, where it is written, and its name. Served from the + * snapshot, never from the live chain — the game thread is parked in the + * break loop, but the loop polls, and a poll runs Flan. + * + * Refused while running, like every other break verb and for the same + * reason: a chain read by one thread while another pushes and pops it is + * not a backtrace, it is a race with a plausible shape. */ + if (strcmp(line, "backtrace") == 0) { + if (!(atomic_load(&depth) > 0)) { reply(o, "err not stopped\n"); return; } + snapshot *s = snap_top(); + if (s == NULL) { reply(o, "err no frame snapshot\n"); return; } + if (s->fn == 0 && s->ftotal == 0) { + /* Not "no frames": a release build has no shadow stack at all, and + * answering with an empty backtrace would read as a program with an + * empty stack, which is not a thing that can be stopped. */ + reply(o, "err this program was not built with --dev, so it has no " + "shadow stack to walk\n"); + return; + } + for (int32_t i = 0; i < s->fn; i++) { + char hdr[64]; + /* Both fingerprints go before the location and the location before + * the name, because the name is the one field that can contain a + * space and so has to be last. */ + int k = snprintf(hdr, sizeof hdr, "%d %c %d %d %d ", i, + s->fmine[i] ? '+' : '-', s->fslots[i], s->fsig[i], + s->frsig[i]); + if (k > 0) emit(o, hdr, (size_t)k); + if (s->fllen[i] > 0) + emit(o, s->ftext + s->floff[i], (size_t)s->fllen[i]); + else + reply(o, "?"); + reply(o, " "); + emit(o, s->ftext + s->fnoff[i], (size_t)s->fnlen[i]); + reply(o, "\n"); + } + if (s->ftotal > s->fn) { + char more[64]; + int k = snprintf(more, sizeof more, "... %d\n", s->ftotal - s->fn); + if (k > 0) emit(o, more, (size_t)k); + } + reply(o, ".\n"); + return; + } + /* Which of a frame's slots have been bound at the point it stopped. One + * line per slot: the index and [+] or [-]. + * + * The daemon asks this before it builds a thunk, and that order is the + * safety: an unbound slot is a null address, a thunk that rendered one + * would dereference it, and a program stopped in a break loop is the last + * place to take a fault. It is answered from the snapshot, so the set the + * daemon is told about is the set the thunk will resolve against. + * + * It says nothing about *what* a slot holds, or what it is called. Those + * are facts about the build, and the daemon owns the build — [Tast.fn] + * carries [slots] and [snames] beside each other. Sending them from here + * would be a second copy of them that could drift. */ + if (strncmp(line, "locals ", 7) == 0) { + if (!(atomic_load(&depth) > 0)) { reply(o, "err not stopped\n"); return; } + snapshot *s = snap_top(); + if (s == NULL) { reply(o, "err no frame snapshot\n"); return; } + char *end = NULL; + long at = strtol(line + 7, &end, 10); + if (end == line + 7) { reply(o, "err locals wants a frame index\n"); return; } + if (at < 0 || at >= s->fn) { reply(o, "err no frame at that index\n"); return; } + if (s->fslots[at] == 0) { + reply(o, "err that frame records no slots; it has no named local, or " + "this build does not record them\n"); + return; + } + for (int32_t i = 0; i < s->fslots[at]; i++) { + char l[32]; + int k = snprintf(l, sizeof l, "%d %c\n", i, + flan_dev_frame_slot(s->fframe[at], i) ? '+' : '-'); + if (k > 0) emit(o, l, (size_t)k); + } + reply(o, ".\n"); + return; + } + /* Take the i'th, optionally checking that the caller and this snapshot + * still agree on what the i'th is called. The name is not the lookup - + * that is the bug - it is a receipt: a client that listed, prompted, and + * chose has the name in hand for nothing, and sending it turns a bare + * integer into something that can be wrong out loud. */ + if (strncmp(line, "restart-at ", 11) == 0) { + if (!(atomic_load(&depth) > 0)) { reply(o, "err not stopped\n"); return; } + snapshot *s = snap_top(); + if (s == NULL) { reply(o, "err no restart snapshot\n"); return; } + char *end = NULL; + long idx = strtol(line + 11, &end, 10); + if (end == line + 11) { reply(o, "err restart-at wants an index\n"); return; } + if (idx < 0 || idx >= s->n) { + reply(o, "err no restart at that index\n"); + return; + } + while (*end == ' ') end++; + if (*end != '\0') { + size_t k = strlen(end); + if (k != (size_t)s->len[idx] || + memcmp(end, s->names + s->off[idx], k) != 0) { + reply(o, "err that index is now "); + reply(o, s->names + s->off[idx]); + reply(o, ", not what you named; list the restarts again\n"); + return; + } + } + if (!s->reachable[idx]) { + /* Refused, with the reason, rather than accepted and dropped. The + * transfer would unwind to the thunk this break is inside and stop + * there, and the program would carry on as if nothing had been + * chosen. */ + reply(o, "err restart "); + reply(o, s->names + s->off[idx]); + reply(o, " is below the evaluation this break is inside, so a " + "transfer to it has nowhere to land; choose one offered " + "above it, or abort\n"); + return; + } + atomic_store(&chosen_index, (int)idx); + atomic_store(&chosen_gen, s->gen); + /* Published last, so the game thread never reads an index that is about + * to change, or one whose generation has not arrived yet. */ + atomic_store(&chosen_ready, 1); + reply(o, "ok\n"); + return; + } + /* By name, still, for a person at a raw socket - and now defined as + * exactly [restart-at] on the first index offering the name, which is + * what §4's walk already meant. So the two verbs cannot disagree, and a + * shadowed name is reachable through the other one rather than nowhere. */ + if (strncmp(line, "restart ", 8) == 0) { + if (!(atomic_load(&depth) > 0)) { reply(o, "err not stopped\n"); return; } + snapshot *s = snap_top(); + if (s == NULL) { reply(o, "err no restart snapshot\n"); return; } + size_t k = strlen(line + 8); + if (k == 0) { reply(o, "err bad restart name\n"); return; } + int32_t at = -1; + for (int32_t i = 0; i < s->n && at < 0; i++) + if ((size_t)s->len[i] == k && memcmp(s->names + s->off[i], line + 8, k) == 0) + at = i; + if (at < 0) { + reply(o, "err no restart named "); + reply(o, line + 8); + reply(o, " is active\n"); + return; + } + if (!s->reachable[at]) { + reply(o, "err restart "); + reply(o, line + 8); + reply(o, " is below the evaluation this break is inside, so a " + "transfer to it has nowhere to land; choose one offered " + "above it, or abort\n"); + return; + } + atomic_store(&chosen_index, at); + atomic_store(&chosen_gen, s->gen); + atomic_store(&chosen_ready, 1); + reply(o, "ok\n"); + return; + } + if (strcmp(line, "abort") == 0) { + if (!(atomic_load(&depth) > 0)) { reply(o, "err not stopped\n"); return; } + reply(o, "ok\n"); + atomic_store(&aborting, 1); + return; + } + if (strcmp(line, "result") == 0) { + uint64_t gen = 0, len = 0; + 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(o, "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); + if (k > 0) { + emit(o, hdr, (size_t)k); + if (len > 0) emit(o, v, (size_t)len); + } + 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 one producer runs at a time, so room seen now is + * room still there at [publish] below. */ + if (!queue_room()) { + reply(o, "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(o, "err "); + reply(o, dlerror()); + reply(o, "\n"); + return; + } + install_fn f = (install_fn)(uintptr_t)dlsym(h, "flan_reload_install"); + 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(o, "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. */ + void *transient = + (c == NULL) ? NULL : dlsym(h, "flan_reload_transient"); + /* Answer before queueing, not after. The game thread can install and run + * to completion between the two, and a program that exits there would tear + * down this connection with the reply still unwritten — which reaches the + * sender as a reset, not as an answer. */ + reply(o, "ok\n"); + /* "queued", not "installed": the store happens on the game thread, at a + * 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; } /* One connection, one line, one module. Loading here rather than in the game @@ -602,6 +949,7 @@ static void serve(int fd) { struct timeval tv = { 2, 0 }; setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof tv); } + sink o = { fd, NULL, 0, 0 }; char line[4096]; size_t n = 0; for (;;) { @@ -611,290 +959,49 @@ static void serve(int fd) { line[n] = '\0'; char *nl = strchr(line, '\n'); if (nl == NULL) { - if (n == sizeof line - 1) { reply(fd, "err path too long\n"); return; } + if (n == sizeof line - 1) { reply(&o, "err path too long\n"); return; } continue; } *nl = '\0'; - /* The one verb that is not a module: read back the value of the last - * expression evaluated, with the counter that says whether it is a new - * one. The daemon polls this rather than the agent holding a connection - * open across a frame boundary it does not control. */ - /* Only while stopped: what is on offer, and which one to take. Both are - * refused when the program is running, by name, rather than silently - * doing nothing — there is no restart stack to walk from here. */ - /* Whether the program is stopped, and on what. Answered while it is - * running too — "running" is an answer, not a refusal — because this is - * the one question an editor asks without knowing the state already, and - * refusing it would leave nothing to poll. */ - if (strcmp(line, "status") == 0) { - if ((atomic_load(&depth) > 0)) { - reply(fd, "stopped "); - reply(fd, condition_name); - reply(fd, "\n"); - } else - reply(fd, "running\n"); - return; - } - /* One line per restart, innermost first: the index it is taken by, a flag - * for whether it can be taken at all, and the name. The index leads - * because it is the identity - two frames can offer [retry] and only one - * of them is the one meant, which is the whole reason this is not a list - * of names any more. Read from the snapshot, never from the live stack. */ - if (strcmp(line, "restarts") == 0) { - if (!(atomic_load(&depth) > 0)) { reply(fd, "err not stopped\n"); return; } - snapshot *s = snap_top(); - if (s == NULL) { reply(fd, "err no restart snapshot\n"); return; } - for (int32_t i = 0; i < s->n; i++) { - char hdr[32]; - int k = snprintf(hdr, sizeof hdr, "%d %c ", i, - s->reachable[i] ? '+' : '-'); - if (k > 0) send(fd, hdr, (size_t)k, MSG_NOSIGNAL); - send(fd, s->names + s->off[i], (size_t)s->len[i], MSG_NOSIGNAL); - reply(fd, "\n"); - } - reply(fd, ".\n"); - return; - } - /* Where the stopped thread is. One line per frame, innermost first: - * the index, whether the frame is the program's or the evaluation's, how - * many slots it has, where it is written, and its name. Served from the - * snapshot, never from the live chain — the game thread is parked in the - * break loop, but the loop polls, and a poll runs Flan. - * - * Refused while running, like every other break verb and for the same - * reason: a chain read by one thread while another pushes and pops it is - * not a backtrace, it is a race with a plausible shape. */ - if (strcmp(line, "backtrace") == 0) { - if (!(atomic_load(&depth) > 0)) { reply(fd, "err not stopped\n"); return; } - snapshot *s = snap_top(); - if (s == NULL) { reply(fd, "err no frame snapshot\n"); return; } - if (s->fn == 0 && s->ftotal == 0) { - /* Not "no frames": a release build has no shadow stack at all, and - * answering with an empty backtrace would read as a program with an - * empty stack, which is not a thing that can be stopped. */ - reply(fd, "err this program was not built with --dev, so it has no " - "shadow stack to walk\n"); - return; - } - for (int32_t i = 0; i < s->fn; i++) { - char hdr[64]; - /* Both fingerprints go before the location and the location before - * the name, because the name is the one field that can contain a - * space and so has to be last. */ - int k = snprintf(hdr, sizeof hdr, "%d %c %d %d %d ", i, - s->fmine[i] ? '+' : '-', s->fslots[i], s->fsig[i], - s->frsig[i]); - if (k > 0) send(fd, hdr, (size_t)k, MSG_NOSIGNAL); - if (s->fllen[i] > 0) - send(fd, s->ftext + s->floff[i], (size_t)s->fllen[i], MSG_NOSIGNAL); - else - reply(fd, "?"); - reply(fd, " "); - send(fd, s->ftext + s->fnoff[i], (size_t)s->fnlen[i], MSG_NOSIGNAL); - reply(fd, "\n"); - } - if (s->ftotal > s->fn) { - char more[64]; - int k = snprintf(more, sizeof more, "... %d\n", s->ftotal - s->fn); - if (k > 0) send(fd, more, (size_t)k, MSG_NOSIGNAL); - } - reply(fd, ".\n"); - return; - } - /* Which of a frame's slots have been bound at the point it stopped. One - * line per slot: the index and [+] or [-]. - * - * The daemon asks this before it builds a thunk, and that order is the - * safety: an unbound slot is a null address, a thunk that rendered one - * would dereference it, and a program stopped in a break loop is the last - * place to take a fault. It is answered from the snapshot, so the set the - * daemon is told about is the set the thunk will resolve against. - * - * It says nothing about *what* a slot holds, or what it is called. Those - * are facts about the build, and the daemon owns the build — [Tast.fn] - * carries [slots] and [snames] beside each other. Sending them from here - * would be a second copy of them that could drift. */ - if (strncmp(line, "locals ", 7) == 0) { - if (!(atomic_load(&depth) > 0)) { reply(fd, "err not stopped\n"); return; } - snapshot *s = snap_top(); - if (s == NULL) { reply(fd, "err no frame snapshot\n"); return; } - char *end = NULL; - long at = strtol(line + 7, &end, 10); - if (end == line + 7) { reply(fd, "err locals wants a frame index\n"); return; } - if (at < 0 || at >= s->fn) { reply(fd, "err no frame at that index\n"); return; } - if (s->fslots[at] == 0) { - reply(fd, "err that frame records no slots; it has no named local, or " - "this build does not record them\n"); - return; - } - for (int32_t i = 0; i < s->fslots[at]; i++) { - char l[32]; - int k = snprintf(l, sizeof l, "%d %c\n", i, - flan_dev_frame_slot(s->fframe[at], i) ? '+' : '-'); - if (k > 0) send(fd, l, (size_t)k, MSG_NOSIGNAL); - } - reply(fd, ".\n"); - return; - } - /* Take the i'th, optionally checking that the caller and this snapshot - * still agree on what the i'th is called. The name is not the lookup - - * that is the bug - it is a receipt: a client that listed, prompted, and - * chose has the name in hand for nothing, and sending it turns a bare - * integer into something that can be wrong out loud. */ - if (strncmp(line, "restart-at ", 11) == 0) { - if (!(atomic_load(&depth) > 0)) { reply(fd, "err not stopped\n"); return; } - snapshot *s = snap_top(); - if (s == NULL) { reply(fd, "err no restart snapshot\n"); return; } - char *end = NULL; - long idx = strtol(line + 11, &end, 10); - if (end == line + 11) { reply(fd, "err restart-at wants an index\n"); return; } - if (idx < 0 || idx >= s->n) { - reply(fd, "err no restart at that index\n"); - return; - } - while (*end == ' ') end++; - if (*end != '\0') { - size_t k = strlen(end); - if (k != (size_t)s->len[idx] || - memcmp(end, s->names + s->off[idx], k) != 0) { - reply(fd, "err that index is now "); - reply(fd, s->names + s->off[idx]); - reply(fd, ", not what you named; list the restarts again\n"); - return; - } - } - if (!s->reachable[idx]) { - /* Refused, with the reason, rather than accepted and dropped. The - * transfer would unwind to the thunk this break is inside and stop - * there, and the program would carry on as if nothing had been - * chosen. */ - reply(fd, "err restart "); - reply(fd, s->names + s->off[idx]); - reply(fd, " is below the evaluation this break is inside, so a " - "transfer to it has nowhere to land; choose one offered " - "above it, or abort\n"); - return; - } - atomic_store(&chosen_index, (int)idx); - atomic_store(&chosen_gen, s->gen); - /* Published last, so the game thread never reads an index that is about - * to change, or one whose generation has not arrived yet. */ - atomic_store(&chosen_ready, 1); - reply(fd, "ok\n"); - return; - } - /* By name, still, for a person at a raw socket - and now defined as - * exactly [restart-at] on the first index offering the name, which is - * what §4's walk already meant. So the two verbs cannot disagree, and a - * shadowed name is reachable through the other one rather than nowhere. */ - if (strncmp(line, "restart ", 8) == 0) { - if (!(atomic_load(&depth) > 0)) { reply(fd, "err not stopped\n"); return; } - snapshot *s = snap_top(); - if (s == NULL) { reply(fd, "err no restart snapshot\n"); return; } - size_t k = strlen(line + 8); - if (k == 0) { reply(fd, "err bad restart name\n"); return; } - int32_t at = -1; - for (int32_t i = 0; i < s->n && at < 0; i++) - if ((size_t)s->len[i] == k && memcmp(s->names + s->off[i], line + 8, k) == 0) - at = i; - if (at < 0) { - reply(fd, "err no restart named "); - reply(fd, line + 8); - reply(fd, " is active\n"); - return; - } - if (!s->reachable[at]) { - reply(fd, "err restart "); - reply(fd, line + 8); - reply(fd, " is below the evaluation this break is inside, so a " - "transfer to it has nowhere to land; choose one offered " - "above it, or abort\n"); - return; - } - atomic_store(&chosen_index, at); - atomic_store(&chosen_gen, s->gen); - atomic_store(&chosen_ready, 1); - reply(fd, "ok\n"); - return; - } - if (strcmp(line, "abort") == 0) { - if (!(atomic_load(&depth) > 0)) { reply(fd, "err not stopped\n"); return; } - reply(fd, "ok\n"); - atomic_store(&aborting, 1); - return; - } - if (strcmp(line, "result") == 0) { - uint64_t gen = 0, len = 0; - 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); - if (k > 0) { - send(fd, hdr, (size_t)k, MSG_NOSIGNAL); - if (len > 0) send(fd, v, (size_t)len, MSG_NOSIGNAL); - } - 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 "); - reply(fd, dlerror()); - reply(fd, "\n"); - return; - } - install_fn f = (install_fn)(uintptr_t)dlsym(h, "flan_reload_install"); - 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. */ - void *transient = - (c == NULL) ? NULL : dlsym(h, "flan_reload_transient"); - /* Answer before queueing, not after. The game thread can install and run - * to completion between the two, and a program that exits there would tear - * down this connection with the reply still unwritten — which reaches the - * 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. 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"); + pthread_mutex_lock(&request_lock); + handle_line(line, &o); + pthread_mutex_unlock(&request_lock); return; } } +/* The direct hand-off. In a [flan dev] build the compiler is a thread in this + * process, so a request that used to be a connect, a write and a read on a + * unix socket looping back into this address space is now this call — the same + * verb table, answered into a buffer instead of onto an fd. + * + * The buffer is malloc'd here and freed by the caller. It is not a static, + * because the socket's accept loop can be answering [status] on another thread + * while this runs, and two answers into one buffer is a race with a plausible + * shape. + * + * The rule this must not break is the one that keeps the collector off the + * frame thread: nothing here calls into OCaml and nothing here runs on the + * game thread. A delivery is left in the ring exactly as the listener leaves + * it, and the game thread picks it up when it reaches a frame boundary. */ +char *flan_agent_request(const char *line, uint64_t *len) { + char stack[4096]; + sink o = { -1, NULL, 0, 0 }; + size_t n = strlen(line); + if (n >= sizeof stack) { + *len = 0; + return NULL; + } + memcpy(stack, line, n + 1); + pthread_mutex_lock(&request_lock); + handle_line(stack, &o); + pthread_mutex_unlock(&request_lock); + *len = (uint64_t)o.len; + return o.buf; +} + +void flan_agent_request_free(char *p) { free(p); } + static void *accept_loop(void *arg) { (void)arg; for (;;) {