From 5ed6ac1f25b0b793c74ffc23005a82530618eb55 Mon Sep 17 00:00:00 2001 From: Paige Sun Date: Sun, 5 Jun 2022 11:02:11 -0700 Subject: [PATCH] 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 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 --- Libraries/Network/RCTNetworking.mm | 62 ++++++++++++++++-------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/Libraries/Network/RCTNetworking.mm b/Libraries/Network/RCTNetworking.mm index c9004d648bed33..a0dcbed2b43aa2 100644 --- a/Libraries/Network/RCTNetworking.mm +++ b/Libraries/Network/RCTNetworking.mm @@ -179,6 +179,8 @@ - (void)invalidate { [super invalidate]; + std::lock_guard lock(_handlersLock); + for (NSNumber *requestID in _tasksByRequestID) { [_tasksByRequestID[requestID] cancel]; } @@ -203,37 +205,13 @@ - (void)invalidate if (!request.URL) { return nil; } - - { - std::lock_guard 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 a, id 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> *handlers = [self prioritizedHandlers]; + if (RCT_DEBUG) { // Check for handler conflicts float previousPriority = 0; id previousHandler = nil; - for (id handler in _handlers) { + for (id handler in handlers) { float priority = [handler respondsToSelector:@selector(handlerPriority)] ? [handler handlerPriority] : 0; if (previousHandler && priority < previousPriority) { return previousHandler; @@ -256,7 +234,7 @@ - (void)invalidate } // Normal code path - for (id handler in _handlers) { + for (id handler in handlers) { if ([handler canHandleRequest:request]) { return handler; } @@ -264,6 +242,34 @@ - (void)invalidate return nil; } +- (NSArray> *)prioritizedHandlers +{ + std::lock_guard lock(_handlersLock); + if (_handlers) { + return _handlers; + } + + NSArray> *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 a, id 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 *)stripNullsInRequestHeaders:(NSDictionary *)headers { NSMutableDictionary *result = [NSMutableDictionary dictionaryWithCapacity:headers.count];