Skip to content

fix(ios): bound AudioDeviceModuleObserver JS waits to break the deadlock#90

Merged
hiroshihorie merged 2 commits into
masterfrom
hiroshi/fix-adm-indefinite-lock
Jun 17, 2026
Merged

fix(ios): bound AudioDeviceModuleObserver JS waits to break the deadlock#90
hiroshihorie merged 2 commits into
masterfrom
hiroshi/fix-adm-indefinite-lock

Conversation

@hiroshihorie

Copy link
Copy Markdown
Member

Problem

On iOS the six RTCAudioDeviceModule delegate callbacks in AudioDeviceModuleObserver block the native audio worker thread on dispatch_semaphore_wait(..., DISPATCH_TIME_FOREVER) while waiting for a JS reply. If the JS thread is at the same time parked inside a blocking-synchronous bridge call (for example peerConnectionAddTransceiver, which runs dispatch_sync(workerQueue) into libwebrtc and back onto the worker thread that is running this delegate), the reply never arrives and the app freezes permanently. There is no crash, every React Native touchable goes dead, and the only recourse is force-killing the app.

In practice this is triggered by publishing a microphone track and then a camera track back-to-back right after connect, or by subscribing to a remote audio-plus-video peer on join. The mic publish flips the engine from playout-only to duplex, and the camera publish issues the synchronous addTransceiver that lands in the same few-millisecond window.

Refs livekit/client-sdk-react-native#389 and #89.

Fix

  1. Bound each of the six waits to 2 seconds instead of waiting forever. On timeout the observer logs through os_log and returns the default value of 0 (proceed), so the engine operation degrades gracefully instead of deadlocking. The timeout itself is what breaks the circular wait, because it releases the worker thread.

  2. Add a request-id echo so a late reply from a round that already timed out cannot be misattributed to the next round. Native stamps every event with a monotonic id, JS echoes it back on resolve, and the observer only accepts a resolve whose id matches the in-flight round. A small pre-send drain covers the narrow case where a matching reply signals just past its round deadline.

Returning 0 on timeout rather than an error code is intentional. A non-zero return makes libwebrtc roll back the engine operation, and the callers in AudioState do not retry and ignore the StartRecording return value, so an error would leave audio silently broken with no recovery. Returning 0 also matches the existing behavior when no JS handler is registered.

Scope

Fully contained in this package. The request-id stays internal to react-native-webrtc and is stripped before the app-facing handler runs, so the public handler API is unchanged and no changes are needed in @livekit/react-native.

Testing

  • tsc --noEmit and eslint --max-warnings 0 pass.
  • Not yet built in a host app. Compilation and a real-device repro of the publish race are still to be done.

The six RTCAudioDeviceModule delegate callbacks blocked the native audio
thread on dispatch_semaphore_wait(..., DISPATCH_TIME_FOREVER) waiting for a
JS reply. If the JS thread is parked inside a blocking-synchronous bridge
call (e.g. peerConnectionAddTransceiver -> dispatch_sync(workerQueue) ->
libwebrtc -> worker thread -> this delegate), the reply never arrives and the
app freezes permanently (livekit/client-sdk-react-native#389, #89).

Bound each wait to 2s; on timeout log via os_log and return the default 0 so
the engine operation degrades gracefully instead of deadlocking.

To avoid a late resolve from a timed-out round being misattributed to the
next round (cross-round response aliasing), stamp every event with a
monotonic requestId that JS echoes back; the observer only accepts a resolve
whose id matches the in-flight round. A pre-send drain backstops the narrow
case where a matching resolve signals just past its round's deadline.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses an iOS deadlock where RTCAudioDeviceModule delegate callbacks block the native audio thread indefinitely while synchronously waiting for a JS response, which can circularly deadlock when JS is itself blocked in a synchronous bridge call into WebRTC.

Changes:

  • Add a bounded (2s) timeout for the JS round-trip wait in AudioDeviceModuleObserver, logging and returning default 0 on timeout to avoid permanent freezes.
  • Add a monotonic requestId to each native engine event and require JS to echo it back on the corresponding resolve call to prevent late/stale resolves from being misattributed.
  • Update the JS event wiring and the iOS exported resolve methods to pass/accept requestId.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/AudioDeviceModuleEvents.ts Echoes requestId back to native for each engine callback resolve while keeping app-facing handlers unchanged.
ios/RCTWebRTC/WebRTCModule+RTCAudioDeviceModule.m Extends the synchronous resolve methods to accept (requestId, result) and forwards to the observer.
ios/RCTWebRTC/AudioDeviceModuleObserver.m Implements bounded wait + request-id matching to break deadlocks and prevent stale responses from affecting later rounds.
ios/RCTWebRTC/AudioDeviceModuleObserver.h Updates observer resolve method signatures to include requestId.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…ound misattribution

dispatch_semaphore_signal ran after the @synchronized block in resolveRequestId.
A resolve preempted between releasing the lock and signalling could, across a
round boundary, wake the next round and hand it the previous round's result (and
drop the previous round's real result in favor of the timeout default). Moving
the signal inside the lock guarantees it is posted before the next round's
send-side critical section starts, so the pre-send drain reliably clears any
stray signal and a stale result can never be misattributed to a later round.
@hiroshihorie hiroshihorie merged commit 196cbb3 into master Jun 17, 2026
9 checks passed
@hiroshihorie hiroshihorie deleted the hiroshi/fix-adm-indefinite-lock branch June 17, 2026 18:49
hiroshihorie added a commit that referenced this pull request Jun 18, 2026
…gistered (#91)

## Summary

Builds on #90 (the bounded-wait deadlock fix) to remove the JS
round-trip entirely for AudioDeviceModule engine hooks that have no
registered handler, instead of only bounding the wait. For those hooks
this closes the deadlock window rather than capping it at 2 seconds.

Stacked on #90. This PR targets the #90 branch, so it should land after
#90 and the diff here is only the delta on top.

## Background

#90 bounds each of the six RTCAudioDeviceModule delegate waits to 2
seconds so a stuck JS thread can no longer freeze the app forever. But a
hook with no JS handler has nothing to wait for, so blocking it at all
is pure risk with no benefit.

## Change

Track per-hook handler registration with is-prefixed BOOL active flags.
The JS layer pushes a flag to native whenever a handler is set or
cleared. When a hook is inactive the observer returns 0 immediately
without sending the event or waiting, so the unhandled hooks never enter
the blocking path and cannot contribute to the deadlock.

In stock config this removes engineCreated, willStart, didStop and
willRelease from the blocking path entirely, including willStart, which
appeared in both reported freeze traces. The two hooks LiveKit registers
by default (willEnable and didDisable) keep the bounded-wait and
request-id safety net from #90.

The flags are written on the JS thread (handler registration) and read
on the native audio thread (delegate callbacks), so they are declared
atomic. The multi-field request-id state stays under @synchronized
because it needs a true critical section.

## Scope

Self-contained in this package. The active flags and request ids are
internal to react-native-webrtc and never reach app handlers, so the
public handler API is unchanged and no changes are needed in
@livekit/react-native.

## Testing

- npm run lint (eslint and tsc) passes.
- clang-format check passes.
- iOS and Android native builds run in CI.

Refs #89, livekit/client-sdk-react-native#389.
@davidliu davidliu mentioned this pull request Jun 18, 2026
davidliu added a commit that referenced this pull request Jun 18, 2026
a95d57d fix(ios): skip AudioDeviceModule JS round-trips when no handler
is registered (#91) ( Hiroshi Horie 2026-06-18 16:20:29 +0900)
c751bd6 ios: bump WebRTC-SDK to 144.7559.10 (#92) ( Hiroshi Horie
2026-06-18 16:11:31 +0900)
196cbb3 fix(ios): bound AudioDeviceModuleObserver JS waits to break the
deadlock (#90) ( Hiroshi Horie 2026-06-18 03:49:07 +0900)
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