From 9b76e217bb16935069f0ea5b60f4c4d9b73f86d6 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Thu, 17 Sep 2020 13:52:44 -0700 Subject: [PATCH] Copy all blocks generated by TurboModules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: The Apple documentation states: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Blocks/Articles/bxUsing.html#//apple_ref/doc/uid/TP40007502-CH5-SW1 > Typically, you shouldn’t need to copy (or retain) a block. You only need to make a copy when you expect the block to be used after destruction of the scope within which it was declared. Copying moves a block to the heap. All blocks generated in the TurboModule infra, for callbacks and promise resolve/reject handlers, can be used after the destruction of the scope within which they were declared. Therefore, let's copy them in the hopes that they mitigate T75876134. **Note:** We copy blocks before pushing them into the `retainedObjects` array in the legacy Infra as well. Context: D2559997 (https://github.com/facebook/react-native/commit/71da2917e577b7ec659083408cff7f9981d6600f), D5589246 (https://github.com/facebook/react-native/commit/2a6965df9063c795b3d3098c4db76e5f595ba44f) Changelog: [Internal] Reviewed By: fkgozali Differential Revision: D23764329 fbshipit-source-id: e71360977bdff74dc570bd40f0139792860f811f --- .../core/platform/ios/RCTTurboModule.mm | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.mm b/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.mm index 4e5cdc8a4e..c8950c5718 100644 --- a/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.mm +++ b/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.mm @@ -171,7 +171,7 @@ convertJSIFunctionToCallback(jsi::Runtime &runtime, const jsi::Function &value, { auto weakWrapper = CallbackWrapper::createWeak(value.getFunction(runtime), runtime, jsInvoker); BOOL __block wrapperWasCalled = NO; - return ^(NSArray *responses) { + RCTResponseSenderBlock callback = ^(NSArray *responses) { if (wrapperWasCalled) { throw std::runtime_error("callback arg cannot be called more than once"); } @@ -194,6 +194,8 @@ convertJSIFunctionToCallback(jsi::Runtime &runtime, const jsi::Function &value, wrapperWasCalled = YES; }; + + return [callback copy]; } namespace facebook { @@ -331,12 +333,11 @@ jsi::Value ObjCTurboModule::performMethodInvocation( bool wasMethodSync = isMethodSync(returnType); void (^block)() = ^{ - if (!weakModule) { + id strongModule = weakModule; + if (!strongModule) { return; } - id strongModule = weakModule; - if (wasMethodSync) { TurboModulePerfLogger::syncMethodCallExecutionStart(moduleName, methodNameStr.c_str()); } else { @@ -635,10 +636,13 @@ jsi::Value ObjCTurboModule::invokeObjCMethod( runtime, jsInvoker_, ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock) { - [inv setArgument:(void *)&resolveBlock atIndex:count + 2]; - [inv setArgument:(void *)&rejectBlock atIndex:count + 3]; - [retainedObjectsForInvocation addObject:resolveBlock]; - [retainedObjectsForInvocation addObject:rejectBlock]; + RCTPromiseResolveBlock resolveCopy = [resolveBlock copy]; + RCTPromiseRejectBlock rejectCopy = [rejectBlock copy]; + + [inv setArgument:(void *)&resolveCopy atIndex:count + 2]; + [inv setArgument:(void *)&rejectCopy atIndex:count + 3]; + [retainedObjectsForInvocation addObject:resolveCopy]; + [retainedObjectsForInvocation addObject:rejectCopy]; // The return type becomes void in the ObjC side. performMethodInvocation(runtime, VoidKind, methodName, inv, retainedObjectsForInvocation); })