Skip to content

fix: iOS recorder callback teardown race#1057

Open
radko93 wants to merge 1 commit intosoftware-mansion:mainfrom
radko93:fix/ios-recorder-callback-teardown-race
Open

fix: iOS recorder callback teardown race#1057
radko93 wants to merge 1 commit intosoftware-mansion:mainfrom
radko93:fix/ios-recorder-callback-teardown-race

Conversation

@radko93
Copy link
Copy Markdown

@radko93 radko93 commented May 8, 2026

Closes #

⚠️ Breaking changes ⚠️

None

Introduced changes

IOSRecorderCallback::cleanup() and ~IOSRecorderCallback() currently nil the ObjC instance vars (converter_, bufferFormat_, converterInputBuffer_, ...) before destroying the TaskOffloader. The worker thread in taskOffloaderFunction reads those same ObjC fields without synchronization, so on teardown it can dereference fields that are mid-release.

This PR reorders teardown so the worker is joined before any ObjC field is released:

  1. isInitialized_.store(false) — fence further audio-thread enqueues.
  2. offloader_.reset() — signals shutdown via the dummy item, joins the worker.
  3. nil the ObjC fields, zero the circular buffers.

In the destructor specifically, the existing implementation relied on the unique_ptr<TaskOffloader> member destructor (which runs after the body), so the body's nil assignments raced the worker. Explicitly resetting at the top of the body closes that window.

cleanup() still flushes any frames the worker had pushed into circularBuffer_ before being joined (emitAudioData(true)). Frames sitting undrained in the SPSC channel at cleanup time are dropped — this matches the prior behaviour, the previous code was racing the same drop incorrectly.

The race is not deterministically reproducible (it depends on worker scheduling) so this PR does not add a unit test. We applied this change downstream and confirmed the corresponding production iOS SIGSEGV signature drops to zero. Representative symbolicated stack from production (seen across iOS 17/18/26, multiple iPhone models):

Thread <worker> — crashed, SIGSEGV (SEGV_ACCERR)
0   AVFAudio                  -[AVAudioBuffer mutableAudioBufferList] + 424
1   AVFAudio                  -[AVAudioBuffer mutableAudioBufferList] + 420
2   <app>                     audioapi::IOSRecorderCallback::taskOffloaderFunction(CallbackData) + 168 (IOSRecorderCallback.mm:166)
3-7 <app>                     <TaskOffloader worker thread proxy>
8   libsystem_pthread.dylib   _pthread_start + 136

Thread <JS> — not crashed, waiting
0   libsystem_kernel.dylib    __ulock_wait
1   libsystem_pthread.dylib   _pthread_join
2   libc++.1.dylib            std::__1::thread::join()
3   <app>                     audioapi::task_offloader::TaskOffloader<...>::~TaskOffloader() + 68 (TaskOffloader.hpp:47)
4-5 <app>                     audioapi::IOSRecorderCallback::cleanup() (IOSRecorderCallback.mm:122)
6   <app>                     audioapi::IOSAudioRecorder::stop() (IOSAudioRecorder.mm:362)

The crashing frame at IOSRecorderCallback.mm:166 is converterInputBuffer_.mutableAudioBufferList->mBuffers[i].mData — the worker dereferencing an AVAudioPCMBuffer* instance variable that the JS thread is concurrently nil-ing inside cleanup(). The JS thread is in _pthread_join waiting for that same worker, which won't return because it just SIGSEGV'd. With the reorder in this PR, offloader_.reset() (which performs the join) completes before any ObjC field is released, so the worker is provably done before fields are nil.

A separate audio-buffer-lifetime issue in the same component is being tracked at #1053 (with proposed fix in #1054); intentionally out of scope for this PR.

Checklist

  • Linked relevant issue
  • Updated relevant documentation (N/A — no public API change)
  • Added/Conducted relevant tests (N/A — race not deterministically reproducible; see body)
  • Performed self-review of the code
  • Updated Web Audio API coverage (N/A)
  • Added support for web (N/A)
  • Updated old arch android spec file (N/A — iOS-only)

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.

1 participant