From 5275895af5136bc278c0c5eb07ae93e395c5b29b Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Thu, 17 Dec 2020 17:22:11 -0800 Subject: [PATCH] Roll out TurboModule block copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: ## Description During TurboModule method invocation, when we convert JavaScript callbacks (i.e: jsi::Functions) into ObjC blocks inside the TurboModule jsi::HostObject, we [push them into an array](https://fburl.com/diffusion/r1n75ikx) that [gets cleared after the method gets invoked](https://fburl.com/diffusion/ubbvdmfs). Prior to this experiment, we pushed these ObjC blocks into the array without copying them first. This goes contrary to ObjC best practices, which suggest that if you need to retain a block past its creation context for later invocation, you should defensively copy it to prevent it from being freed early. See the [Apple docs](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. The diff that introduced the fix: D23764329 (https://github.com/facebook/react-native/commit/9b76e217bb16935069f0ea5b60f4c4d9b73f86d6). Reviewed By: PeteTheHeat Differential Revision: D25617672 fbshipit-source-id: 24863b31a2d82c8d39a91c8ea8eb3a62724b800a --- React/Base/RCTBridge.h | 4 ---- React/Base/RCTBridge.m | 11 ----------- .../core/platform/ios/RCTTurboModule.mm | 15 +++------------ 3 files changed, 3 insertions(+), 27 deletions(-) diff --git a/React/Base/RCTBridge.h b/React/Base/RCTBridge.h index 5ce3ebe35a3c39..d86a0d27b97b20 100644 --- a/React/Base/RCTBridge.h +++ b/React/Base/RCTBridge.h @@ -160,10 +160,6 @@ RCT_EXTERN void RCTEnableTurboModuleEagerInit(BOOL enabled); RCT_EXTERN BOOL RCTTurboModuleSharedMutexInitEnabled(void); RCT_EXTERN void RCTEnableTurboModuleSharedMutexInit(BOOL enabled); -// Turn on TurboModule block copy -RCT_EXTERN BOOL RCTTurboModuleBlockCopyEnabled(void); -RCT_EXTERN void RCTEnableTurboModuleBlockCopy(BOOL enabled); - // Turn on TurboModule JS Codegen RCT_EXTERN BOOL RCTTurboModuleJSCodegenEnabled(void); RCT_EXTERN void RCTEnableTurboModuleJSCodegen(BOOL enabled); diff --git a/React/Base/RCTBridge.m b/React/Base/RCTBridge.m index 1adc463bf060e1..d8d5594a12fa1b 100644 --- a/React/Base/RCTBridge.m +++ b/React/Base/RCTBridge.m @@ -135,17 +135,6 @@ void RCTEnableTurboModuleSharedMutexInit(BOOL enabled) turboModuleSharedMutexInitEnabled = enabled; } -static BOOL turboModuleBlockCopyEnabled = NO; -BOOL RCTTurboModuleBlockCopyEnabled(void) -{ - return turboModuleBlockCopyEnabled; -} - -void RCTEnableTurboModuleBlockCopy(BOOL enabled) -{ - turboModuleBlockCopyEnabled = enabled; -} - static BOOL turboModuleJSCodegenEnabled = NO; BOOL RCTTurboModuleJSCodegenEnabled(void) { diff --git a/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm b/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm index 4a8dc20832c07e..de1bed23f8ea50 100644 --- a/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm +++ b/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm @@ -195,11 +195,7 @@ static int32_t getUniqueId() wrapperWasCalled = YES; }; - if (RCTTurboModuleBlockCopyEnabled()) { - return [callback copy]; - } - - return callback; + return [callback copy]; } namespace facebook { @@ -651,13 +647,8 @@ static int32_t getUniqueId() jsInvoker_, methodNameStr, ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock) { - RCTPromiseResolveBlock resolveCopy = resolveBlock; - RCTPromiseRejectBlock rejectCopy = rejectBlock; - - if (RCTTurboModuleBlockCopyEnabled()) { - resolveCopy = [resolveBlock copy]; - rejectCopy = [rejectBlock copy]; - } + RCTPromiseResolveBlock resolveCopy = [resolveBlock copy]; + RCTPromiseRejectBlock rejectCopy = [rejectBlock copy]; [inv setArgument:(void *)&resolveCopy atIndex:count + 2]; [inv setArgument:(void *)&rejectCopy atIndex:count + 3];