Conversation
…y setBuffer AudioBufferSourceNodeHostObject::setBuffer() deep-copied the entire AudioBuffer on every `.buffer = x` reassignment, even when reassigning the exact same underlying buffer. That's exactly what seeking is. A live AudioBufferSourceNode can't be repositioned, so the standard pattern is to stop/disconnect it and create a fresh source node wrapping the already-decoded buffer at a new offset. Every seek therefore triggered a full, unnecessary copy of the whole track's PCM data. On a 4-minute stereo buffer this is a real full-size copy (~85MB) per seek. The old copy isn't freed until its replacement's scheduled audio event actually runs, so repeated rapid seeking accumulates full-size buffers faster than the audio thread can free the previous ones. The growing native heap eventually crashes the app via a Hermes GC OOM once it can no longer find room to grow the JS heap. This caches the defensive copy on the JS-visible AudioBufferHostObject and reuses it across repeated reassignments of the same buffer. The cache is invalidated only when the buffer's data could actually have been mutated: copyToChannel, or a live getChannelData() view having escaped to JS, since JS could write through that view at any later time. This keeps the exact copy-on-first-touch semantics the original code needed for pitch-correction and mutation safety, while making the "same buffer, many source nodes" pattern free after the first copy instead of paying for a fresh copy on every single reassignment. The caching logic lives in a new, standalone ImmutableBufferCache utility rather than directly in AudioBufferHostObject, because HostObjects/*.cpp is excluded from this project's C++ test suite (see common/cpp/test/CMakeLists.txt) and a plain utility class can be unit tested without a jsi::Runtime. Also documents this in the best-practices guide. The existing "reuse the same AudioBuffer across nodes" guidance was already correct, but silently expensive before this fix; it is now actually cheap as advertised. Verified: - The library's own C++ test suite still passes in full, plus 4 new unit tests for ImmutableBufferCache covering reuse, distinct-copy identity, and both invalidation paths (420/420 total). - A minimal repro app (creating a fresh AudioBufferSourceNode/GainNode pair wrapping the same AudioBuffer every ~150ms, simulating rapid seeking) went from ~71MB leaked per seek (visibly crashing within ~60 iterations) to no measurable per-seek growth across three consecutive 60-iteration runs, measured via `adb shell dumpsys meminfo` before/after/+30s-settled. - A real app using this pattern for playback seeking held native heap flat (Android, Samsung Galaxy S24+) across ~200 rapid seeks that previously crashed within a similar span. Fixes #1263
mdydek
added
the
refactor
7 tasks
WPT non-regression comparisonPASS — no regressions · 1 improved section(s) · overall 2665 → 2667 (+2)
Unchanged sections (27)
Baseline: Workflow run · this comment is updated on every push. |
mdydek
marked this pull request as draft
September 11, 2026 11:49
mdydek
marked this pull request as ready for review
September 11, 2026 11:56
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Direct continuation of the work from #1281, different mental model, still addresses #1263
Introduced changes
Screens from https://github.com/WentTheFox/AudioApiLeakRepro
before:
after:
getChannelDatagets you the copy of the new data, which could affect the absn, but only after the start we will respect possible changesby implementing this approach usual use case of the audiobuffersourcenode and creating new node with already present buffer allocates only memory for the metadata of the node
Checklist