Skip to content

fix(android/ios): cache AudioBuffer copy instead of recopying on every setBuffer - #1281

Closed
WentTheFox wants to merge 2 commits into
software-mansion:mainfrom
WentTheFox:fix/audio-buffer-source-copy-leak
Closed

WentTheFox wants to merge 2 commits into
software-mansion:mainfrom
WentTheFox:fix/audio-buffer-source-copy-leak

Conversation

@WentTheFox

@WentTheFox WentTheFox commented Sep 10, 2026 •

Copy link
Copy Markdown

Closes #1263

⚠️ Breaking changes ⚠️

None.

Introduced changes

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 updates the best-practices guide. The existing "reuse the same AudioBuffer across nodes" advice was already correct, but silently expensive before this fix; it is now actually cheap as advertised.

by @mdydek - implemented "acquire buffer content" logic in the AudioBufferSourceNode. It blocks and invalidates all the js views of the buffer that were remembered before calling start on the node, thus making changes after start noop in the context of the node

Verification

  • The existing 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 (https://github.com/WentTheFox/AudioApiLeakRepro), creating a fresh AudioBufferSourceNode/GainNode pair wrapping the same AudioBuffer every ~150ms to simulate rapid seeking, went from ~71MB leaked per seek (crashing within ~60 iterations) to no measurable per-seek growth across three consecutive 60-seek runs, measured with adb shell dumpsys meminfo before/after/+30s-settled.
  • A real playback app using this seek pattern held native heap flat across ~200 rapid seeks on a physical device (Samsung Galaxy S24+) that previously crashed within a similar span.

Checklist

  • Linked relevant issue
  • Updated relevant documentation
  • Added/Conducted relevant tests
  • Performed self-review of the code
  • Updated Web Audio API coverage. Not a manual edit here: behavior is unchanged, so the WPT base/head comparison this repo's CI runs automatically on this PR should confirm it
  • Added support for web. Not applicable: AudioBufferSourceNode.web.ts delegates directly to the browser's native implementation, which never had this bug
  • Updated old arch android spec file. Not applicable: AudioBuffer/setBuffer are JSI HostObjects, not TurboModule methods, so nothing in the generated spec changed

🤖 Generated with Claude Code

https://claude.ai/code/session_014PuKaaBmaMbrAtgs1a4xTC

…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 software-mansion#1263
@WentTheFox
WentTheFox marked this pull request as ready for review September 10, 2026 11:20
@github-actions

Copy link
Copy Markdown

WPT non-regression comparison

ERROR — the comparison did not produce a report; the test run itself likely failed.

Workflow run · this comment is updated on every push.

@mdydek mdydek mentioned this pull request Sep 11, 2026
7 tasks
@mdydek

mdydek commented Sep 11, 2026 •

Copy link
Copy Markdown
Member

I changed your approach in #1283, left your commit so after merge you will still have contribution. You can look in mentioned PR's description for more detailed explanation, but tldr it will be easier to iterate this way and I also adressed something called acquire the content there

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.

[Android] Repeated seek (recreate source node, same AudioBuffer) leaks native heap - never released by disconnect(), crashes via Hermes GC OOM

2 participants