Merge fix/d285-audio-thread-race: D285 audio-thread SIGSEGV fixed via D4/D152-class lock

Claude-Session: https://claude.ai/code/session_01KwZViAFaXua3PKgHHjNgvi

docs/dev/findings-index.csv

@@ -162,5 +162,5 @@ D263,RESOLVED,v0.2.1 (user report):,"RESOLVED (not a bug), but refined (v0.2.1 f
 D265,?,"D236 investigation (port-layer, port/fast3d/gfx_pc.cpp):",PARKED — reverted to baseline; evidence + suspect list above.
 D258,OPEN,PAL/JP asset conversion is broken at the source-data level —,"OPEN — repair path verified, not applied (out of v0.2.0 scope)."
 D284,FIXED,"port/src/random.c's MIPS64 dsll32/dsrl32 shift helpers were backwards (modelled as 32-bit shifts, not ""shift by n+32 on the full 64-bit register""), desyncing t…","FIXED, 21-level sweep clean, no crashes —"
-D285,OPEN,"A real Steam Deck SIGSEGV caught during D255 live-testing (v0.2.1 published binary, symbolized on-device via addr2line against its own debug symbols) is a SEPA…","OPEN — case identified (unprotected shared-list read racing a locked writer), NOT fixed; open question whether the missing lock is a genuine N64-decomp-fidelity gap or a legitimate new PC-only lock (…"
+D285,FIXED,"A real Steam Deck SIGSEGV caught during D255 live-testing (v0.2.1 published binary, symbolized on-device via addr2line against its own debug symbols) is a SEPA…","FIXED (M-141, src/snd.c) —"
 D286,OPEN,"User-confirmed real (not system-side) ~8s frame freeze on Caverns, Steam Deck, caught during the same live session as D255/D285 —","OPEN — one confirmed occurrence, no repro trigger identified, not root-caused; distinct from D255/D285."

docs/dev/findings.md

@@ -560,7 +560,7 @@ covers D24–D69; the log continues in §H (D32 procedure, D70–D121).
 | D265 | **D236 investigation (port-layer, `port/fast3d/gfx_pc.cpp`): the tree cards are IA8/IA4, not IA16 — and `import_texture_ia4` decoded the RDP's I2:A2 texel as I3:A1, inverting the alpha ramp. Fixed; awaiting user visual re-test.** The earlier sessions misidentified the format: a live tri-time tile census (probe in `gfx_sp_tri1`, since removed) shows level_36's tree-card classes are (a) **IA8, line=64B → 64×16** (texdump r107), drawn OPA/bilerp/1-cycle/AC_NONE — *genuinely fully opaque* (A nibble F on 1023/1024 texels; opaque on N64 too) — and (b) **IA4, line=64B → 128×16** (texdump r117), drawn ZMODE_XLU/point/2-cycle with m2a=CLR_MEM/m2b=1MA, i.e. texel-alpha blended. There are no 32-wide IA tiles at tri time; the prior "32×32 tile" reading was wrong, and the whole earlier IA16 byte-pair/I11:A5 debate (r107) was analyzing an IA8 image as byte-pairs. **The wall's root cause:** r117 decoded as I2:A2 is a strip of ~13 discrete conifer silhouettes (trunk+canopy shapes visible in the ASCII render, `scratch/r117_ia4_alpha.png`) with a 3-level alpha ramp {0: 95% of texels, 85, 170} — faint dithered distant trees on N64. But `import_texture_ia4` read each nibble as intensity=`part>>1` (3-bit, SCALE_3_8) + alpha=`part&1`: A2=1 canopy pixels became **fully opaque**, A2=2 edge dithering became **invisible** → opaque repeating tree silhouettes = the "wall". The bug is inherited verbatim from the PD port (`pd_port/port/fast3d/gfx_pc.cpp` has the identical I3:A1 read — latent there, fatal for GE's tree strip). **Fix (applied):** `intensity = part>>2`, `alpha = part&3`, both via new `SCALE_2_8`; this is the hardware-correct RDP IA4 layout. Also cleaned up: the ia16 importer comment now states plain I8:A8 (the byte-pair read *is* correct IA16; the old "r107 proof" was misidentified data), and the earlier part-1 dimension fix stays (harmless/correct for real IA16 tiles). **Caveats/owed:** (1) user visual re-test of Surface 1 trees — headless A/B was in flight when work halted; a cross-run region diff was inconclusive (camera drift between captures); (2) the decode change affects *every* IA4 tile game-wide — regression-sweep other levels for dithered/alpha IA4 art that now renders with correct 3-level alpha; (3) if a hard repeating band remains after the fix, it is the opaque IA8 strip (a), which matches hardware and D236 needs re-scoping. **OUTCOME (user re-test): NEGATIVE — trees still render as the wall with the I2:A2 decode applied; the change was fully reverted (`git checkout` of `port/fast3d/gfx_pc.cpp`, back to the original PD-derived importers) and work is parked.** The census/RM/texture data above stands as evidence for the next attempt: the decode was *necessary* (I2:A2 is the hardware layout and r117 is unambiguously an authored tree strip under it) but *not sufficient* — remaining suspects, in order: (i) fast3d's blending of these XLU/2-cycle cards (texel alpha reaching the GL blend at all — the `texture_edge`/`SHADER_OPT_TEXTURE_EDGE` path forces `a>0.19 ? 1 : discard`, which for a 3-level ramp quantizes to opaque/discard, i.e. still hard-edged), (ii) per-card UV windowing/placement (all captured cards sampled the same u:0..64/v:0..16 window — confirm against N64), (iii) fog/distance. Note for the next session: verify what the N64 actually shows in that region before more port-side changes (the opaque IA8 strip is faithful to hardware either way). | PARKED — reverted to baseline; evidence + suspect list above. |
 | D258 | **PAL/JP asset conversion is broken at the source-data level — `scripts/filelist.e.csv` and `filelist.j.csv` carry naming defects, so `gen_romassets.py e/j` (and the d43/d69/d88 emitters' `find_row`) cannot resolve PAL/JP builds. v0.2.0's drop-in ROM feature therefore ships NTSC-U only.** Discovered while scoping issue #6 (drop-in ROM). **PAL:** 31 of the 512 model rows in `filelist.e.csv` have truncated/renamed basenames (an extraction artifact — e.g. `Pbridge_console` ×6 where the resource table says `Pbridge_console1aZ…3bZ`, `Pchrgrenadelauncher` for `PchrgrenadelaunchZ`). **JP:** one font row is mislabeled — `fontBankGothic_fontchartable.bin` @3044276 is named `fontZurichBold_fontchartable.bin` (a duplicate label). Both were proven without a PAL/JP ROM: (a) greedy two-pointer sequence alignment of the 512-row `file_resource_table.inc.c` model order against each CSV's model rows gives an exact 1:1 match (481 by-name + 31 positional for PAL), offsets strictly non-decreasing, and zero size anomalies vs US (all within [0.5×, 2×]); (b) JP font row proven by the contiguity invariant — Gothic kerning @3043600+676 = 3044276 exactly hits the mislabeled row, whose size (8716) equals the US Gothic chartable, while Zurich's own chain is contiguous one row later. **Consequence:** no PAL or JP PC exe has ever been buildable from this tree (latent — only NTSC was ever exercised); the CSV `file` column is a label only (emitters look up by basename, never read the path), so the repair is a pure data fix: rename the 31 PAL model rows + 1 JP font row to the exact table symbols. **Scope decision (v0.2.0):** ship drop-in ROM support for NTSC-U only — `prepare-assets.py`/the frozen `ge007-convert` gate non-US ROMs with a clear message after SHA-1 region detection; the CSV repair + multi-region matrix + per-region exes are deferred to the Part B follow-up. | OPEN — repair path verified, not applied (out of v0.2.0 scope). |
 | D284 | **`port/src/random.c`'s MIPS64 `dsll32`/`dsrl32` shift helpers were backwards (modelled as 32-bit shifts, not "shift by n+32 on the full 64-bit register"), desyncing the PC PRNG stream from the N64's from the very first draw, in every release to date (M-138 security/fidelity review; fix decided M-139b).** Fix: `dsll32(x,n) = x << (n+32)`, `dsrl32(x,n) = x >> (n+32)`, no masking — matches real MIPS64 semantics and `src/random.s`/`src/game/chrObjRandom.s` line-by-line. Verified by hand trace + an independent standalone C reproduction (both diverge from the buggy stream from draw 1). Full body below. | FIXED, 21-level sweep clean, no crashes — RNG stream changes from every prior build; existing replays will diverge, savegames unaffected |
-| D285 | **A real Steam Deck SIGSEGV caught during D255 live-testing (v0.2.1 published binary, symbolized on-device via `addr2line` against its own debug symbols) is a SEPARATE bug from D255** — `FAULT ADDR: 0x62` inside the **audio thread** (`sndHandleEvent`, `src/snd.c:366`, via `amMain`/`portThreadWrapper`), not D255's `0x10`/`vtxstore_fix_refs` on the render/game-logic thread. Root cause (static, not yet gdb-confirmed): the pool-exhaustion preempt-scan in `sndHandleEvent` reads/walks the shared `D_800243E4` active-`ALSoundState` list with **no `osSetIntMask` lock at all** (checked: absent from the function's entire body), while the list's insert path (`sndSetupSound`, reached from 15 different `src/game/*.c` files' direct `sndPlaySfx` calls, e.g. `explosion.c` — i.e. from whatever thread the caller runs on) **does** lock it — an asymmetric-locking gap, the same D4/D147/D152 "N64 interrupts-off is not a real lock on PC" class. Fits the repro pattern (explosion SFX bursts = high-contention window; Frigate's lower SFX load not reproducing it fits a timing race, not a deterministic trigger). Full body below. | OPEN — case identified (unprotected shared-list read racing a locked writer), NOT fixed; open question whether the missing lock is a genuine N64-decomp-fidelity gap or a legitimate new PC-only lock (needs a disassembly check or the D4-class exception call); not yet gdb-confirmed on a live crash. Do not conflate with D255 — different thread, different subsystem. |
+| D285 | **A real Steam Deck SIGSEGV caught during D255 live-testing (v0.2.1 published binary, symbolized on-device via `addr2line` against its own debug symbols) is a SEPARATE bug from D255** — `FAULT ADDR: 0x62` inside the **audio thread** (`sndHandleEvent`, `src/snd.c:366`, via `amMain`/`portThreadWrapper`), not D255's `0x10`/`vtxstore_fix_refs` on the render/game-logic thread. Root cause (static, not yet gdb-confirmed): the pool-exhaustion preempt-scan in `sndHandleEvent` reads/walks the shared `D_800243E4` active-`ALSoundState` list with **no `osSetIntMask` lock at all** (checked: absent from the function's entire body), while the list's insert path (`sndSetupSound`, reached from 15 different `src/game/*.c` files' direct `sndPlaySfx` calls, e.g. `explosion.c` — i.e. from whatever thread the caller runs on) **does** lock it — an asymmetric-locking gap, the same D4/D147/D152 "N64 interrupts-off is not a real lock on PC" class. Fits the repro pattern (explosion SFX bursts = high-contention window; Frigate's lower SFX load not reproducing it fits a timing race, not a deterministic trigger). Full body below. | **FIXED (M-141, `src/snd.c`)** — `osSetIntMask` lock added around the preempt-scan, matching `sndSetupSound`'s existing unconditional (no `#ifdef PORT`) pattern in the same file; build green, `-level_09`/`-level_20` unregressed (1400+ frames, no crash, no D152 lock-stuck warnings). Not yet gdb/live-confirmed against a real reproduced crash — see M-141 body. Do not conflate with D255 — different thread, different subsystem. |
 | D286 | **User-confirmed real (not system-side) ~8s frame freeze on Caverns, Steam Deck, caught during the same live session as D255/D285 — self-recovered, not a crash.** The game's own hang watchdog (`portHeartbeatCheck`) fired once (no frame for ~8s, `frames=46404`), dumped all-thread state via `crashDumpThreads`, then rendering resumed normally and the session continued to a clean close. Not tied to an explosion/terminal (rules out D255/D285). Not root-caused — no distinguishing log line precedes it; leading unconfirmed guess is a heavy room/asset stream-in stall (Caverns is a large level, cross-ref D250's FPS lineage, though the symptom shape differs). Also fixed in passing: the heartbeat log printed a raw microsecond value with a `ms` label (cosmetic, `port/src/libultra.c`). Full body below. | OPEN — one confirmed occurrence, no repro trigger identified, not root-caused; distinct from D255/D285. Do not chase a repro without a better lead. |
 
 Phase 2 replaced the Phase-1 demo loop with the real `mainproc()` on real OS
@@ -10173,6 +10173,8 @@ do {
 
 **Next, concrete:** (1) settle question 1 above (N64 disassembly check or accept the D4-class port-layer exception); (2) if a lock is warranted, wrap `sndHandleEvent`'s preempt-scan (and audit the rest of the function for the same gap against every other `D_800243E4` touch point in the file) in `osSetIntMask(OS_IM_NONE)`/restore, matching `sndSetupSound`'s existing pattern; (3) live-verify via gdb on a real reproduced crash, not just the static read.
 
+**M-141: FIXED, per the accepted D4-class port-layer exception; not yet live-verified.** Question 1 is settled by precedent rather than a fresh N64 disassembly read: `sndSetupSound` (the paired writer) already takes `osSetIntMask(OS_IM_NONE)`/restore completely unconditionally — **no `#ifdef PORT` at all** — around the identical `D_800243E4` list mutation, in this same file, with no N64-behavior caveat noted anywhere near it. Since `osSetIntMask` is genuine N64 API (not a port shim call added out of nowhere) and the port's own `osSetIntMask` implementation (`port/src/libultra.c`, "Interrupt mask -> global recursive lock", D147/D152) is a **process-wide recursive** critical section — safe to nest inside `alEvtqPostEvent`/`alSynSetVol` calls without deadlock risk — adding the identical lock/unlock pair around `sndHandleEvent`'s preempt-scan follows the exact same already-accepted shape as its sibling writer, unconditionally, no `#ifdef PORT` needed (matches the surrounding code's own style). Audited the rest of `sndHandleEvent` (`snd.c:263-680`) for other `D_800243E4` touch points per the open item above: **the preempt-scan is the only one** — nothing else in the function reads or writes that list. Fix: `src/snd.c`, wraps `ALSoundState *iterState = ...` through the closing `} while (limitReached && iterState != NULL);` in `OSIntMask d285Mask = osSetIntMask(OS_IM_NONE);` / `osSetIntMask(d285Mask);`. **Verified:** build green; `-level_09`/`-level_20` boot 1400+ frames each with zero crashes and zero `D152: osSetIntMask lock stuck` warnings (which would fire if this new lock ever wedged). **Not verified:** a `GE_INPUTSCRIPT`-driven rapid-fire stress test (modelled on D219/M-90's scripted-Z-pulse approach) was attempted to force the 8-voice pool exhaustion this scan requires, but the script's own parse-confirmation log line never appeared in this run's output — the scripted-input harness did not visibly activate for a reason not root-caused this session (worth a quick look next time `GE_INPUTSCRIPT` is used: confirm it still fires on current `main`/`release/v0.3.0` before trusting a "no crash" result from it as a real stress test). No live Deck/gdb reproduction attempted this session (no Deck SSH session active). Given the D4/D152 precedent's exact-shape match, confidence in the fix is HIGH; confidence that it's been exercised under real load is LOW — the honest bar met here is "correct by precedent, build/sweep-clean," not "reproduced-then-fixed." Next: an interactive session (ideally the same live Steam Deck setup as D255/M-140, explosion-heavy combat, e.g. Facility/Frigate) should confirm the crash no longer reproduces.
+
 ## D286 — M-140: user-confirmed, real (not system-side) ~8s frame freeze on Caverns, Steam Deck, same live session as D255/D285 — self-recovered, not a crash; case not yet root-caused
 
 **Context.** Caught live during the same Deck SSH session used to fix D255: the game's own hang watchdog (`portHeartbeatCheck`, `port/src/libultra.c`) fired once, logging `kernel heartbeat: no frame rendered for 8008866 us (frames=46404); state:` (unit bug in the log line, see below — the raw value is *microseconds*, printed with a `ms` suffix and no `/1000`; fixed this session, `port/src/libultra.c`) followed by a per-thread/per-queue state dump and a `crashDumpThreads` backtrace of the watchdog thread itself (`portTickThread` → `portHeartbeatCheck` → `crashDumpThreads`, `port/src/libultra.c:266/315`, `port/src/crash.c:458` — this is the watchdog reporting on *itself*, a diagnostic dump, not a fault). **User confirmed by direct observation: this was a real, visible freeze** (not a display/system hiccup — the Deck's own WiFi had one unrelated sleep/wake event earlier in the session, so system-side causes were a live alternative hypothesis, now ruled out by the user's own eyes on the screen), happening "pretty early" in a Caverns playthrough, not tied to an explosion or terminal (ruling out D255/D285 as the cause — this is a third, distinct issue).

src/snd.c

@@ -344,6 +344,19 @@ void sndHandleEvent(ALSndPlayer *sndp, ALSndpEvent *event) {
                         // Not a looped or retriggered sound, and all retries have been exhausted.
                         if (limitReached) {
                             // Check if we can preempt a lower-priority sound.
+                            /* D285: this scan reads/walks D_800243E4 without the
+                             * osSetIntMask lock sndSetupSound (above) already takes
+                             * around every mutation of the same list -- an
+                             * asymmetric-locking gap (writer locked, reader not) in
+                             * the D4/D152 class ("N64's interrupt-disable trick is a
+                             * no-op on a real PC thread"). Locking here changes
+                             * nothing observable on N64 (this function's own caller,
+                             * AL_SNDP_PLAY_EVT, already runs with interrupts masked
+                             * on real hardware per the audio ISR's normal execution
+                             * context); it closes the PC race where a burst of
+                             * sndPlaySfx calls (explosions) from the main thread can
+                             * mutate this list while the audio thread is mid-walk. */
+                            OSIntMask d285Mask = osSetIntMask(OS_IM_NONE);
                             ALSoundState *iterState = (ALSoundState *) D_800243E4.node.prev;
 
                             do {
@@ -381,6 +394,7 @@ void sndHandleEvent(ALSndPlayer *sndp, ALSndpEvent *event) {
                                 }
                                 iterState = (ALSoundState *) iterState->link.prev;
                             } while (limitReached && iterState != NULL);
+                            osSetIntMask(d285Mask);
 
                             if (!limitReached) {
                                 // Retry the sound that was preempted.