From a251316a5f0f96b81a4741998136b7f09ebe11af Mon Sep 17 00:00:00 2001 From: Tadeu Zagallo Date: Tue, 7 Jul 2015 18:23:49 -0700 Subject: [PATCH] [ReactNative] Fix RCTJavaScriptContext deallocation Summary: The context wasn't being explicitly released before, since it'd be immediately released. Now that the executors are bridge modules, it was only being deallocated when the modules were released, what caused the threads to not be released at all. --- React/Base/RCTBatchedBridge.m | 65 ++++++++++------------------ React/Executors/RCTContextExecutor.m | 1 + 2 files changed, 25 insertions(+), 41 deletions(-) diff --git a/React/Base/RCTBatchedBridge.m b/React/Base/RCTBatchedBridge.m index b1bb6cf1a..9b77ad466 100644 --- a/React/Base/RCTBatchedBridge.m +++ b/React/Base/RCTBatchedBridge.m @@ -347,53 +347,36 @@ RCT_NOT_IMPLEMENTED(-initWithBundleURL:(__unused NSURL *)bundleURL RCTLatestExecutor = nil; } - void (^mainThreadInvalidate)(void) = ^{ - RCTAssertMainThread(); + [_mainDisplayLink invalidate]; + _mainDisplayLink = nil; - [_mainDisplayLink invalidate]; - _mainDisplayLink = nil; - - // Invalidate modules - dispatch_group_t group = dispatch_group_create(); - for (RCTModuleData *moduleData in _modules) { - if ([moduleData.instance respondsToSelector:@selector(invalidate)]) { - [moduleData dispatchBlock:^{ - [(id)moduleData.instance invalidate]; - } dispatchGroup:group]; - } - moduleData.queue = nil; + // Invalidate modules + dispatch_group_t group = dispatch_group_create(); + for (RCTModuleData *moduleData in _modules) { + if (moduleData.instance == _javaScriptExecutor) { + continue; } - dispatch_group_notify(group, dispatch_get_main_queue(), ^{ - _modules = nil; - _modulesByName = nil; - _frameUpdateObservers = nil; - }); - }; - if (!_javaScriptExecutor) { - - // No JS thread running - mainThreadInvalidate(); - return; + if ([moduleData.instance respondsToSelector:@selector(invalidate)]) { + [moduleData dispatchBlock:^{ + [(id)moduleData.instance invalidate]; + } dispatchGroup:group]; + } + moduleData.queue = nil; } + dispatch_group_notify(group, dispatch_get_main_queue(), ^{ + [_javaScriptExecutor executeBlockOnJavaScriptQueue:^{ + [_jsDisplayLink invalidate]; + _jsDisplayLink = nil; - [_javaScriptExecutor executeBlockOnJavaScriptQueue:^{ + [_javaScriptExecutor invalidate]; + _javaScriptExecutor = nil; + }]; - /** - * JS Thread deallocations - */ - [_javaScriptExecutor invalidate]; - _javaScriptExecutor = nil; - - [_jsDisplayLink invalidate]; - _jsDisplayLink = nil; - - /** - * Main Thread deallocations - */ - dispatch_async(dispatch_get_main_queue(), mainThreadInvalidate); - - }]; + _modules = nil; + _modulesByName = nil; + _frameUpdateObservers = nil; + }); } #pragma mark - RCTBridge methods diff --git a/React/Executors/RCTContextExecutor.m b/React/Executors/RCTContextExecutor.m index 200a3f645..0b922d4d0 100644 --- a/React/Executors/RCTContextExecutor.m +++ b/React/Executors/RCTContextExecutor.m @@ -320,6 +320,7 @@ static NSError *RCTNSErrorFromJSError(JSContextRef context, JSValueRef jsError) onThread:_javaScriptThread withObject:nil waitUntilDone:NO]; + _context = nil; } - (void)dealloc