Skip to content

Commit

Permalink
Protect _handlers in RCTNetworking with mutex even if RCTNetworking h…
Browse files Browse the repository at this point in the history
…as 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
  • Loading branch information
p-sun authored and facebook-github-bot committed Jun 5, 2022
1 parent 0035cc9 commit 5ed6ac1
Showing 1 changed file with 34 additions and 28 deletions.
62 changes: 34 additions & 28 deletions Libraries/Network/RCTNetworking.mm
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ - (void)invalidate
{
[super invalidate];

std::lock_guard<std::mutex> lock(_handlersLock);

for (NSNumber *requestID in _tasksByRequestID) {
[_tasksByRequestID[requestID] cancel];
}
Expand All @@ -203,37 +205,13 @@ - (void)invalidate
if (!request.URL) {
return nil;
}

{
std::lock_guard<std::mutex> lock(_handlersLock);

if (!_handlers) {
if (_handlersProvider) {
_handlers = _handlersProvider(self.moduleRegistry);
} else {
_handlers = [self.bridge modulesConformingToProtocol:@protocol(RCTURLRequestHandler)];
}

// Get handlers, sorted in reverse priority order (highest priority first)
_handlers = [_handlers sortedArrayUsingComparator:^NSComparisonResult(id<RCTURLRequestHandler> a, id<RCTURLRequestHandler> b) {
float priorityA = [a respondsToSelector:@selector(handlerPriority)] ? [a handlerPriority] : 0;
float priorityB = [b respondsToSelector:@selector(handlerPriority)] ? [b handlerPriority] : 0;
if (priorityA > priorityB) {
return NSOrderedAscending;
} else if (priorityA < priorityB) {
return NSOrderedDescending;
} else {
return NSOrderedSame;
}
}];
}
}

NSArray<id<RCTURLRequestHandler>> *handlers = [self prioritizedHandlers];

if (RCT_DEBUG) {
// Check for handler conflicts
float previousPriority = 0;
id<RCTURLRequestHandler> previousHandler = nil;
for (id<RCTURLRequestHandler> handler in _handlers) {
for (id<RCTURLRequestHandler> handler in handlers) {
float priority = [handler respondsToSelector:@selector(handlerPriority)] ? [handler handlerPriority] : 0;
if (previousHandler && priority < previousPriority) {
return previousHandler;
Expand All @@ -256,14 +234,42 @@ - (void)invalidate
}

// Normal code path
for (id<RCTURLRequestHandler> handler in _handlers) {
for (id<RCTURLRequestHandler> handler in handlers) {
if ([handler canHandleRequest:request]) {
return handler;
}
}
return nil;
}

- (NSArray<id<RCTURLRequestHandler>> *)prioritizedHandlers
{
std::lock_guard<std::mutex> lock(_handlersLock);
if (_handlers) {
return _handlers;
}

NSArray<id<RCTURLRequestHandler>> *newHandlers = _handlersProvider
? _handlersProvider(self.moduleRegistry)
: [self.bridge modulesConformingToProtocol:@protocol(RCTURLRequestHandler)];

// Get handlers, sorted in reverse priority order (highest priority first)
newHandlers = [newHandlers sortedArrayUsingComparator:^NSComparisonResult(id<RCTURLRequestHandler> a, id<RCTURLRequestHandler> b) {
float priorityA = [a respondsToSelector:@selector(handlerPriority)] ? [a handlerPriority] : 0;
float priorityB = [b respondsToSelector:@selector(handlerPriority)] ? [b handlerPriority] : 0;
if (priorityA > priorityB) {
return NSOrderedAscending;
} else if (priorityA < priorityB) {
return NSOrderedDescending;
} else {
return NSOrderedSame;
}
}];

_handlers = newHandlers;
return newHandlers;
}

- (NSDictionary<NSString *, id> *)stripNullsInRequestHeaders:(NSDictionary<NSString *, id> *)headers
{
NSMutableDictionary *result = [NSMutableDictionary dictionaryWithCapacity:headers.count];
Expand Down

0 comments on commit 5ed6ac1

Please sign in to comment.