From d8c046acc4d3c7489bf723502125944e06c06a9e Mon Sep 17 00:00:00 2001 From: chrg6 Date: Fri, 13 Nov 2026 17:56:39 -0500 Subject: [PATCH] Destroy a movie decoder's system events and free their table entries when the decoder goes. The review of the system-event change (Ev-1) found the table still filling. Entries in host memory now count as live for good in the full-table scan, but their objects are freed: ov_movie2.c ov_dtor freed the Video with in_ev and out_ev still bound, so each decoder left two dead entries that nothing reclaimed (AUDIT F-1-08 measured about two entries per reload of a save, which E-4's verifier tied to the decoder pair), and their four kernel handles stayed open. A long session would end with "kern_bind_system_event: table full" once no dead guest entry was left to take. kernel.c gains kern_unbind_system_event(c, ev). On a guest thread it calls the SDK's nn::os::DestroySystemEvent, which zeroes the state byte and closes both handles, and then removes the entry under g_se_lock (the last entry takes its place). The destroy comes first, so a signal still in flight finds the event destroyed or its handle no longer naming it and skips it, rather than finding no entry and binding again on a host thread. Off a guest thread, or when the SDK lacks the function (logged once), only the entry goes. ov_dtor calls it for each bound event after finalize has joined the decoder and convert threads, then frees the Video. net_offline.c frees its HostTask with a bound event in the same way; that file is outside this change and is handed to the coordinator. Not reproduced before the change in a game run. tests/system_event_check.c fills the table with live guest events and the movie's two host-memory events, unbinds one: it leaves the table, its handles are closed and its state byte is 0, the other keeps its entry, and a second unbind changes nothing; with the table full again and the last host entry unbound, a new guest bind succeeds where it would have ended with "table full". The check passes in both modes, movie_planes_check passes, and the compile check of every runtime file (host.h changed) passes, adding no warnings in kernel.c or ov_movie2.c with -Wall -Wextra. Co-Authored-By: Claude Opus 5.5 --- runtime/host.h | 2 ++ runtime/kernel.c | 43 ++++++++++++++++++++++++++----- runtime/ov_movie2.c | 12 ++++++--- tests/system_event_check.c | 51 ++++++++++++++++++++++++++++++++++--- tests/system_event_check.py | 6 ++--- 5 files changed, 98 insertions(+), 16 deletions(-) diff --git a/runtime/host.h b/runtime/host.h index 7084ef29..1b4a805a 100644 --- a/runtime/host.h +++ b/runtime/host.h @@ -358,6 +358,8 @@ void kern_event_clear(uint32_t writable); // server-side clear void kern_bind_system_event(Cpu *c, uint64_t ev, int autoclear); // guest thread only void kern_signal_system_event(uint64_t ev); // any thread void kern_clear_system_event(uint64_t ev); // any thread; a bound, live event only +void kern_unbind_system_event(Cpu *c, uint64_t ev); // before freeing a host-memory event nothing signals now; + // destroys it on a guest thread (c set) int kern_system_event_live(uint64_t ev); // in mapped guest memory, its state byte nonzero int kern_svc(Cpu *c, uint32_t imm); // thread/sync SVCs; 1 if handled uint64_t kern_now_ns(void); diff --git a/runtime/kernel.c b/runtime/kernel.c index 67182514..687579cb 100644 --- a/runtime/kernel.c +++ b/runtime/kernel.c @@ -2158,8 +2158,10 @@ void kern_event_clear(uint32_t hw) { // Each entry also keeps the serial number of the Event its handle named at bind time (h_new hands a closed number to // the next new object at once, so the number alone does not say it is still that event), and whether the object lies // in guest memory: host services keep some in their own allocations (ov_movie2.c Video.in_ev/out_ev, net_offline.c -// HostTask.event), which mem_readable does not know and the SDK never destroys. -// Lock order: G before g_se_lock (system_event_set); kern_bind_system_event never holds both. +// HostTask.event), which mem_readable does not know and the game never destroys. A service that frees such an object +// gives its entry back first (kern_unbind_system_event, from ov_movie2.c ov_dtor). +// Lock order: G before g_se_lock (system_event_set); kern_bind_system_event and kern_unbind_system_event never hold +// both. #define SE_STATE 0x28 static struct { uint64_t ev, serial; uint32_t wh; uint8_t guest; } g_se[512]; static int g_nse; @@ -2205,10 +2207,11 @@ void kern_bind_system_event(Cpu *c, uint64_t ev, int autoclear) { int i = 0; while (i < g_nse && g_se[i].ev != ev) i++; if (i == g_nse && g_nse == (int)(sizeof g_se / sizeof *g_se)) { - // Full: entries are never removed, and a long session binds events at new addresses (two per reload of a - // save). An entry in guest memory whose event the guest destroyed (its state byte is 0, or its memory is gone) - // gives its slot. Entries in host memory count as live: their services never destroy them through the SDK, and - // taking a live one would make the service's next signal bind again on a host thread, which is fatal. + // Full: entries in guest memory are never removed, and a long session binds events at new addresses (two per + // reload of a save). An entry in guest memory whose event the guest destroyed (its state byte is 0, or its + // memory is gone) gives its slot. Entries in host memory count as live: the game never destroys them, their + // services remove them before freeing them (kern_unbind_system_event), and taking a live one would make the + // service's next signal bind again on a host thread, which is fatal. i = 0; while (i < g_nse && (!g_se[i].guest || (mem_readable(g_se[i].ev + SE_STATE, 1) && se_live(g_se[i].ev)))) i++; if (i == g_nse) rt_fatal("kern_bind_system_event: table full (%d live system events)", g_nse); @@ -2259,6 +2262,34 @@ void kern_clear_system_event(uint64_t ev) { if (se_find(ev, NULL) && se_live(ev)) system_event_set(ev, 0); } +// For a service about to free a SystemEventType it bound in its own memory (ov_movie2.c ov_dtor), with nothing left to +// signal it: on a guest thread the SDK's DestroySystemEvent zeroes the state byte and closes both kernel handles, which +// would otherwise stay open for the rest of the run; then the table entry goes, and its slot is free for the next bind. +// The destroy comes first, so a signal still in flight finds the event destroyed (kern_signal_system_event) or its +// handle no longer naming it (system_event_set) and skips it, rather than finding no entry and binding again. Off a +// guest thread, or when the SDK lacks the function, only the entry goes. +void kern_unbind_system_event(Cpu *c, uint64_t ev) { + if (!se_find(ev, NULL)) return; + if (c && tcur && se_live(ev)) { + static uint64_t destroy; // 1: looked up and not loaded + uint64_t fn = __atomic_load_n(&destroy, __ATOMIC_RELAXED); + if (!fn) { + fn = ldr_symbol("_ZN2nn2os18DestroySystemEventEPNS0_15SystemEventTypeE"); + if (!fn) + rt_log("kernel: the SDK's DestroySystemEvent is not loaded: unbound system events keep their handles"); + __atomic_store_n(&destroy, fn = fn ? fn : 1, __ATOMIC_RELAXED); + } + if (fn > 1) rt_call(c, fn, 1, ev); + } + plat_mutex_lock(&g_se_lock); + for (int i = 0; i < g_nse; i++) + if (g_se[i].ev == ev) { + g_se[i] = g_se[--g_nse]; + break; + } + plat_mutex_unlock(&g_se_lock); +} + // A SystemEventType that still holds an event: mapped guest memory with its state byte nonzero. For services that keep // their own lists of bound events and reuse the slots of destroyed ones (gfx_present.c gfx_vsync_bind). int kern_system_event_live(uint64_t ev) { return mem_readable(ev + SE_STATE, 1) && se_live(ev); } diff --git a/runtime/ov_movie2.c b/runtime/ov_movie2.c index bb44199e..5313bc80 100644 --- a/runtime/ov_movie2.c +++ b/runtime/ov_movie2.c @@ -640,9 +640,15 @@ static void ov_ctor(Cpu *c) { *(Video **)arg_p(c, 0) = NULL; } static void ov_dtor(Cpu *c) { Video **slot = arg_p(c, 0); if (!slot || !*slot) return; - finalize(*slot); - set_live(*slot, 0); - free(*slot); // the bound events' kernel handles stay, as for any guest event never destroyed + Video *v = *slot; + finalize(v); + set_live(v, 0); + // The decoder and convert threads are joined, so nothing signals the events now: they are destroyed and leave the + // kernel's table of system events before their memory goes (kernel.c kern_unbind_system_event). Kept, each + // decoder left two dead entries and their kernel handles, and a long session filled the table. + if (v->in_bound) kern_unbind_system_event(c, (uint64_t)(uintptr_t)v->in_ev); + if (v->out_bound) kern_unbind_system_event(c, (uint64_t)(uintptr_t)v->out_ev); + free(v); *slot = NULL; gfx_movie_stop(); } diff --git a/tests/system_event_check.c b/tests/system_event_check.c index 732b761b..9215ee47 100644 --- a/tests/system_event_check.c +++ b/tests/system_event_check.c @@ -5,7 +5,9 @@ // number went to an unrelated event or the state byte reads nonzero again; a host thread signalling // while a guest thread destroys and binds the event again never ends the process; with the table full, // a new bind takes a destroyed guest-memory entry and keeps the entries in host memory, which a host -// thread then still signals; kern_system_event_live tells live guest events from destroyed ones +// thread then still signals; kern_system_event_live tells live guest events from destroyed ones; +// kern_unbind_system_event destroys a host-memory event and frees its entry, so a full table takes a +// new guest bind once the host entries are unbound // full every entry live: the next bind ends with "table full" (the script checks the exit code and text) #include "kernel.c" @@ -32,8 +34,11 @@ int rt_knob(const char *name, const char *def) { (void)name; return strcmp(def, int64_t frame_step_world_us(void) { return -1; } void ipc_handle_closed(int kind, void *obj) { (void)kind, (void)obj; } const char *ldr_describe(uint64_t a, char *buf, int n) { snprintf(buf, (size_t)n, "%#llx", (unsigned long long)a); return buf; } -// The SDK's two functions kern_bind_system_event calls: 1 CreateSystemEvent, 2 GetWritableHandleOfSystemEvent. -uint64_t ldr_symbol(const char *name) { return strstr(name, "CreateSystemEvent") ? 1 : 2; } +// The SDK's functions the table calls: 1 CreateSystemEvent, 2 GetWritableHandleOfSystemEvent (kern_bind_system_event), +// 3 DestroySystemEvent (kern_unbind_system_event). +uint64_t ldr_symbol(const char *name) { + return strstr(name, "CreateSystemEvent") ? 1 : strstr(name, "DestroySystemEvent") ? 3 : 2; +} void perf_main_thread(void) {} void perf_size(PerfSizes *s, const char *name, uint64_t count, uint64_t limit, uint64_t bytes) { (void)s, (void)name, (void)count, (void)limit, (void)bytes; // kern_sizes is not called here @@ -61,8 +66,10 @@ void rt_fatal(const char *fmt, ...) { } void rt_dispatch(Cpu *c) { (void)c; rt_fatal("rt_dispatch in the test"); } void rt_svc(Cpu *c, uint32_t imm, uint64_t pc) { (void)pc; kern_svc(c, imm); } +static void destroy(void *p); // CreateSystemEvent(ev, mode, ipc) makes a kernel event, keeps its writable handle at +0 of the SystemEventType and its -// readable one at +4, and sets the state byte; GetWritableHandleOfSystemEvent(ev) reads +0. +// readable one at +4, and sets the state byte; GetWritableHandleOfSystemEvent(ev) reads +0; DestroySystemEvent(ev) as +// destroy below. uint64_t rt_call(Cpu *c, uint64_t fn, int n, ...) { (void)c; va_list ap; @@ -77,6 +84,10 @@ uint64_t rt_call(Cpu *c, uint64_t fn, int n, ...) { __atomic_store_n(p + SE_STATE, 1, __ATOMIC_RELEASE); return 0; } + if (fn == 3) { + destroy(p); + return 0; + } uint32_t wh; memcpy(&wh, p, 4); return wh; @@ -210,6 +221,37 @@ static void full_table(int all_live) { "a host thread signals the movie's events with the table full"); } +// ov_movie2.c ov_dtor with the table full of live guest events and the movie's two host-memory events: each unbound +// event is destroyed (its handles closed) and leaves the table, the other keeps its entry, and once the last host +// entry is gone a new guest bind finds room (the full-table scan keeps host entries, so it would end with "table full"). +static void unbind_host(void) { + int n = (int)(sizeof g_se / sizeof *g_se); + CHECK(g_nse == n, "the table is full before the unbinds (%d of %d)", g_nse, n); + uint32_t in_w = wh_of(movie_in), in_r = rh_of(movie_in); + kern_unbind_system_event(&cpu, ev_of(movie_in)); + CHECK(se_find(ev_of(movie_in), NULL) == 0, "an unbound event leaves the table"); + CHECK(g_nse == n - 1, "the table has one entry fewer (%d)", g_nse); + CHECK(se_find(ev_of(movie_out), NULL) != 0, "the other host-memory event keeps its entry"); + CHECK(movie_in[SE_STATE] == 0 && signaled(in_w) == -1, "the unbound event is destroyed: state 0, handle %#x closed", + in_w); + lock(); + int r_open = h_peek(in_r, O_EVENT_R) != NULL; + unlock(); + CHECK(!r_open, "its readable handle %#x is closed too", in_r); + kern_unbind_system_event(&cpu, ev_of(movie_in)); // a second unbind finds nothing and leaves the table alone + CHECK(g_nse == n - 1, "a second unbind changes nothing (%d)", g_nse); + // Full again with live guest entries and one host entry; the host entry unbound, the next guest bind takes its room. + int next = n + 2; + bind(guest[next++]); + CHECK(g_nse == n, "full again (%d)", g_nse); + kern_unbind_system_event(&cpu, ev_of(movie_out)); + bind(guest[next]); + CHECK(se_find(ev_of(guest[next]), NULL) != 0 && g_nse == n, + "a guest bind succeeds once the host entry is unbound (%d entries)", g_nse); + free(movie_in), free(movie_out); + movie_in = movie_out = NULL; +} + int main(int argc, char **argv) { setvbuf(stdout, NULL, _IONBF, 0); me.cur = &cpu; @@ -219,6 +261,7 @@ int main(int argc, char **argv) { race(); full_table(full); if (full) return 0; + unbind_host(); printf(fails ? "system_event_check: %d failed\n" : "system_event_check: ok\n", fails); return fails ? 1 : 0; } diff --git a/tests/system_event_check.py b/tests/system_event_check.py index 8bc59c09..a6851ed8 100644 --- a/tests/system_event_check.py +++ b/tests/system_event_check.py @@ -1,7 +1,7 @@ """Build and run tests/system_event_check.c: the system-event table of runtime/kernel.c (kern_bind_system_event, -kern_signal_system_event, kern_clear_system_event) against destroyed events, reused handle numbers, a host thread racing -a destroy and bind, and a full table holding events in host memory; with the platform layer linked and the rest of the -runtime stubbed. No game files: the module layout the runtime includes is a stand-in written to the test's build folder. +kern_signal_system_event, kern_clear_system_event, kern_unbind_system_event) against destroyed events, reused handle +numbers, a host thread racing a destroy and bind, a full table holding events in host memory, and host-memory events +unbound before their memory is freed; with the platform layer linked and the rest of the runtime stubbed. No game files: the module layout the runtime includes is a stand-in written to the test's build folder. Usage: system_event_check.py """