Skip to content

Commit 5ed6ac1

Browse files
p-sunfacebook-github-bot
authored andcommitted
Protect _handlers in RCTNetworking with mutex even if RCTNetworking has been deallocated
Summary: Changelog: [iOS][Fixed][Internal] - Protect handlers in RCTNetworking with mutex even if RCTNetworking has been deallocated # The Logview We get this error in LogView: `Unhandled JS Exception: Error: Exception in HostFunction: mutex lock failed: Invalid argument`, which is an C++ std::error. "This typically happens when .lock() is called on a mutex that is not yet constructed, or has already been destructed." # Hypothesis of issue The LogView says the line that throws this softerror is [RCTNetworking.ios.js](https://github.com/facebook/react-native/blob/8bd3edec88148d0ab1f225d2119435681fbbba33/Libraries/Network/RCTNetworking.ios.js#L87). Inside RCTNetworking, there's only [one mutex and only one line where is is being used](https://github.com/facebook/react-native/blob/8bd3edec88148d0ab1f225d2119435681fbbba33/Libraries/Network/RCTNetworking.mm#L207-L215 ), to protect the _handlers array. My guess is that RCTNetworking was deallocated, which made `_handlersLock` nil, so it resulted in this error when we tried to lock it. # This diff * Add `std::lock_guard<std::mutex> lock(_handlersLock);` to `invalidate()` * Move `_handlersLock` logic to its own method instead of using a lambda. If `self` is being deallocated, I would guess that this method (`[self prioritizedHandlers]`) would return nil. Referencing this for correct ways to use lock_guard with mutex: https://devblogs.microsoft.com/oldnewthing/20210709-00/?p=105425 Reviewed By: fkgozali Differential Revision: D36886233 fbshipit-source-id: 60246f4d9bbc1d834497e4fb8a61d9c0e9623510
1 parent 0035cc9 commit 5ed6ac1

File tree

1 file changed

+34
-28
lines changed

1 file changed

+34
-28
lines changed

Libraries/Network/RCTNetworking.mm

Lines changed: 34 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,8 @@ - (void)invalidate
179179
{
180180
[super invalidate];
181181

182+
std::lock_guard<std::mutex> lock(_handlersLock);
183+
182184
for (NSNumber *requestID in _tasksByRequestID) {
183185
[_tasksByRequestID[requestID] cancel];
184186
}
@@ -203,37 +205,13 @@ - (void)invalidate
203205
if (!request.URL) {
204206
return nil;
205207
}
206-
207-
{
208-
std::lock_guard<std::mutex> lock(_handlersLock);
209-
210-
if (!_handlers) {
211-
if (_handlersProvider) {
212-
_handlers = _handlersProvider(self.moduleRegistry);
213-
} else {
214-
_handlers = [self.bridge modulesConformingToProtocol:@protocol(RCTURLRequestHandler)];
215-
}
216-
217-
// Get handlers, sorted in reverse priority order (highest priority first)
218-
_handlers = [_handlers sortedArrayUsingComparator:^NSComparisonResult(id<RCTURLRequestHandler> a, id<RCTURLRequestHandler> b) {
219-
float priorityA = [a respondsToSelector:@selector(handlerPriority)] ? [a handlerPriority] : 0;
220-
float priorityB = [b respondsToSelector:@selector(handlerPriority)] ? [b handlerPriority] : 0;
221-
if (priorityA > priorityB) {
222-
return NSOrderedAscending;
223-
} else if (priorityA < priorityB) {
224-
return NSOrderedDescending;
225-
} else {
226-
return NSOrderedSame;
227-
}
228-
}];
229-
}
230-
}
231-
208+
NSArray<id<RCTURLRequestHandler>> *handlers = [self prioritizedHandlers];
209+
232210
if (RCT_DEBUG) {
233211
// Check for handler conflicts
234212
float previousPriority = 0;
235213
id<RCTURLRequestHandler> previousHandler = nil;
236-
for (id<RCTURLRequestHandler> handler in _handlers) {
214+
for (id<RCTURLRequestHandler> handler in handlers) {
237215
float priority = [handler respondsToSelector:@selector(handlerPriority)] ? [handler handlerPriority] : 0;
238216
if (previousHandler && priority < previousPriority) {
239217
return previousHandler;
@@ -256,14 +234,42 @@ - (void)invalidate
256234
}
257235

258236
// Normal code path
259-
for (id<RCTURLRequestHandler> handler in _handlers) {
237+
for (id<RCTURLRequestHandler> handler in handlers) {
260238
if ([handler canHandleRequest:request]) {
261239
return handler;
262240
}
263241
}
264242
return nil;
265243
}
266244

245+
- (NSArray<id<RCTURLRequestHandler>> *)prioritizedHandlers
246+
{
247+
std::lock_guard<std::mutex> lock(_handlersLock);
248+
if (_handlers) {
249+
return _handlers;
250+
}
251+
252+
NSArray<id<RCTURLRequestHandler>> *newHandlers = _handlersProvider
253+
? _handlersProvider(self.moduleRegistry)
254+
: [self.bridge modulesConformingToProtocol:@protocol(RCTURLRequestHandler)];
255+
256+
// Get handlers, sorted in reverse priority order (highest priority first)
257+
newHandlers = [newHandlers sortedArrayUsingComparator:^NSComparisonResult(id<RCTURLRequestHandler> a, id<RCTURLRequestHandler> b) {
258+
float priorityA = [a respondsToSelector:@selector(handlerPriority)] ? [a handlerPriority] : 0;
259+
float priorityB = [b respondsToSelector:@selector(handlerPriority)] ? [b handlerPriority] : 0;
260+
if (priorityA > priorityB) {
261+
return NSOrderedAscending;
262+
} else if (priorityA < priorityB) {
263+
return NSOrderedDescending;
264+
} else {
265+
return NSOrderedSame;
266+
}
267+
}];
268+
269+
_handlers = newHandlers;
270+
return newHandlers;
271+
}
272+
267273
- (NSDictionary<NSString *, id> *)stripNullsInRequestHeaders:(NSDictionary<NSString *, id> *)headers
268274
{
269275
NSMutableDictionary *result = [NSMutableDictionary dictionaryWithCapacity:headers.count];

0 commit comments

Comments
 (0)