Skip to content

Commit

Permalink
Call stopObserving on correct queue
Browse files Browse the repository at this point in the history
Summary:
Since `dealloc` can be called from any thread, this would result `stopObserving` being called on a different thread/queue than the specified `methodQueue`. We specifically encountered this issue with a module needing the main queue having its `stopObserving` called on a background queue.

Changelog:
[iOS][Fixed] - Call [RCTEventEmitter stopObserving] on specified method queue

Reviewed By: RSNara

Differential Revision: D23821741

fbshipit-source-id: 693c3be6876f863da6dd214a829af2cc13a09c3f
  • Loading branch information
appden authored and facebook-github-bot committed Sep 22, 2020
1 parent 18f29db commit 23717e4
Show file tree
Hide file tree
Showing 9 changed files with 13 additions and 9 deletions.
1 change: 1 addition & 0 deletions Libraries/NativeAnimation/RCTNativeAnimatedModule.mm
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ - (instancetype)init

- (void)invalidate
{
[super invalidate];
[_nodesManager stopAnimationLoop];
[self.bridge.eventDispatcher removeDispatchObserver:self];
[self.bridge.uiManager.observerCoordinator removeObserver:self];
Expand Down
1 change: 1 addition & 0 deletions Libraries/NativeAnimation/RCTNativeAnimatedTurboModule.mm
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ - (instancetype)init

- (void)invalidate
{
[super invalidate];
[_nodesManager stopAnimationLoop];
[self.bridge.eventDispatcher removeDispatchObserver:self];
[self.bridge.uiManager.observerCoordinator removeObserver:self];
Expand Down
4 changes: 3 additions & 1 deletion Libraries/Network/RCTNetworking.mm
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ - (instancetype)initWithHandlersProvider:(NSArray<id<RCTURLRequestHandler>> * (^

- (void)invalidate
{
[super invalidate];

for (NSNumber *requestID in _tasksByRequestID) {
[_tasksByRequestID[requestID] cancel];
}
Expand Down Expand Up @@ -680,7 +682,7 @@ - (RCTNetworkTask *)networkTaskWithRequest:(NSURLRequest *)request completionBlo
@"timeout": @(query.timeout()),
@"withCredentials": @(query.withCredentials()),
};

// TODO: buildRequest returns a cancellation block, but there's currently
// no way to invoke it, if, for example the request is cancelled while
// loading a large file to build the request body
Expand Down
2 changes: 1 addition & 1 deletion React/CoreModules/RCTAppState.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@

#import <React/RCTEventEmitter.h>

@interface RCTAppState : RCTEventEmitter <RCTInvalidating>
@interface RCTAppState : RCTEventEmitter

@end
5 changes: 0 additions & 5 deletions React/CoreModules/RCTAppState.mm
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,6 @@ - (void)stopObserving
[[NSNotificationCenter defaultCenter] removeObserver:self];
}

- (void)invalidate
{
[self stopObserving];
}

#pragma mark - App Notification Methods

- (void)handleMemoryWarning
Expand Down
1 change: 1 addition & 0 deletions React/CoreModules/RCTDevSettings.mm
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ - (dispatch_queue_t)methodQueue

- (void)invalidate
{
[super invalidate];
#if ENABLE_PACKAGER_CONNECTION
[[RCTPackagerConnection sharedPackagerConnection] removeHandler:_reloadToken];
#endif
Expand Down
2 changes: 2 additions & 0 deletions React/CoreModules/RCTWebSocketModule.mm
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ - (NSArray *)supportedEvents

- (void)invalidate
{
[super invalidate];

_contentHandlers = nil;
for (RCTSRWebSocket *socket in _sockets.allValues) {
socket.delegate = nil;
Expand Down
4 changes: 3 additions & 1 deletion React/Modules/RCTEventEmitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* RCTEventEmitter is an abstract base class to be used for modules that emit
* events to be observed by JS.
*/
@interface RCTEventEmitter : NSObject <RCTBridgeModule, RCTJSInvokerModule>
@interface RCTEventEmitter : NSObject <RCTBridgeModule, RCTJSInvokerModule, RCTInvalidating>

@property (nonatomic, weak) RCTBridge *bridge;

Expand All @@ -37,6 +37,8 @@
- (void)startObserving;
- (void)stopObserving;

- (void)invalidate NS_REQUIRES_SUPER;

- (void)addListener:(NSString *)eventName;
- (void)removeListeners:(double)count;

Expand Down
2 changes: 1 addition & 1 deletion React/Modules/RCTEventEmitter.m
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ - (void)stopObserving
// Does nothing
}

- (void)dealloc
- (void)invalidate
{
if (_listenerCount > 0) {
[self stopObserving];
Expand Down

0 comments on commit 23717e4

Please sign in to comment.