fix(recorder): copy audio buffer before async handoff to fix use-after-free#1054
Open
christian-apollo wants to merge 3 commits intosoftware-mansion:mainfrom
Open
Conversation
The receiver block on iOS (CoreAudio) and Android (Oboe) is invoked synchronously and owns the audio buffer only for the duration of that call. Both AndroidRecorderCallback::receiveAudioData and IOSRecorderCallback::receiveAudioData pushed the raw pointer onto an SPSC queue and dereferenced it later from the offloader thread, which is a use-after-free — the buffer can be reused or freed by the audio system before the worker thread reads it. Copy the buffer into an owned allocation in receiveAudioData and free it after taskOffloaderFunction is done with it (along both code paths, including the converter path on iOS and the resampling path on Android). Skip processing and frees on the cleanup sentinel (data == nullptr) which the TaskOffloader destructor sends. Crash signature on iOS: Crashed: Thread 0 libsystem_platform.dylib _platform_memmove + 144 1 ApolloScooters audioapi::IOSRecorderCallback::taskOffloaderFunction 2 ApolloScooters std::__thread_proxy<...IOSRecorderCallback::prepare...>
mdydek
approved these changes
May 7, 2026
Collaborator
mdydek
left a comment
There was a problem hiding this comment.
nice one, while reviewing found another fatal race condition (destructor of the callback object could be fired in the same time when receiver function, thus making f.e. converterOutputBuffer_ invalid), I fixed it in this pr
7 tasks
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
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.
Closes #1053
Introduced changes
IOSRecorderCallback::receiveAudioDataandAndroidRecorderCallback::receiveAudioDatawere pushing the raw audio buffer pointer received from the platform (CoreAudio render callback on iOS, Oboe data callback on Android) onto an SPSC queue and then dereferencing it asynchronously from the offloader worker thread. Both the iOSAudioBufferList *and the Androidvoid *are owned by the audio system only for the duration of the synchronous callback — by the time the worker thread runsmemcpy(iOS) orma_data_converter_process_pcm_frames(Android), the source buffer can already be reused or freed. That's a use-after-free; in production it surfaces as a_platform_memmovesegfault insideIOSRecorderCallback::taskOffloaderFunction.This PR copies the buffer into an owned allocation in
receiveAudioDataand frees it after the worker is done with it on every code path:AudioBufferListheader plus per-channelmData, deep-copy the contents, send the owned pointer through the queue, and free it via a smallfreeOwnedAudioBufferListhelper after both the fast (matching format) and the converter (resampling) paths have read from it.ma_mallocan owned buffer,std::memcpythe frames, andma_freeafter both the fast and resampling paths intaskOffloaderFunction. Also added a guard that ignores thedata == nullptrsentinel that theTaskOffloaderdestructor sends to unblock the worker, since that sentinel carries no allocation to free.The fix is symmetric on both platforms; the underlying mistake (capturing a borrowed buffer and reading it across threads) was the same on both sides.
Reproduction
The race is hard to reproduce deterministically — it depends on the SPSC worker being preempted long enough for the audio system to reuse the source slot before the worker reads it. Inspection alone shows the bug, and the production crash trace in #1053 lands exactly inside the iOS
memcpyloop atIOSRecorderCallback.mm:164.Checklist