Skip to content

Commit 196cbb3

Browse files
authored
fix(ios): bound AudioDeviceModuleObserver JS waits to break the deadlock (#90)
## 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.
1 parent 7713a83 commit 196cbb3

4 files changed

Lines changed: 264 additions & 104 deletions

File tree

ios/RCTWebRTC/AudioDeviceModuleObserver.h

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,14 @@ NS_ASSUME_NONNULL_BEGIN
77

88
- (instancetype)initWithWebRTCModule:(WebRTCModule *)module;
99

10-
// Methods to receive results from JS
11-
- (void)resolveEngineCreatedWithResult:(NSInteger)result;
12-
- (void)resolveWillEnableEngineWithResult:(NSInteger)result;
13-
- (void)resolveWillStartEngineWithResult:(NSInteger)result;
14-
- (void)resolveDidStopEngineWithResult:(NSInteger)result;
15-
- (void)resolveDidDisableEngineWithResult:(NSInteger)result;
16-
- (void)resolveWillReleaseEngineWithResult:(NSInteger)result;
10+
// Methods to receive results from JS. requestId echoes the id sent with the
11+
// corresponding event so stale responses from timed-out rounds can be dropped.
12+
- (void)resolveEngineCreatedWithRequestId:(NSInteger)requestId result:(NSInteger)result;
13+
- (void)resolveWillEnableEngineWithRequestId:(NSInteger)requestId result:(NSInteger)result;
14+
- (void)resolveWillStartEngineWithRequestId:(NSInteger)requestId result:(NSInteger)result;
15+
- (void)resolveDidStopEngineWithRequestId:(NSInteger)requestId result:(NSInteger)result;
16+
- (void)resolveDidDisableEngineWithRequestId:(NSInteger)requestId result:(NSInteger)result;
17+
- (void)resolveWillReleaseEngineWithRequestId:(NSInteger)requestId result:(NSInteger)result;
1718

1819
@end
1920

ios/RCTWebRTC/AudioDeviceModuleObserver.m

Lines changed: 206 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,34 @@
11
#import "AudioDeviceModuleObserver.h"
22
#import <React/RCTLog.h>
3+
#import <os/log.h>
34

45
NS_ASSUME_NONNULL_BEGIN
56

7+
// Upper bound on how long a delegate callback parks the native audio thread
8+
// waiting for JS to respond. The wait used to be DISPATCH_TIME_FOREVER, which
9+
// deadlocks the app if the JS thread is itself blocked inside a synchronous
10+
// bridge call (e.g. peerConnectionAddTransceiver) that is transitively waiting
11+
// on this same audio operation. Bounding the wait turns that permanent freeze
12+
// into a recoverable stall: on timeout we return the default 0 ("proceed") so
13+
// the engine operation degrades gracefully.
14+
//
15+
// Caveat for willEnableEngine: returning 0 here lets the engine proceed even
16+
// though the JS handler (which configures/activates the AVAudioSession) never
17+
// completed. libwebrtc re-validates the session *category* after this callback
18+
// but not that it was *activated*, so a timeout on willEnableEngine can start
19+
// the engine against an unconfigured session. That degraded-but-recoverable
20+
// outcome is deliberately preferred over the unrecoverable deadlock.
21+
static const int64_t kJSResponseTimeoutSeconds = 2;
22+
23+
static os_log_t ADMObserverLog(void) {
24+
static os_log_t log;
25+
static dispatch_once_t onceToken;
26+
dispatch_once(&onceToken, ^{
27+
log = os_log_create("com.livekit.react-native-webrtc", "AudioDeviceModuleObserver");
28+
});
29+
return log;
30+
}
31+
632
@interface AudioDeviceModuleObserver ()
733

834
@property(weak, nonatomic) WebRTCModule *module;
@@ -20,6 +46,15 @@ @interface AudioDeviceModuleObserver ()
2046
@property(nonatomic, assign) NSInteger didDisableEngineResult;
2147
@property(nonatomic, assign) NSInteger willReleaseEngineResult;
2248

49+
// Monotonic id stamped on every event sent to JS, and the id of the round
50+
// currently being awaited (0 = none). JS echoes the id back when it resolves;
51+
// the observer only accepts a resolve whose id matches the in-flight round, so a
52+
// late resolve from a round that already timed out cannot be misattributed to
53+
// the next round. Both are guarded by @synchronized(self) since the send side
54+
// runs on the native audio thread and the resolve side on the JS thread.
55+
@property(nonatomic, assign) NSInteger requestIdSeq;
56+
@property(nonatomic, assign) NSInteger awaitingRequestId;
57+
2358
@end
2459

2560
@implementation AudioDeviceModuleObserver
@@ -38,6 +73,57 @@ - (instancetype)initWithWebRTCModule:(WebRTCModule *)module {
3873
return self;
3974
}
4075

76+
#pragma mark - Bounded JS round-trip
77+
78+
// Sends an event to JS and blocks the calling (native audio) thread until JS
79+
// resolves the matching semaphore or kJSResponseTimeoutSeconds elapses.
80+
//
81+
// Each round is tagged with a unique requestId that JS echoes back on resolve;
82+
// -resolveRequestId:... drops any resolve whose id does not match the in-flight
83+
// round, so a late resolve from a previously timed-out round cannot signal this
84+
// round's semaphore. The pre-send drain below is the remaining safety net: it
85+
// clears a stray signal from the narrow case where a matching resolve raced the
86+
// previous round past its timeout deadline (signalling after that round had
87+
// already given up). No legitimate signal for *this* round can exist at drain
88+
// time because the event has not been sent yet.
89+
//
90+
// Returns the JS-provided result on success, or 0 on timeout.
91+
- (NSInteger)sendEventAndWaitWithName:(NSString *)eventName
92+
body:(NSDictionary *)body
93+
semaphore:(dispatch_semaphore_t)semaphore
94+
resultBlock:(NSInteger (^)(void))resultBlock {
95+
NSInteger requestId;
96+
@synchronized(self) {
97+
requestId = ++self.requestIdSeq;
98+
self.awaitingRequestId = requestId;
99+
}
100+
101+
while (dispatch_semaphore_wait(semaphore, DISPATCH_TIME_NOW) == 0) {
102+
// Drain a stray signal left by a resolve that raced the previous round's timeout.
103+
}
104+
105+
NSMutableDictionary *payload = [body mutableCopy];
106+
payload[@"requestId"] = @(requestId);
107+
[self.module sendEventWithName:eventName body:payload];
108+
109+
dispatch_time_t deadline = dispatch_time(DISPATCH_TIME_NOW, kJSResponseTimeoutSeconds * NSEC_PER_SEC);
110+
if (dispatch_semaphore_wait(semaphore, deadline) != 0) {
111+
@synchronized(self) {
112+
// Stop accepting this round's resolve; if it arrives now it is stale.
113+
if (self.awaitingRequestId == requestId) {
114+
self.awaitingRequestId = 0;
115+
}
116+
}
117+
os_log_error(ADMObserverLog(),
118+
"Timed out after %llds waiting for JS to respond to %{public}@; returning default 0",
119+
(long long)kJSResponseTimeoutSeconds,
120+
eventName);
121+
return 0;
122+
}
123+
124+
return resultBlock();
125+
}
126+
41127
#pragma mark - RTCAudioDeviceModuleDelegate
42128

43129
- (void)audioDeviceModule:(RTCAudioDeviceModule *)audioDeviceModule
@@ -55,13 +141,15 @@ - (void)audioDeviceModule:(RTCAudioDeviceModule *)audioDeviceModule
55141
- (NSInteger)audioDeviceModule:(RTCAudioDeviceModule *)audioDeviceModule didCreateEngine:(AVAudioEngine *)engine {
56142
RCTLog(@"[AudioDeviceModuleObserver] Engine created - waiting for JS response");
57143

58-
[self.module sendEventWithName:kEventAudioDeviceModuleEngineCreated body:@{}];
59-
60-
// Wait indefinitely for JS to respond
61-
dispatch_semaphore_wait(self.engineCreatedSemaphore, DISPATCH_TIME_FOREVER);
144+
NSInteger result = [self sendEventAndWaitWithName:kEventAudioDeviceModuleEngineCreated
145+
body:@{}
146+
semaphore:self.engineCreatedSemaphore
147+
resultBlock:^NSInteger {
148+
return self.engineCreatedResult;
149+
}];
62150

63-
RCTLog(@"[AudioDeviceModuleObserver] Engine created - JS returned: %ld", (long)self.engineCreatedResult);
64-
return self.engineCreatedResult;
151+
RCTLog(@"[AudioDeviceModuleObserver] Engine created - JS returned: %ld", (long)result);
152+
return result;
65153
}
66154

67155
- (NSInteger)audioDeviceModule:(RTCAudioDeviceModule *)audioDeviceModule
@@ -72,21 +160,22 @@ - (NSInteger)audioDeviceModule:(RTCAudioDeviceModule *)audioDeviceModule
72160
isPlayoutEnabled,
73161
isRecordingEnabled);
74162

75-
[self.module sendEventWithName:kEventAudioDeviceModuleEngineWillEnable
76-
body:@{
77-
@"isPlayoutEnabled" : @(isPlayoutEnabled),
78-
@"isRecordingEnabled" : @(isRecordingEnabled),
79-
}];
163+
NSInteger result = [self sendEventAndWaitWithName:kEventAudioDeviceModuleEngineWillEnable
164+
body:@{
165+
@"isPlayoutEnabled" : @(isPlayoutEnabled),
166+
@"isRecordingEnabled" : @(isRecordingEnabled),
167+
}
168+
semaphore:self.willEnableEngineSemaphore
169+
resultBlock:^NSInteger {
170+
return self.willEnableEngineResult;
171+
}];
80172

81-
// Wait indefinitely for JS to respond
82-
dispatch_semaphore_wait(self.willEnableEngineSemaphore, DISPATCH_TIME_FOREVER);
83-
84-
RCTLog(@"[AudioDeviceModuleObserver] Engine will enable - JS returned: %ld", (long)self.willEnableEngineResult);
173+
RCTLog(@"[AudioDeviceModuleObserver] Engine will enable - JS returned: %ld", (long)result);
85174

86175
AVAudioSession *audioSession = [AVAudioSession sharedInstance];
87176
RCTLog(@"[AudioDeviceModuleObserver] Audio session category: %@", audioSession.category);
88177

89-
return self.willEnableEngineResult;
178+
return result;
90179
}
91180

92181
- (NSInteger)audioDeviceModule:(RTCAudioDeviceModule *)audioDeviceModule
@@ -97,17 +186,18 @@ - (NSInteger)audioDeviceModule:(RTCAudioDeviceModule *)audioDeviceModule
97186
isPlayoutEnabled,
98187
isRecordingEnabled);
99188

100-
[self.module sendEventWithName:kEventAudioDeviceModuleEngineWillStart
101-
body:@{
102-
@"isPlayoutEnabled" : @(isPlayoutEnabled),
103-
@"isRecordingEnabled" : @(isRecordingEnabled),
104-
}];
105-
106-
// Wait indefinitely for JS to respond
107-
dispatch_semaphore_wait(self.willStartEngineSemaphore, DISPATCH_TIME_FOREVER);
108-
109-
RCTLog(@"[AudioDeviceModuleObserver] Engine will start - JS returned: %ld", (long)self.willStartEngineResult);
110-
return self.willStartEngineResult;
189+
NSInteger result = [self sendEventAndWaitWithName:kEventAudioDeviceModuleEngineWillStart
190+
body:@{
191+
@"isPlayoutEnabled" : @(isPlayoutEnabled),
192+
@"isRecordingEnabled" : @(isRecordingEnabled),
193+
}
194+
semaphore:self.willStartEngineSemaphore
195+
resultBlock:^NSInteger {
196+
return self.willStartEngineResult;
197+
}];
198+
199+
RCTLog(@"[AudioDeviceModuleObserver] Engine will start - JS returned: %ld", (long)result);
200+
return result;
111201
}
112202

113203
- (NSInteger)audioDeviceModule:(RTCAudioDeviceModule *)audioDeviceModule
@@ -118,17 +208,18 @@ - (NSInteger)audioDeviceModule:(RTCAudioDeviceModule *)audioDeviceModule
118208
isPlayoutEnabled,
119209
isRecordingEnabled);
120210

121-
[self.module sendEventWithName:kEventAudioDeviceModuleEngineDidStop
122-
body:@{
123-
@"isPlayoutEnabled" : @(isPlayoutEnabled),
124-
@"isRecordingEnabled" : @(isRecordingEnabled),
125-
}];
126-
127-
// Wait indefinitely for JS to respond
128-
dispatch_semaphore_wait(self.didStopEngineSemaphore, DISPATCH_TIME_FOREVER);
129-
130-
RCTLog(@"[AudioDeviceModuleObserver] Engine did stop - JS returned: %ld", (long)self.didStopEngineResult);
131-
return self.didStopEngineResult;
211+
NSInteger result = [self sendEventAndWaitWithName:kEventAudioDeviceModuleEngineDidStop
212+
body:@{
213+
@"isPlayoutEnabled" : @(isPlayoutEnabled),
214+
@"isRecordingEnabled" : @(isRecordingEnabled),
215+
}
216+
semaphore:self.didStopEngineSemaphore
217+
resultBlock:^NSInteger {
218+
return self.didStopEngineResult;
219+
}];
220+
221+
RCTLog(@"[AudioDeviceModuleObserver] Engine did stop - JS returned: %ld", (long)result);
222+
return result;
132223
}
133224

134225
- (NSInteger)audioDeviceModule:(RTCAudioDeviceModule *)audioDeviceModule
@@ -139,29 +230,32 @@ - (NSInteger)audioDeviceModule:(RTCAudioDeviceModule *)audioDeviceModule
139230
isPlayoutEnabled,
140231
isRecordingEnabled);
141232

142-
[self.module sendEventWithName:kEventAudioDeviceModuleEngineDidDisable
143-
body:@{
144-
@"isPlayoutEnabled" : @(isPlayoutEnabled),
145-
@"isRecordingEnabled" : @(isRecordingEnabled),
146-
}];
147-
148-
// Wait indefinitely for JS to respond
149-
dispatch_semaphore_wait(self.didDisableEngineSemaphore, DISPATCH_TIME_FOREVER);
150-
151-
RCTLog(@"[AudioDeviceModuleObserver] Engine did disable - JS returned: %ld", (long)self.didDisableEngineResult);
152-
return self.didDisableEngineResult;
233+
NSInteger result = [self sendEventAndWaitWithName:kEventAudioDeviceModuleEngineDidDisable
234+
body:@{
235+
@"isPlayoutEnabled" : @(isPlayoutEnabled),
236+
@"isRecordingEnabled" : @(isRecordingEnabled),
237+
}
238+
semaphore:self.didDisableEngineSemaphore
239+
resultBlock:^NSInteger {
240+
return self.didDisableEngineResult;
241+
}];
242+
243+
RCTLog(@"[AudioDeviceModuleObserver] Engine did disable - JS returned: %ld", (long)result);
244+
return result;
153245
}
154246

155247
- (NSInteger)audioDeviceModule:(RTCAudioDeviceModule *)audioDeviceModule willReleaseEngine:(AVAudioEngine *)engine {
156248
RCTLog(@"[AudioDeviceModuleObserver] Engine will release - waiting for JS response");
157249

158-
[self.module sendEventWithName:kEventAudioDeviceModuleEngineWillRelease body:@{}];
159-
160-
// Wait indefinitely for JS to respond
161-
dispatch_semaphore_wait(self.willReleaseEngineSemaphore, DISPATCH_TIME_FOREVER);
250+
NSInteger result = [self sendEventAndWaitWithName:kEventAudioDeviceModuleEngineWillRelease
251+
body:@{}
252+
semaphore:self.willReleaseEngineSemaphore
253+
resultBlock:^NSInteger {
254+
return self.willReleaseEngineResult;
255+
}];
162256

163-
RCTLog(@"[AudioDeviceModuleObserver] Engine will release - JS returned: %ld", (long)self.willReleaseEngineResult);
164-
return self.willReleaseEngineResult;
257+
RCTLog(@"[AudioDeviceModuleObserver] Engine will release - JS returned: %ld", (long)result);
258+
return result;
165259
}
166260

167261
- (NSInteger)audioDeviceModule:(RTCAudioDeviceModule *)audioDeviceModule
@@ -192,34 +286,73 @@ - (void)audioDeviceModuleDidUpdateDevices:(RTCAudioDeviceModule *)audioDeviceMod
192286

193287
#pragma mark - Resolve methods from JS
194288

195-
- (void)resolveEngineCreatedWithResult:(NSInteger)result {
196-
self.engineCreatedResult = result;
197-
dispatch_semaphore_signal(self.engineCreatedSemaphore);
289+
// Applies a JS response only if its requestId matches the round currently being
290+
// awaited. A non-matching id means the round already timed out and moved on, so
291+
// the response is dropped without touching the result or signalling — preventing
292+
// a stale value from being handed to a later round.
293+
- (void)resolveRequestId:(NSInteger)requestId store:(void (^)(void))store semaphore:(dispatch_semaphore_t)semaphore {
294+
@synchronized(self) {
295+
if (requestId != self.awaitingRequestId) {
296+
return;
297+
}
298+
self.awaitingRequestId = 0;
299+
store();
300+
// Signal inside the lock so this signal is always posted before the next
301+
// round's send-side @synchronized block can start. Otherwise a resolve
302+
// preempted between releasing the lock and signalling could, across a round
303+
// boundary, wake the next round and hand it this round's result. Keeping it
304+
// inside the lock also lets the next round's pre-send drain reliably clear
305+
// any stray signal left by a resolve that raced its own timeout.
306+
dispatch_semaphore_signal(semaphore);
307+
}
308+
}
309+
310+
- (void)resolveEngineCreatedWithRequestId:(NSInteger)requestId result:(NSInteger)result {
311+
[self resolveRequestId:requestId
312+
store:^{
313+
self.engineCreatedResult = result;
314+
}
315+
semaphore:self.engineCreatedSemaphore];
198316
}
199317

200-
- (void)resolveWillEnableEngineWithResult:(NSInteger)result {
201-
self.willEnableEngineResult = result;
202-
dispatch_semaphore_signal(self.willEnableEngineSemaphore);
318+
- (void)resolveWillEnableEngineWithRequestId:(NSInteger)requestId result:(NSInteger)result {
319+
[self resolveRequestId:requestId
320+
store:^{
321+
self.willEnableEngineResult = result;
322+
}
323+
semaphore:self.willEnableEngineSemaphore];
203324
}
204325

205-
- (void)resolveWillStartEngineWithResult:(NSInteger)result {
206-
self.willStartEngineResult = result;
207-
dispatch_semaphore_signal(self.willStartEngineSemaphore);
326+
- (void)resolveWillStartEngineWithRequestId:(NSInteger)requestId result:(NSInteger)result {
327+
[self resolveRequestId:requestId
328+
store:^{
329+
self.willStartEngineResult = result;
330+
}
331+
semaphore:self.willStartEngineSemaphore];
208332
}
209333

210-
- (void)resolveDidStopEngineWithResult:(NSInteger)result {
211-
self.didStopEngineResult = result;
212-
dispatch_semaphore_signal(self.didStopEngineSemaphore);
334+
- (void)resolveDidStopEngineWithRequestId:(NSInteger)requestId result:(NSInteger)result {
335+
[self resolveRequestId:requestId
336+
store:^{
337+
self.didStopEngineResult = result;
338+
}
339+
semaphore:self.didStopEngineSemaphore];
213340
}
214341

215-
- (void)resolveDidDisableEngineWithResult:(NSInteger)result {
216-
self.didDisableEngineResult = result;
217-
dispatch_semaphore_signal(self.didDisableEngineSemaphore);
342+
- (void)resolveDidDisableEngineWithRequestId:(NSInteger)requestId result:(NSInteger)result {
343+
[self resolveRequestId:requestId
344+
store:^{
345+
self.didDisableEngineResult = result;
346+
}
347+
semaphore:self.didDisableEngineSemaphore];
218348
}
219349

220-
- (void)resolveWillReleaseEngineWithResult:(NSInteger)result {
221-
self.willReleaseEngineResult = result;
222-
dispatch_semaphore_signal(self.willReleaseEngineSemaphore);
350+
- (void)resolveWillReleaseEngineWithRequestId:(NSInteger)requestId result:(NSInteger)result {
351+
[self resolveRequestId:requestId
352+
store:^{
353+
self.willReleaseEngineResult = result;
354+
}
355+
semaphore:self.willReleaseEngineSemaphore];
223356
}
224357

225358
@end

0 commit comments

Comments
 (0)