From 44e199186e2ecfbdd4d9497e4a7f036e57809664 Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Fri, 11 Sep 2026 07:09:44 +0700 Subject: [PATCH] An expression's module is unloaded; a redefinition's never can be C-x C-e is the case that repeats - you evaluate expressions constantly and redefine functions occasionally - and it is also the one case where unloading is safe. The thunk is called directly by flan_reload_call rather than through a cell, and it takes no registry slot, so once it has returned nothing points into its text and the value it produced has been copied out. The module says so with flan_reload_transient and the agent dlcloses it. Skipping the registry matters for more than tidiness: the table holds 4096 names and an expression evaluated in a loop would have exhausted it. A module that publishes a body can never make this claim, since leaving a pointer behind is its whole purpose. Measured on a running program: sixteen expression evaluations retain zero mappings, each redefinition retains three, permanently and correctly. --- NEXT.md | 12 ++++++++++++ lib/emit.ml | 36 +++++++++++++++++++++++++++--------- test/test_session.ml | 20 ++++++++++++++++++++ vendor/agent/flan_agent.c | 12 ++++++++++-- 4 files changed, 69 insertions(+), 11 deletions(-) diff --git a/NEXT.md b/NEXT.md index 9b8ce36..6085cfa 100644 --- a/NEXT.md +++ b/NEXT.md @@ -751,6 +751,18 @@ An evaluation is **not** a declaration: the thunk is built against the program and never spliced into it, so `describe` does not fill up with `eval/N` for every expression ever typed. +**The module is unloaded afterwards**, which is the one case where that is +safe. The thunk is called directly by `flan_reload_call` rather than through a +cell, and it takes no registry slot — so once it has returned, nothing points +into its text and the value it produced has been copied out. It declares that +with `@flan_reload_transient` and the agent `dlclose`s it. Measured: sixteen +expression evaluations retain **zero** mappings, where each *redefinition* +retains three, permanently and correctly — a module that publishes a body +exists precisely to leave a pointer behind, and can never claim this. + +Skipping the registry matters for more than tidiness: the table holds 4096 +names and an expression evaluated in a loop would exhaust it. + The test that matters is the same expression twice: the fixture increments `ticks` every frame, so two evaluations must disagree. A value computed in the compiler, or read out of a copy of the program's state, would not. diff --git a/lib/emit.ml b/lib/emit.ml index 37acfbb..e152277 100644 --- a/lib/emit.ml +++ b/lib/emit.ml @@ -1005,8 +1005,17 @@ let redefinition ?(checks = true) ?(dev = false) ?(known = fun _ -> true) in let targets = List.map target fns in let m = new_module ~checks ~dev ~known p in + (* A thunk the module runs itself is excluded from all of this: it is called + directly by [flan_reload_call], so it needs no cell, must not be published + into one, and must not take a registry slot — there are 4096 of those and + an expression evaluated in a loop would exhaust them. Nothing pointing + into the module is also what lets the agent unload it afterwards. *) + let transient f = call = Some f in let new_fns = - List.filter (fun (f : Tast.fn) -> not (known f.Tast.name)) p.Tast.fns + List.filter + (fun (f : Tast.fn) -> + (not (known f.Tast.name)) && not (transient f.Tast.name)) + p.Tast.fns and new_globals = List.filter (fun (g : Tast.global) -> not (known g.Tast.gname)) p.Tast.globals in @@ -1028,12 +1037,13 @@ let redefinition ?(checks = true) ?(dev = false) ?(known = fun _ -> true) the installer below. *) List.iter (fun (f : Tast.fn) -> - Buffer.add_string m.out - (if known f.Tast.name then - Printf.sprintf "%s = external global ptr\n" (cellname f.Tast.name) - else - Printf.sprintf "%s = internal global ptr null\n" - (cellptr f.Tast.name))) + if not (transient f.Tast.name) then + Buffer.add_string m.out + (if known f.Tast.name then + Printf.sprintf "%s = external global ptr\n" (cellname f.Tast.name) + else + Printf.sprintf "%s = internal global ptr null\n" + (cellptr f.Tast.name))) p.Tast.fns; if new_fns <> [] || new_globals <> [] then Buffer.add_string m.out @@ -1106,7 +1116,8 @@ let redefinition ?(checks = true) ?(dev = false) ?(known = fun _ -> true) p.Tast.globals; List.iter (fun (f : Tast.fn) -> - if known f.Tast.name then + if transient f.Tast.name then () + else if known f.Tast.name then Buffer.add_string b (Printf.sprintf " store ptr %s, ptr %s\n" (fname f.Tast.name) (cellname f.Tast.name)) @@ -1129,7 +1140,14 @@ let redefinition ?(checks = true) ?(dev = false) ?(known = fun _ -> true) Buffer.add_string m.out (Printf.sprintf "\ndefine void @flan_reload_call() {\nentry:\n call %s %s()\n \ - ret void\n}\n" (ll Types.Unit) (fname fn)) + ret void\n}\n" (ll Types.Unit) (fname fn)); + (* Nothing outside this module refers to anything in it once the call has + returned — no cell holds an address in its text, the registry has no + slot for it, and the value it produced was copied out. So it says so, + and the agent unloads it. A module that publishes a body can never say + this: its whole purpose is to leave a pointer behind. *) + if fns = [ fn ] && consts = [] then + Buffer.add_string m.out "\n@flan_reload_transient = global i8 1\n" | None -> () end; finish m diff --git a/test/test_session.ml b/test/test_session.ml index bd6a116..4e5521f 100644 --- a/test/test_session.ml +++ b/test/test_session.ml @@ -175,6 +175,26 @@ let () = (String.concat " " c.Session.fns) | exception Loc.Error (_, m) -> fail "redefining game-draw: %s" m); + (* An expression's thunk leaves nothing behind, and the module says so, which + is what lets the agent unload it: nothing may point into its text + afterwards. So it is called directly rather than through a cell, and it + must not take a registry slot either — there are 4096 of those and an + expression evaluated in a loop would exhaust them. A module that publishes + a body can never say this; its whole purpose is to leave a pointer. *) + let t, _ = Session.create ~file:"programs/reload.flan" in + let e = Session.eval_expr t "(+ 1 2)" in + if not (has e.Session.ir "@flan_reload_transient") then + fail "an expression's module did not declare itself unloadable"; + if not (has e.Session.ir "define void @flan_reload_call") then + fail "an expression's module carried no thunk to run"; + if has e.Session.ir "flan.cellp.eval" then + fail "an expression's thunk took a registry slot"; + if has e.Session.ir "call ptr @flan_dev_cell" then + fail "an expression's thunk was looked up by name"; + let c = Session.eval t "(defn bump [] i64 (set counter (+ counter 1)) counter)" in + if has c.Session.ir "@flan_reload_transient" then + fail "a module that publishes a body claimed to be unloadable"; + if !failures = 0 then print_endline "session: all tests passed" else begin Printf.printf "\n%d failure(s)\n" !failures; diff --git a/vendor/agent/flan_agent.c b/vendor/agent/flan_agent.c index 092594f..d1f15ff 100644 --- a/vendor/agent/flan_agent.c +++ b/vendor/agent/flan_agent.c @@ -54,7 +54,11 @@ const char *flan_dev_result_get(uint64_t *gen, uint64_t *len); * the loader. Overflow drops the oldest request rather than stalling; a dev * loop that queues 64 reloads between two frames has a bigger problem. */ #define QUEUE 64 -typedef struct { install_fn install; call_fn call; } job; +/* [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: + * a cell holds an address inside a module's text, and unloading it would leave + * every call site pointing at unmapped memory. */ +typedef struct { install_fn install; call_fn call; void *handle; } job; static job queue[QUEUE]; static atomic_uint head; /* written by the listener */ @@ -83,6 +87,7 @@ int32_t flan_agent_poll(void) { if (j.install != NULL) { j.install(); n++; } /* After the install, so a thunk sees the bodies its own module published. */ if (j.call != NULL) { j.call(); } + if (j.handle != NULL) { dlclose(j.handle); } } atomic_store_explicit(&tail, t, memory_order_relaxed); return n; @@ -160,6 +165,9 @@ static void serve(int fd) { if (f == NULL) { 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 @@ -167,7 +175,7 @@ static void serve(int fd) { 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 }); + publish((job){ f, c, transient == NULL ? NULL : h }); return; } }