From d5fc3aa657f47ce43902d4d3a35573f7bd47e7e5 Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Fri, 25 Sep 2026 15:18:55 +0700 Subject: [PATCH] The agent binds the daemon's socket only in the process the daemon launched, named by FLAN_AGENT_OWNER --- README.md | 6 +-- TODO.org | 7 ---- lib/dev.ml | 7 ++++ test/test_agent.ml | 78 ++++++++++++++++++++++++++++++++++++--- vendor/agent/flan_agent.c | 49 +++++++++++++++++------- 5 files changed, 119 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 0ed952ca..873ef788 100644 --- a/README.md +++ b/README.md @@ -262,9 +262,9 @@ driver at all — it goes `llc` + `ld -shared` + `dlopen`, which is what makes is a confusing shape of failure to meet without warning. Variables beginning `FLAN_DEV_` other than `FLAN_DEV_LEAKS`, plus -`FLAN_AGENT_SOCKET` and `FLAN_COMPILER_STAMP`, are internal: `flan dev` sets -them across its own `exec` to hand the merged binary what it needs. Setting -them by hand is not supported. +`FLAN_AGENT_SOCKET`, `FLAN_AGENT_OWNER` and `FLAN_COMPILER_STAMP`, are +internal: `flan dev` sets them across its own `exec` to hand the merged binary +what it needs. Setting them by hand is not supported. ## Checking it diff --git a/TODO.org b/TODO.org index 899c9b6e..aa50db50 100644 --- a/TODO.org +++ b/TODO.org @@ -1637,13 +1637,6 @@ prunes a package nothing calls into in a release build. =flan dev= links the agent's C into every program it builds whether or not the source imports it; a release build links it only when the program calls into it. -** NEXT FLAN_AGENT_SOCKET in a shell's environment steals the socket -Decided 2026-09-25: narrow the gate. The daemon also exports its own pid, and the constructor binds the socket only when that pid is the program's parent (or the program itself, in a merged build). -Binding unlinks the path first, and before the constructor that unlink was reached -only by an explicit call. A sentence about the shape of the gate rather than an -observed problem: only the daemon sets the variable and it never runs release -builds. The fix, if it is ever felt, is a narrower gate. - ** DONE The daemon's "has not called (agent/start ...)" note is unreachable CLOSED: [2026-09-25] Retired, with the matching arm of an evaluation's timeout, because it named the diff --git a/lib/dev.ml b/lib/dev.ml index 7f052f25..ccbe8084 100644 --- a/lib/dev.ml +++ b/lib/dev.ml @@ -5251,6 +5251,10 @@ let two_process ?(debug = false) ?(x86 = true) ~file ~sock () = environment. Guessing instead would fail silently — everything compiles, the module is built, and nothing ever receives it. *) Unix.putenv "FLAN_AGENT_SOCKET" agent; + (* The child binds that path only if this pid is its parent, so a process + that merely inherited the variable leaves the socket alone + (flan_agent.c, [daemon_socket]). *) + Unix.putenv "FLAN_AGENT_OWNER" (string_of_int (Unix.getpid ())); (* And who its parent is, which is the child's licence to end itself. vendor/agent/flan_agent.c carries the argument at length; the half that belongs here is that this daemon is the only thing that ever kills its @@ -6264,6 +6268,9 @@ let start_merged ?(debug = false) ?(x86 = true) ~file ~sock () = that the game thread's [getenv] cannot race the compiler thread's [putenv]: there is no ordering left to get wrong. *) Unix.putenv "FLAN_AGENT_SOCKET" agent; + (* This pid, because the exec below keeps it: the program is the owner + flan_agent.c's [daemon_socket] looks for. *) + Unix.putenv "FLAN_AGENT_OWNER" (string_of_int (Unix.getpid ())); Unix.putenv "FLAN_DEV_SOURCE" file; Unix.putenv "FLAN_DEV_SOCK" sock; Unix.putenv "FLAN_DEV_DIR" dir; diff --git a/test/test_agent.ml b/test/test_agent.ml index def7521c..f4847b28 100644 --- a/test/test_agent.ml +++ b/test/test_agent.ml @@ -61,6 +61,13 @@ let send path line = Unix.close s; Buffer.contents b +(* The pair the daemon sets: the path, and the pid it is meant for. This test + binary is the parent of every program it starts, which is the + --two-process shape. *) +let daemon_env path = + [| "FLAN_AGENT_SOCKET=" ^ path; + "FLAN_AGENT_OWNER=" ^ string_of_int (Unix.getpid ()) |] + let () = match Sys.command "command -v clang > /dev/null 2>&1 && command -v llc > /dev/null 2>&1" with | 0 -> @@ -298,7 +305,7 @@ let () = let bad = tmp "bad.out" in let bfd' = ofd bad in let benv = - Array.append aenv [| "FLAN_AGENT_SOCKET=/nonexistent-dir/agent.sock" |] + Array.append aenv (daemon_env "/nonexistent-dir/agent.sock") in let bpid' = Unix.create_process_env aexe [| aexe |] benv Unix.stdin bfd' bfd' in Unix.close bfd'; @@ -325,8 +332,69 @@ let () = "cannot listen\n" end; + (* ── The variable inherited by a process the daemon did not start ── *) + + (* A shell opened from inside a [flan dev] program carries its + FLAN_AGENT_SOCKET, and so does anything run from that shell. Binding + unlinks the path first, so honouring it there would take the session's + socket from its program. FLAN_AGENT_OWNER names the process the daemon + launched; pid 1 is neither this program nor its parent, so the variable + is not this program's, and it picks and announces a path of its own as + if nothing were set. The file standing in for the session's socket has + to still be the same file afterwards. *) + let stolen = tmp "stolen.sock" and serr = tmp "stolen.err" in + Out_channel.with_open_bin stolen (fun oc -> + output_string oc "the session's"); + let senv = + Array.append aenv + [| "FLAN_AGENT_SOCKET=" ^ stolen; "FLAN_AGENT_OWNER=1" |] + in + let s1 = ofd (tmp "stolen.out") and s2 = ofd serr in + let spid = Unix.create_process_env aexe [| aexe |] senv Unix.stdin s1 s2 in + Unix.close s1; + Unix.close s2; + let prefix = "flan agent: listening on " in + let sannounced () = + let text = In_channel.with_open_bin serr In_channel.input_all in + List.find_map + (fun l -> + if String.length l > String.length prefix + && String.sub l 0 (String.length prefix) = prefix + then Some (String.sub l (String.length prefix) + (String.length l - String.length prefix)) + else None) + (String.split_on_char '\n' text) + in + (match + if await (fun () -> sannounced () <> None) then sannounced () else None + with + | None -> + fail "a program with someone else's FLAN_AGENT_SOCKET announced no \ + socket of its own"; + (try Unix.kill spid Sys.sigkill with Unix.Unix_error _ -> ()) + | Some p -> + if p = stolen then fail "the inherited path was bound: %S" p; + if not (await (fun () -> Sys.file_exists p)) then + fail "nothing was bound at the announced %S" p + else ignore (send p aso); + let reaped = + await ~ms:5000 (fun () -> + match Unix.waitpid [ Unix.WNOHANG ] spid with + | 0, _ -> false + | _ -> true) + in + if not reaped then begin + (try Unix.kill spid Sys.sigkill with Unix.Unix_error _ -> ()); + fail "the program with an inherited variable never finished" + end); + (match In_channel.with_open_bin stolen In_channel.input_all with + | "the session's" -> () + | _ -> fail "the inherited FLAN_AGENT_SOCKET's file was replaced" + | exception Sys_error _ -> + fail "the inherited FLAN_AGENT_SOCKET's file was removed"); + List.iter (fun f -> try Sys.remove f with Sys_error _ -> ()) - [ aexe; aso; aout; aerr; bad ]; + [ aexe; aso; aout; aerr; bad; stolen; serr; tmp "stolen.out" ]; (* ── No (agent/start) at all ────────────────────────────────────── *) @@ -359,7 +427,7 @@ let () = let nc = Session.eval nt "(defn tick [] i64 1000)" in ignore (Build.shared ~opts:dev ~ir:nc.Session.ir ~out:nso ()); let nenv = - Array.append (Unix.environment ()) [| "FLAN_AGENT_SOCKET=" ^ nsock |] + Array.append (Unix.environment ()) (daemon_env nsock) in let nfd = Unix.openfile nout [ Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC ] 0o600 @@ -412,7 +480,7 @@ let () = bt.Session.host ~out:bexe); let bfd = Unix.openfile bout [ Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC ] 0o600 in let env = - Array.append (Unix.environment ()) [| "FLAN_AGENT_SOCKET=" ^ bsock |] + Array.append (Unix.environment ()) (daemon_env bsock) in let bpid = Unix.create_process_env bexe [| bexe |] env Unix.stdin bfd bfd @@ -753,7 +821,7 @@ let () = 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 |] + Array.append (Unix.environment ()) (daemon_env lsock) in let lpid = Unix.create_process_env lexe [| lexe |] lenv Unix.stdin lfd lfd in Unix.close lfd; diff --git a/vendor/agent/flan_agent.c b/vendor/agent/flan_agent.c index 47aae148..efe49f02 100644 --- a/vendor/agent/flan_agent.c +++ b/vendor/agent/flan_agent.c @@ -2459,17 +2459,40 @@ failed: return -1; } +/* The daemon's socket for this process, or NULL when there is none. + * + * FLAN_AGENT_SOCKET alone is not enough, because an environment is inherited: + * a shell started from inside a [flan dev] program, or anything that program + * starts, carries it too, and binding unlinks the path first, so such a + * process would take the session's socket from the program it belongs to. So + * the daemon also names the process it launched, in FLAN_AGENT_OWNER, and the + * path is honoured only there: the owner is this process in a merged build, + * where the launcher execs into the program, and this process's parent under + * --two-process, where the daemon started it. */ +static const char *daemon_socket(void) { + const char *env = getenv("FLAN_AGENT_SOCKET"); + const char *own = getenv("FLAN_AGENT_OWNER"); + char *end; + long pid; + if (env == NULL || env[0] == '\0' || own == NULL || own[0] == '\0') + return NULL; + pid = strtol(own, &end, 10); + if (end == own || *end != '\0' || pid <= 0) return NULL; + if (pid != (long)getpid() && pid != (long)getppid()) return NULL; + return env; +} + /* [path] is a Flan string: ptr and len, not NUL-terminated. * - * FLAN_AGENT_SOCKET overrides it. A program's source has to name some path, - * and the daemon that launches the program is the one that knows where it + * The daemon's socket overrides it (see [daemon_socket]). A program's source + * has to name some path, and the daemon that launches the program is the one that knows where it * wants to talk to it — without the override the daemon would have to guess, * and guessing wrong fails silently: everything compiles, the module is built, * and nothing ever receives it. */ int32_t flan_agent_start(const uint8_t *path, int64_t len) { char buf[sizeof(((struct sockaddr_un *)0)->sun_path)]; - const char *env = getenv("FLAN_AGENT_SOCKET"); - if (env != NULL && env[0] != '\0') return start_on(env) < 0 ? -1 : 0; + const char *env = daemon_socket(); + if (env != NULL) return start_on(env) < 0 ? -1 : 0; if (len <= 0 || (size_t)len >= sizeof buf) return -1; memcpy(buf, path, (size_t)len); buf[len] = '\0'; @@ -2493,8 +2516,8 @@ int32_t flan_agent_start_auto(void) { char path[sizeof(((struct sockaddr_un *)0)->sun_path)]; struct timespec ts; int32_t r; - const char *env = getenv("FLAN_AGENT_SOCKET"); - if (env != NULL && env[0] != '\0') return start_on(env) < 0 ? -1 : 0; + const char *env = daemon_socket(); + if (env != NULL) return start_on(env) < 0 ? -1 : 0; if (clock_gettime(CLOCK_REALTIME, &ts) != 0) ts.tv_nsec = 0; snprintf(path, sizeof path, "/tmp/flan-agent-%ld-%08lx.sock", (long)getpid(), (unsigned long)(ts.tv_nsec & 0xffffffffL)); @@ -2511,11 +2534,11 @@ int32_t flan_agent_start_auto(void) { /* And the call itself, gone. A program under [flan dev] that imports this * package gets the listener before main, without asking. * - * FLAN_AGENT_SOCKET is the whole condition, and it is the right one: the - * daemon sets it in both shapes — before the fork in --two-process, before the - * exec in the merged build — and nothing else on a machine sets it. So an - * ordinary run of an ordinary program falls straight through here and this - * costs it one getenv. (Not FLAN_DEV_PARENT, which is deliberately unset in + * [daemon_socket] is the condition: the daemon sets both of its variables in + * both shapes — before the fork in --two-process, before the exec in the + * merged build — so an ordinary run of an ordinary program falls straight + * through here, and so does a process that only inherited them. (Not + * FLAN_DEV_PARENT, which is deliberately unset in * the merged build; gating on it would quietly skip half the daemon.) * * WHAT THIS DOES NOT REACH, because it is a fact about linking rather than a @@ -2540,8 +2563,8 @@ int32_t flan_agent_start_auto(void) { * cannot, in either shape — it has an editor to hear from first, and a module * to compile after that. */ __attribute__((constructor)) static void auto_start(void) { - const char *env = getenv("FLAN_AGENT_SOCKET"); - if (env == NULL || env[0] == '\0') return; + const char *env = daemon_socket(); + if (env == NULL) return; /* The answer is dropped because there is nobody to give it to: this is ELF * init, before main, before the program has decided anything. What matters * is that a failure here is not final — [start_on] gives [started] back, so