Skip to content

Fix crashes and corruption with many custom music tracks (audio heap overflow) - #6932

Open
lunks wants to merge 1 commit into
HarbourMasters:developfrom
lunks:fix/6900-audio-heap-cache-overflow
Open

Fix crashes and corruption with many custom music tracks (audio heap overflow)#6932
lunks wants to merge 1 commit into
HarbourMasters:developfrom
lunks:fix/6900-audio-heap-cache-overflow

Conversation

@lunks

@lunks lunks commented Jul 15, 2026

Copy link
Copy Markdown

Fixes a crash with large custom music packs

Companion to #6916 — that PR fixed one out-of-bounds write (AudioLoad_SetFontLoadStatus); this one fixes a separate overflow it doesn't touch.

The audio engine keeps a fixed 32-entry list of permanently-loaded audio (permanentCache) and never checks the limit when adding to it. With a large custom pack, every track loads permanently (both its soundfont and its sequence), so the list fills past 32 within a session and each further entry gets written past the end — over neighbouring audio state, eventually the sequence players' script pointers, which crashes the audio thread (#6900, still reproducible after #6916).

Fix

  • Grow the list to 512 and stop cleanly at the limit instead of writing out of bounds.
  • Same bounds check on three other cache lists written the same way.
  • Size the sequence load-status array to the id range custom tracks actually use.

Vanilla sessions stay well under the limit, so stock behaviour is unchanged. Technical detail is in the commit message.

Build Artifacts

@serprex

serprex commented Jul 15, 2026

Copy link
Copy Markdown
Member
  1. what about fix(audio): bounds-check fontId to stop OOB crash with large SAF packs #6916?
  2. holy what model/prompt are you using to give this much comment slop?

@lunks

lunks commented Jul 15, 2026

Copy link
Copy Markdown
Author
  1. what about fix(audio): bounds-check fontId to stop OOB crash with large SAF packs #6916?
  2. holy what model/prompt are you using to give this much comment slop?
  1. Each fixes a different overflow. Theirs is AudioLoad_SetFontLoadStatus, ours is on permanentCache via AudioHeap_AllocPermanent.
  2. Terribly sorry for it, I was overzealous on the PR description and overlooked the final commit. Amending it. It's Claude Opus/Fable combined with my oversight.

@serprex
serprex self-requested a review July 15, 2026 17:17
…music packs)

Root cause of three identical field crashes (audio thread, opcode fetch
through a pointer with its low 32 bits overwritten, seconds after scene
transitions): AudioHeap_AllocPermanent writes permanentCache[index] with
index = permanentPool.count and no bound against the 32-entry array. In
SoH every soundfont sync-load is forced permanent, and custom sequences
whose SEQ.xml says CachePolicy="Temporary" ALSO allocate permanently
(the factory stores the LUS enum where CACHE_TEMPORARY == 0, while
AudioLoad_SyncLoad's switch reads 0 with the ROM convention
'permanent'). A pack with ~60 streamed customs plus vanilla fonts pushes
count past 32 within a session, after which each allocation sprays a
{ptr, size, tableType/id} triplet at 24-byte stride through
gAudioContext - entry[135]'s ptr field lands exactly on
seqPlayers[0].scriptState.pc and entry[156] on seqPlayers[1]'s (both
verified against the crash-dump registers).

- permanentCache raised 32 -> 512 (12 KB) and AllocPermanent refuses
  allocations past the array instead of corrupting memory.
- Same unbounded-index disease fixed in the three sibling writers:
  AllocCached's persistent path (16-entry array; CACHE_EITHER degrades
  to temporary, hard persistent requests fail cleanly),
  AllocPersistentSampleCacheEntry, AllocTemporarySampleCacheEntry.
- seqLoadStatus malloc sized for the full id space (sequenceMapSize +
  0xF) matching sequenceMap; custom ids above sequenceMapSize previously
  overflowed the allocation by up to 15 bytes.

Upstream SoH bugs, not branch-introduced - this branch's many-track
packs merely made the overflow reachable in normal play. Standalone
upstreamable fix.

Claude-Session: https://claude.ai/code/session_01Sxa5kfXEtLwWmZ1Ht692oa
@lunks
lunks force-pushed the fix/6900-audio-heap-cache-overflow branch from 61bda67 to 36c0103 Compare July 15, 2026 17:18
@sefirosu562

Copy link
Copy Markdown

Crashes are completely fixed, that's what my case testing ended up on at least.

Tested with my earlier ~60 song setup (see #6900) and another where i literally just dumped every SAF mod i have, like 2GB of unorganized junk. it seemed to be at least ~600-700 tracks, so most tracks were corrupted, or displayed the wrong name (shocker i know) — but no crashes !!

Both #6916 and #6932 combined seemed to fix the most frustrating symptoms at least. I might do another test with ~300-400 tracks to confirm the 512 limit.

Regular gameplay even with a bulky music pack will not be affected by crashes anymore. Probably.

@serprex serprex added this to the 9.3.0 milestone Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants