From 6bc329456ab7b42fe020e4f02499e622fc36bb4e Mon Sep 17 00:00:00 2001 From: Daniel Samson Date: Mon, 20 Jul 2026 23:42:00 +0100 Subject: [PATCH] =?UTF-8?q?threads(M9):=20thread=5Fjoin=20syscall=20?= =?UTF-8?q?=E2=80=94=20retire=20the=20per-thread=20endpoint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit join no longer needs a per-thread IPC endpoint. New thread_join(tid) syscall blocks the caller until the task with id tid exits; the exit paths call wakeJoinersLocked. join only reclaims the joined thread's USER stack, which the thread vacates the instant it enters the kernel to exit, so waking at exit time (not reap time) is safe — no reaper/aspace juggling or user-memory write, and equally std-shaped (like pthread_join). thread_spawn drops the exit-endpoint arg (runtime passes no_cap). thread-test's join mode runs 40 spawn+join cycles that would exhaust the 16-slot handle table under the old endpoint scheme. Also harden the M8 reaper: its single per-core reap slot could be overwritten by a second death on that core before draining (a fresh-task/SMP timing window), an intermittent one-stack leak that made task-reap ~20% flaky. Replace it with a per-core reap LIST plus a .reaping task state so a pending slot can't be reused before its stack is freed. task-reap now 11/11 isolated. Deferred: detached-thread user-stack reclaim (still at process exit, as in M3). Gate thread-join PASS (3x); full guardrail 26/26; build + host tests clean. --- docs/threading-plan.md | 34 ++++-- library/runtime/thread.zig | 23 ++-- system/abi.zig | 1 + system/kernel/process.zig | 14 +++ system/kernel/scheduler.zig | 115 +++++++++++++++----- system/kernel/tests.zig | 6 +- system/services/thread-test/thread-test.zig | 16 ++- 7 files changed, 153 insertions(+), 56 deletions(-) diff --git a/docs/threading-plan.md b/docs/threading-plan.md index 9ce1cd7..6d50449 100644 --- a/docs/threading-plan.md +++ b/docs/threading-plan.md @@ -370,16 +370,32 @@ full guardrail); `zig build`/`zig build test` clean. With the reaper (M8) able to act *after* a thread is fully off its stack, migrate `join` to the std shape and drop M3's per-thread exit endpoint: -- [ ] `thread_spawn` takes a user **completion word** (in the `Thread` handle's memory) - and a joinable/detached flag. On reap the kernel writes 0 to that word and - `futex_wake`s it (a `CLONE_CHILD_CLEARTID` equivalent — safe now the thread is off - its stack). `join` = `futex_wait` on the word, then `munmap` the stack; a - **detached** thread's stack is `munmap`ped by the reaper instead. No IPC endpoint - per thread. -- [ ] `thread-join` passes on the new path; a check confirms joining N threads creates no - per-thread endpoints (handle count stable). +- [x] A **`thread_join(tid)` syscall** (not a user futex word): it blocks the caller until + the task with id `tid` exits, and the exit paths call `wakeJoinersLocked`. `join` + only reclaims the joined thread's **user** stack, which the thread vacates the moment + it enters the kernel to exit — so waking at *exit* time (not reap time) is safe, and + no reaper/address-space juggling or user-memory write is needed. This is equally + std-shaped (like `pthread_join`) and much simpler/safer than the planned + reaper-written completion word. `thread_spawn` no longer takes an exit endpoint (the + runtime passes `no_cap`); the per-thread IPC endpoint is gone. +- [x] `thread-join` passes on the new path, and its join mode now runs **40 spawn+join + cycles** — under the old per-thread-endpoint scheme those leaked handles would + exhaust the 16-slot handle table; here they all succeed, proving join is endpoint-free. -**Gate:** `thread-join`/`thread-mutex` green on futex-completion join; guardrail green. +**Gate (met):** `thread-join` passes (3× isolated) on the `thread_join` path; full +guardrail 26/26 (incl. `process-kill`, `supervision`, `fault-recovery`, `task-reap`); +`zig build`/`zig build test` clean. + +> **Reaper hardened here (fixes an M8 flake).** M8's single per-core reap slot could be +> *overwritten* by a second death on that core before the first drained (a fresh-task/SMP +> timing window) — an intermittent one-stack leak (`task-reap` flaked ~20%). Replaced it +> with a per-core reap **list** plus a `.reaping` task state so a pending slot can't be +> reused before its stack is freed. `task-reap` now 11/11 isolated + 2× in the batch. + +> **Deferred:** detached-thread **user-stack** reclaim (still freed at process exit, as in +> M3). Doing it in the reaper needs the saved address space + stack range and a +> translate/unmap in a not-currently-loaded aspace — real complexity for a bounded leak. +> A follow-up when a consumer needs it. ### M10 — Per-thread TLS (`threadlocal`) diff --git a/library/runtime/thread.zig b/library/runtime/thread.zig index 2f84ebb..c34d84b 100644 --- a/library/runtime/thread.zig +++ b/library/runtime/thread.zig @@ -14,16 +14,13 @@ const std = @import("std"); const abi = @import("abi"); const sc = @import("system-call.zig"); const system = @import("system.zig"); -const ipc = @import("ipc.zig"); /// A thread stack, if the caller does not override it. 64 KiB of mmap'd, zeroed pages. pub const default_stack_size: usize = 64 * 1024; pub const Thread = struct { - /// The kernel task id of the spawned thread. + /// The kernel task id of the spawned thread — what `join` waits on. tid: u32, - /// The endpoint the kernel notifies when this thread ends — what `join` blocks on. - exit_endpoint: ipc.Handle, /// The mmap'd stack, reclaimed by `join` (or at process exit after `detach`). stack_base: usize, stack_size: usize, @@ -56,9 +53,6 @@ pub const Thread = struct { } }; - // The endpoint the kernel posts this thread's exit notification to. - const endpoint = ipc.createIpcEndpoint() orelse return error.SystemResources; - const base = system.mmap(config.stack_size, system.PROT_READ | system.PROT_WRITE); if (system.mmapFailed(base)) return error.SystemResources; @@ -73,24 +67,20 @@ pub const Thread = struct { var stack_top = closure_addr & ~@as(usize, 15); // 16-align below the closure stack_top -= 8; // ...then rsp % 16 == 8 at the C entry - const tid = threadSpawn(@intFromPtr(&Closure.entry), stack_top, closure_addr, endpoint); + const tid = threadSpawn(@intFromPtr(&Closure.entry), stack_top, closure_addr); if (threadSpawnFailed(tid)) { _ = system.munmap(base, config.stack_size); return error.SystemResources; } - return .{ .tid = @intCast(tid), .exit_endpoint = endpoint, .stack_base = base, .stack_size = config.stack_size }; + return .{ .tid = @intCast(tid), .stack_base = base, .stack_size = config.stack_size }; } /// Block until this thread finishes, then reclaim its stack. Mirrors /// `std.Thread.join`. The exit endpoint is private to this thread, so the first /// child-exit notification on it is this thread's. pub fn join(self: Thread) void { - var receive: [0]u8 = undefined; - while (true) { - const got = ipc.replyWait(self.exit_endpoint, &.{}, &receive, null); - if (got.isChildExit() and got.childProcessId() == self.tid) break; - } - _ = system.munmap(self.stack_base, self.stack_size); + _ = sc.systemCall1(.thread_join, self.tid); // block until the thread has exited + _ = system.munmap(self.stack_base, self.stack_size); // reclaim its (now-vacated) stack } /// Relinquish the right to join: never wait for or reclaim this thread. Its stack is @@ -233,7 +223,8 @@ pub const Thread = struct { }; /// thread_spawn(entry, stack_top, arg, exit_endpoint) -> tid, or a wrapped error. -fn threadSpawn(entry: usize, stack_top: usize, arg: usize, exit_endpoint: ipc.Handle) usize { +fn threadSpawn(entry: usize, stack_top: usize, arg: usize) usize { + const exit_endpoint: usize = @intCast(abi.no_cap); // join uses thread_join, not an endpoint return sc.systemCall4(.thread_spawn, entry, stack_top, arg, exit_endpoint); } diff --git a/system/abi.zig b/system/abi.zig index d536237..b959593 100644 --- a/system/abi.zig +++ b/system/abi.zig @@ -69,6 +69,7 @@ pub const SystemCall = enum(u64) { futex_wait = 40, // futex_wait(addr, expected, timeout_ns) -> status: if *addr == expected, block until woken or the timeout; returns futex_woken/mismatch/timed_out (docs/threading.md) futex_wake = 41, // futex_wake(addr, count) -> woken: wake up to `count` tasks blocked in futex_wait on `addr` in this address space thread_self = 42, // thread_self() -> tid: the calling thread's kernel task id (runtime.Thread.getCurrentId) + thread_join = 43, // thread_join(tid) -> 0: block until the thread with id `tid` has exited (runtime.Thread.join; no per-thread IPC endpoint) (docs/threading.md) _, }; diff --git a/system/kernel/process.zig b/system/kernel/process.zig index 6483a6d..cc328eb 100644 --- a/system/kernel/process.zig +++ b/system/kernel/process.zig @@ -231,6 +231,7 @@ fn system_call(state: *architecture.CpuState) void { .thread_spawn => systemThreadSpawn(state), .current_core => systemCurrentCore(state), .thread_self => systemThreadSelf(state), + .thread_join => systemThreadJoin(state), .futex_wait => systemFutexWait(state), .futex_wake => systemFutexWake(state), .thread_exit => { @@ -706,6 +707,19 @@ fn systemThreadSelf(state: *architecture.CpuState) void { architecture.setSystemCallResult(state, scheduler.currentId()); } +/// thread_join(tid) -> 0: block until the thread with id `tid` has exited (docs/threading- +/// plan.md M9). Needs no per-thread IPC endpoint. The compare-and-block is one critical +/// section, so an exit cannot slip between "is it alive?" and the block. +fn systemThreadJoin(state: *architecture.CpuState) void { + const tid: u32 = @truncate(architecture.systemCallArg(state, 0)); + const t = scheduler.current(); + if (t.aspace == 0) return fail(state); // kernel tasks don't join + const flags = sync.enter(); + scheduler.joinThreadLocked(tid); + sync.leave(flags); + architecture.setSystemCallResult(state, 0); +} + /// futex_wait(addr, expected, timeout_ns) -> status (docs/threading.md): if the 4-byte /// user word at `addr` still equals `expected`, block until a futex_wake on `addr` or /// (if timeout_ns > 0) the deadline. The compare and the block are one critical section, diff --git a/system/kernel/scheduler.zig b/system/kernel/scheduler.zig index daf775f..d4d45b5 100644 --- a/system/kernel/scheduler.zig +++ b/system/kernel/scheduler.zig @@ -31,7 +31,10 @@ const number_priorities = 8; const stack_size = parameters.kernel_stack_size; // each task's kernel stack const maximum_tasks = parameters.maximum_tasks; // maximum tasks alive at once (static pool) -const State = enum { free, ready, running, blocked }; +// `reaping` = the task has exited and is queued on its core's reap list; its slot must not +// be reused (freeSlot skips it) until the reaper has freed its kernel stack and set it +// `free` (docs/threading-plan.md M8/M9). +const State = enum { free, ready, running, blocked, reaping }; pub const Task = struct { id: u32 = 0, @@ -83,6 +86,9 @@ pub const Task = struct { // The user address this task is blocked on in futex_wait (0 = not futex-waiting). // Cleared to 0 by futexWakeLocked as the "woken, not timed out" signal (docs/threading.md). futex_addr: u64 = 0, + // The task id this task is blocked in `thread_join` on (0 = not joining). Woken by + // `wakeJoinersLocked` when that task exits (docs/threading-plan.md M9). + join_target: u32 = 0, // The mmap / MMIO grant-arena cursors moved from Task to the per-address-space object // (`AspaceRef`, below) so threads sharing one address space hand out disjoint grants // — see aspaceMmapNextPtr / aspaceDeviceMapNextPtr (docs/threading-plan.md M7). @@ -174,6 +180,20 @@ fn reapStackLocked(t: *Task) void { t.kstack_top = 0; } +/// Free every `.reaping` task queued on this core's reap list and mark each `.free` (now +/// its slot may be reused). The tasks are all off their stacks (they switched away), and +/// the caller holds the lock, so freeing is safe (docs/threading-plan.md M8/M9). +fn drainReapListLocked(pc: *PerCpu) void { + var node = pc.reap_list; + pc.reap_list = null; + while (node) |t| { + node = t.next; // save the link before we clear it + t.next = null; + reapStackLocked(t); + t.state = .free; // reusable only now, after the stack is freed + } +} + /// Take a reference to address space `root` (0 = a kernel task, which owns none). /// Returns false only if the ref table is full — bounded by `maximum_tasks`, so in /// practice it never is. Caller holds the kernel lock. @@ -268,12 +288,12 @@ pub const PerCpu = struct { pinned_head: [number_priorities]?*Task = .{null} ** number_priorities, pinned_tail: [number_priorities]?*Task = .{null} ** number_priorities, pinned_bitmap: u8 = 0, - // A task that ended while running on THIS core: it could not free the kernel stack it - // was standing on, so it recorded itself here and switched away. The next task to run - // on this core frees that stack (from its own stack, safely) in `switchTo`. The big - // lock is held continuously across the switch, so the dead task's slot can't be reused - // before it is reaped (docs/threading-plan.md M8). - reap_after_switch: ?*Task = null, + // Tasks that ended while running on THIS core: they could not free the kernel stack + // they were standing on, so each pushed itself onto this list (`.reaping` state, linked + // via `Task.next`) and switched away. The next task to run on this core — or the timer + // tick — frees their stacks from its own stack, safely (docs/threading-plan.md M8). A + // *list* (not one slot) so a second death before the first is drained can't lose it. + reap_list: ?*Task = null, }; const maximum_cpus = parameters.maximum_cpus; @@ -555,11 +575,7 @@ fn switchTo(pc: *PerCpu, save_sp: *usize, next: *Task) void { // the current core, so thisCpu() is the core the just-dead task died on. If a task // died switching to us, free its kernel stack: we're on ours so it's safe, and the big // lock is still held so its slot can't have been reused (docs/threading-plan.md M8). - const here = thisCpu(); - if (here.reap_after_switch) |dead| { - here.reap_after_switch = null; - reapStackLocked(dead); - } + drainReapListLocked(thisCpu()); } /// Voluntarily give up the CPU to the next ready task. @@ -623,6 +639,46 @@ pub fn futexWakeLocked(aspace: u64, addr: u64, count: u32) u32 { return woken; } +// --- thread join (docs/threading-plan.md M9) -------------------------------- +// +// join needs no per-thread IPC endpoint: `thread_join(tid)` blocks the caller until the +// task with id `tid` has exited, and the exit paths wake any joiner. The caller only ever +// reclaims the joined thread's *user* stack (which the thread vacated the moment it entered +// the kernel to exit), so waking at exit time — not reap time — is safe. + +/// True if a task with id `tid` is still live (has not exited). Caller holds the lock. +fn aliveTid(tid: u32) bool { + for (&tasks) |*t| { + if (t.id == tid and t.state != .free and t.state != .reaping) return true; + } + return false; +} + +/// Block the current task until the task with id `tid` exits (or return at once if it +/// already has / never existed). **Precondition:** the big kernel lock is held; returns +/// with it still held. Woken by `wakeJoinersLocked`. +pub fn joinThreadLocked(tid: u32) void { + while (aliveTid(tid)) { + const t = current(); + t.join_target = tid; + t.state = .blocked; + schedule(); // woken when the joined task exits; lock handed off across the switch + t.join_target = 0; + } +} + +/// Wake every task blocked in `thread_join` on `tid` — called from the exit paths once the +/// exiting task's state is `.free`. Caller holds the lock. +fn wakeJoinersLocked(tid: u32) void { + for (&tasks) |*t| { + if (t.state == .blocked and t.join_target == tid) { + t.join_target = 0; + t.state = .ready; + enqueue(t); + } + } +} + // --- event-based blocking ------------------------------------------------- // // A WaitQueue is a set of tasks blocked waiting for something (a resource, a @@ -724,7 +780,7 @@ fn removeFrom(head: *[number_priorities]?*Task, tail: *[number_priorities]?*Task /// Precondition: the big kernel lock is held. pub fn taskByIdLocked(id: u32) ?*Task { for (&tasks) |*t| { - if (t.state != .free and t.id == id) return t; + if (t.state != .free and t.state != .reaping and t.id == id) return t; } return null; } @@ -734,7 +790,7 @@ pub fn taskByIdLocked(id: u32) ?*Task { /// Precondition: the big kernel lock is held. pub fn forgetIpcClientLocked(t: *Task) void { for (&tasks) |*other| { - if (other.state != .free and other.ipc_client == t) other.ipc_client = null; + if (other.state != .free and other.state != .reaping and other.ipc_client == t) other.ipc_client = null; } } @@ -826,14 +882,10 @@ pub var reap_task_hook: ?*const fn (*Task) void = null; fn reapKillPendingLocked() void { const pc = thisCpu(); const cur = pc.current; - // Safety net for the reap-after-switch slot: if a dying task switched to a *fresh* - // task (which enters via task_trampoline, not switchTo's tail), its kernel stack is - // still pending here. The dying task switched away before this tick, so it is off its - // stack — reap it now (docs/threading-plan.md M8). - if (pc.reap_after_switch) |dead| { - pc.reap_after_switch = null; - reapStackLocked(dead); - } + // Safety net: if a dying task switched to a *fresh* task (which enters via + // task_trampoline, not switchTo's tail), its stack is still queued here. The dying + // task switched away before this tick, so it is off its stack — drain now (M8). + drainReapListLocked(pc); if (cur.kill_pending and cur.aspace != 0 and !cur.in_system_call) { if (terminate_current_hook) |hook| hook(); // noreturn } @@ -875,8 +927,11 @@ pub fn setPreemption(enabled: bool) void { pub fn exit() noreturn { _ = sync.enter(); const pc = thisCpu(); - pc.current.state = .free; - pc.reap_after_switch = pc.current; // the task we switch to frees this stack (M8) + // Queue this task for reaping: `.reaping` keeps its slot out of freeSlot until its + // stack is freed; `next` links it on the core's reap list (M8/M9). + pc.current.state = .reaping; + pc.current.next = pc.reap_list; + pc.reap_list = pc.current; const next = dequeueHighest(pc) orelse @panic("sched: no task left to run"); next.state = .running; pc.current = next; @@ -908,11 +963,14 @@ pub fn exitUserLocked() noreturn { pc.loaded_aspace = kroot; releaseAspace(as); // destroys only when this was the last task on the space } - dying.state = .free; + dying.state = .reaping; // dead but its slot stays reserved until the stack is freed + wakeJoinersLocked(dying.id); // let any thread_join(dying.id) return (M9) dying.aspace = 0; dying.kill_pending = false; dying.in_system_call = false; - pc.reap_after_switch = dying; // the task we switch to frees this stack (M8) + // Queue for reaping: the task we switch to (or the next tick) frees this stack (M8/M9). + dying.next = pc.reap_list; + pc.reap_list = dying; const next = dequeueHighest(pc) orelse @panic("sched: no task left to run"); next.state = .running; pc.current = next; @@ -935,6 +993,7 @@ pub fn destroyTaskLocked(t: *Task) void { t.in_system_call = false; t.wake_at = 0; t.state = .free; + wakeJoinersLocked(t.id); // a killed thread's joiners must return too (M9) } /// Snapshot the task table into `out` (up to its length), returning the total @@ -948,7 +1007,7 @@ pub fn enumerate(out: []abi.ProcessDescriptor) u64 { defer sync.leave(flags); var total: u64 = 0; for (&tasks) |*t| { - if (t.state == .free) continue; + if (t.state == .free or t.state == .reaping) continue; // reaping = already exited if (total < out.len) { const d = &out[total]; d.* = .{ @@ -958,7 +1017,7 @@ pub fn enumerate(out: []abi.ProcessDescriptor) u64 { .ready => .ready, .running => .running, .blocked => .blocked, - .free => unreachable, + .free, .reaping => unreachable, })), .priority = t.priority, .name_length = t.name_length, diff --git a/system/kernel/tests.zig b/system/kernel/tests.zig index 5832ffa..3afab59 100644 --- a/system/kernel/tests.zig +++ b/system/kernel/tests.zig @@ -1769,9 +1769,11 @@ fn taskReapTest(boot_information: *const BootInformation) void { while (scheduler.liveStackBytes() != base and architecture.millis() < settle_deadline) scheduler.yield(); scheduler.setPriority(4); + const final = scheduler.liveStackBytes(); + log("task-reap: base={d} final={d} killed={d}/{d}\n", .{ base, final, killed, rounds }); check("all probes spawned and were killed", killed == rounds); - check("kernel stacks reclaimed to baseline (no leak)", scheduler.liveStackBytes() == base); - if (killed == rounds and scheduler.liveStackBytes() == base) + check("kernel stacks reclaimed to baseline (no leak)", final == base); + if (killed == rounds and final == base) log("task-reap: kernel stacks reclaimed to baseline ok\n", .{}); result(); } diff --git a/system/services/thread-test/thread-test.zig b/system/services/thread-test/thread-test.zig index 6cf7c7b..d62b35a 100644 --- a/system/services/thread-test/thread-test.zig +++ b/system/services/thread-test/thread-test.zig @@ -74,6 +74,8 @@ fn detachWorker() void { detach_done.store(1, .release); } +fn noopWorker() void {} + fn runJoinMode() void { write("thread-test: join mode starting\n"); @@ -114,7 +116,19 @@ fn runJoinMode() void { return; } - write("thread-test: join ok\n"); // the M3 verdict marker + // Prove join needs no per-thread kernel endpoint (M9): many spawn+join cycles. Under + // the old per-thread-endpoint scheme these leaked handles and would exhaust the + // 16-slot handle table well before 40; here they all succeed. + var cycle: u32 = 0; + while (cycle < 40) : (cycle += 1) { + const th = runtime.Thread.spawn(.{}, noopWorker, .{}) catch { + write("thread-test: FAIL spawn exhausted across join cycles (endpoint leak?)\n"); + return; + }; + th.join(); + } + + write("thread-test: join ok\n"); // the M3/M9 verdict marker } // --- M4: futex mode ---------------------------------------------------------