diff --git a/docs/threading-plan.md b/docs/threading-plan.md index c8f255a..4843167 100644 --- a/docs/threading-plan.md +++ b/docs/threading-plan.md @@ -94,24 +94,31 @@ first unchecked box. --- -## M1 — Address-space refcount (kernel foundation, no API, no behaviour change) +## M1 — Address-space refcount (kernel foundation, no API, no behaviour change) ✅ The one invariant change threads require, landed and proven **before** anything shares an address space. Today aspace is 1:1 with a task and teardown destroys it on any user task's exit; make destruction happen on the **last** exit. -- [ ] A refcount keyed by the address-space root: `createAddressSpace` - ([process.zig](../system/kernel/process.zig)) sets it to 1; a helper - `retainAspace`/`releaseAspace` adjusts it under the big kernel lock. -- [ ] Task teardown ([scheduler.zig](../system/kernel/scheduler.zig), the - `destroyAddressSpace(t.aspace)` path) decrements and only destroys at **zero**. -- [ ] `-Dtest-case=aspace-refcount`: spawn and exit several processes in sequence and - assert the frame allocator's free count returns to the pre-spawn **baseline** - (each aspace destroyed exactly once — no leak, no double-free). +- [x] A refcount keyed by the address-space root, held in `scheduler.zig` + (`aspace_refs`): `retainAspace` takes a reference in `spawnUserLocked` (on the + success path, after the slot + stack are secured), all under the big kernel lock. +- [x] Both task-teardown paths ([scheduler.zig](../system/kernel/scheduler.zig): + `exitUserLocked` and `destroyTaskLocked`) call `releaseAspace`, which decrements + and only `destroyAddressSpace`s at **zero**; an unretained space (hand-built test + spaces) is destroyed directly, preserving prior behaviour. +- [x] `-Dtest-case=aspace-refcount`: spawn and reap several ring-3 processes in sequence + and assert (via test-observable `liveAspaceCount`/`aspaceDestroyCount`) that the + live-space count returns to **baseline** and destructions advance by exactly that + many — each space destroyed exactly once, no leak, no double-free. (Refcount + observables, not raw frame counts, since kernel stacks are still leaked on exit.) -**Gate:** `python3 test/qemu_test.py aspace-refcount` logs `aspace-refcount: frames -reclaimed to baseline ok`, and the full guardrail set passes unchanged (the reframing -is invisible until an aspace is actually shared). +**Gate (met):** `python3 test/qemu_test.py aspace-refcount` passes +(`aspace-refcount: spaces released to baseline ok` → `DANOS-TEST-RESULT: PASS`), and the +full guardrail set passes unchanged — 13/13 (`smoke`, `sched`, `priority`, `smp`, +`affinity`, `process`, `process-kill`, `supervision`, `fault-recovery`, +`vfs-client-death`, `ipc`, `ipc-cap`, `display-service`); default `zig build` clean, +`zig build test` green. The reframing is invisible until an aspace is actually shared. ## M2 — `thread_spawn` + `thread_exit`: a thread runs in the shared address space diff --git a/system/kernel/scheduler.zig b/system/kernel/scheduler.zig index 533ba60..50de887 100644 --- a/system/kernel/scheduler.zig +++ b/system/kernel/scheduler.zig @@ -136,6 +136,67 @@ pub const ipc_maximum_handles = 16; pub const HandleObject = struct { kind: u8, ptr: *anyopaque }; var tasks = [_]Task{.{}} ** maximum_tasks; + +/// Address-space reference counts: one live entry per address space, counting the +/// tasks that share it. An address space is 1:1 with a process today; threads +/// (docs/threading.md) will push a count above 1, and `destroyAddressSpace` must run +/// only when the **last** task on an address space exits. All access is under the big +/// kernel lock. There can be no more live address spaces than tasks, so the table is +/// sized to the task pool and never overflows in practice. +const AspaceRef = struct { root: u64 = 0, count: u32 = 0 }; +var aspace_refs = [_]AspaceRef{.{}} ** maximum_tasks; +var aspace_destroy_count: u64 = 0; + +/// 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. +fn retainAspace(root: u64) bool { + if (root == 0) return true; + var free: ?*AspaceRef = null; + for (&aspace_refs) |*entry| { + if (entry.count != 0 and entry.root == root) { + entry.count += 1; + return true; + } + if (entry.count == 0 and free == null) free = entry; + } + const slot = free orelse return false; + slot.* = .{ .root = root, .count = 1 }; + return true; +} + +/// Drop a reference to `root`; destroy the address space when the **last** one drops. +/// A `root` with no entry — never retained, e.g. a hand-built test space — is +/// destroyed directly, preserving the pre-refcount behaviour. Caller holds the lock. +fn releaseAspace(root: u64) void { + if (root == 0) return; + for (&aspace_refs) |*entry| { + if (entry.count == 0 or entry.root != root) continue; + entry.count -= 1; + if (entry.count == 0) { + entry.root = 0; + architecture.destroyAddressSpace(root); + aspace_destroy_count += 1; + } + return; + } + architecture.destroyAddressSpace(root); + aspace_destroy_count += 1; +} + +/// Test-observable: how many address spaces are live (entries with a nonzero count). +pub fn liveAspaceCount() u32 { + var live: u32 = 0; + for (&aspace_refs) |*entry| { + if (entry.count != 0) live += 1; + } + return live; +} + +/// Test-observable: total address-space destructions since boot. +pub fn aspaceDestroyCount() u64 { + return aspace_destroy_count; +} var next_id: u32 = 1; /// Per-CPU scheduler state: the task each core is running, its own idle task, and a @@ -330,6 +391,12 @@ pub fn spawnOn(entry: *const fn () void, priority: Priority, cpu: u32) bool { pub fn spawnUserLocked(aspace: u64, entry: u64, user_sp: u64, priority: Priority, task_name: []const u8, supervisor: u32, exit_endpoint: ?*anyopaque) ?u32 { const t = freeSlot() orelse return null; const stack = heap.allocator().alloc(u8, stack_size) catch return null; + // Take this task's reference to the address space before we commit the slot, so a + // failure here leaves nothing to unwind (the caller still owns the raw `aspace`). + if (!retainAspace(aspace)) { + heap.allocator().free(stack); + return null; + } t.* = .{ .id = next_id, .state = .ready, @@ -720,7 +787,7 @@ pub fn exitUserLocked() noreturn { const kroot = architecture.kernelPageTable(); architecture.loadPageTable(kroot); // off the process tables before freeing them pc.loaded_aspace = kroot; - architecture.destroyAddressSpace(as); + releaseAspace(as); // destroys only when this was the last task on the space } dying.state = .free; dying.aspace = 0; @@ -741,7 +808,7 @@ pub fn exitUserLocked() noreturn { /// task isn't running). The kernel stack is leaked, as in `exitUser` (no reaper /// yet). Precondition: the big kernel lock is held. pub fn destroyTaskLocked(t: *Task) void { - if (t.aspace != 0) architecture.destroyAddressSpace(t.aspace); + if (t.aspace != 0) releaseAspace(t.aspace); // destroys only on the last reference t.aspace = 0; t.kill_pending = false; t.in_system_call = false; diff --git a/system/kernel/tests.zig b/system/kernel/tests.zig index 8811a1c..64fd8e9 100644 --- a/system/kernel/tests.zig +++ b/system/kernel/tests.zig @@ -139,6 +139,8 @@ pub fn run(case: []const u8, boot_information: *const BootInformation) void { userPfTest(); } else if (eql(case, "fault-recovery")) { faultRecoveryTest(boot_information); + } else if (eql(case, "aspace-refcount")) { + aspaceRefcountTest(boot_information); } else if (eql(case, "args")) { argsTest(boot_information); } else if (eql(case, "init")) { @@ -1429,6 +1431,41 @@ fn faultRecoveryTest(boot_information: *const BootInformation) void { result(); } +/// Address-space refcount (docs/threading-plan.md M1): every process holds exactly one +/// reference to its address space, released when it dies, so `destroyAddressSpace` runs +/// exactly once per space — no leak, no double-free. Spawn and kill several ring-3 +/// processes (the faulting probe, reaped by the kernel) and confirm the count of live +/// address spaces returns to baseline while destructions advance by exactly that many. +/// This is the foundation threads (shared address spaces) build on: the refactor must be +/// invisible while every space still has exactly one task. +fn aspaceRefcountTest(boot_information: *const BootInformation) void { + _ = boot_information; + log("DANOS-TEST-BEGIN: aspace-refcount\n", .{}); + const base_live = scheduler.liveAspaceCount(); + const base_destroyed = scheduler.aspaceDestroyCount(); + const rounds: u32 = 5; + var killed: u32 = 0; + var round: u32 = 0; + while (round < rounds) : (round += 1) { + process.fault_kill_count = 0; + const probe = spawnFaultingProcess() orelse break; + _ = probe; + // Let the probe fault on its first instruction and be reaped. + scheduler.setPriority(1); + const deadline = architecture.millis() + 5000; + while (process.fault_kill_count < 1 and architecture.millis() < deadline) scheduler.yield(); + scheduler.setPriority(4); + if (process.fault_kill_count >= 1) killed += 1; + } + check("all probes spawned and were killed", killed == rounds); + check("live address-space count returned to baseline", scheduler.liveAspaceCount() == base_live); + check("each address space destroyed exactly once", scheduler.aspaceDestroyCount() == base_destroyed + rounds); + if (killed == rounds and scheduler.liveAspaceCount() == base_live and + scheduler.aspaceDestroyCount() == base_destroyed + rounds) + log("aspace-refcount: spaces released to baseline ok\n", .{}); + result(); +} + /// The full PID-1 path: the bootloader read /system/services/init off the boot volume and /// handed it over; load it as a user ELF and spawn it as a real ring-3 process /// — the same call the normal boot path makes — then confirm it beats. init diff --git a/test/qemu_test.py b/test/qemu_test.py index e956e56..2556441 100644 --- a/test/qemu_test.py +++ b/test/qemu_test.py @@ -293,6 +293,13 @@ CASES = [ "timeout": 60, "expect": r"DANOS-TEST-RESULT: PASS", "fail": r"DANOS-TEST-RESULT: FAIL"}, + + # docs/threading-plan.md M1: address-space refcount — spaces destroyed exactly + # once per process, no leak/double-free (the foundation shared-aspace threads need). + {"name": "aspace-refcount", + "timeout": 60, + "expect": r"DANOS-TEST-RESULT: PASS", + "fail": r"DANOS-TEST-RESULT: FAIL"}, # Process arguments: argv arrives on the SysV entry stack (argv[0] = the spawned # name, argv[1..] = the system_spawn argument blob) and echoes back intact. {"name": "args",