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; } }