Skip to content

Commit 5275895

Browse files
RSNarafacebook-github-bot
authored andcommitted
Roll out TurboModule block copy
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 (9b76e21). Reviewed By: PeteTheHeat Differential Revision: D25617672 fbshipit-source-id: 24863b31a2d82c8d39a91c8ea8eb3a62724b800a
1 parent 5c4f145 commit 5275895

File tree

3 files changed

+3
-27
lines changed

3 files changed

+3
-27
lines changed

React/Base/RCTBridge.h

-4
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,6 @@ RCT_EXTERN void RCTEnableTurboModuleEagerInit(BOOL enabled);
160160
RCT_EXTERN BOOL RCTTurboModuleSharedMutexInitEnabled(void);
161161
RCT_EXTERN void RCTEnableTurboModuleSharedMutexInit(BOOL enabled);
162162

163-
// Turn on TurboModule block copy
164-
RCT_EXTERN BOOL RCTTurboModuleBlockCopyEnabled(void);
165-
RCT_EXTERN void RCTEnableTurboModuleBlockCopy(BOOL enabled);
166-
167163
// Turn on TurboModule JS Codegen
168164
RCT_EXTERN BOOL RCTTurboModuleJSCodegenEnabled(void);
169165
RCT_EXTERN void RCTEnableTurboModuleJSCodegen(BOOL enabled);

React/Base/RCTBridge.m

-11
Original file line numberDiff line numberDiff line change
@@ -135,17 +135,6 @@ void RCTEnableTurboModuleSharedMutexInit(BOOL enabled)
135135
turboModuleSharedMutexInitEnabled = enabled;
136136
}
137137

138-
static BOOL turboModuleBlockCopyEnabled = NO;
139-
BOOL RCTTurboModuleBlockCopyEnabled(void)
140-
{
141-
return turboModuleBlockCopyEnabled;
142-
}
143-
144-
void RCTEnableTurboModuleBlockCopy(BOOL enabled)
145-
{
146-
turboModuleBlockCopyEnabled = enabled;
147-
}
148-
149138
static BOOL turboModuleJSCodegenEnabled = NO;
150139
BOOL RCTTurboModuleJSCodegenEnabled(void)
151140
{

ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm

+3-12
Original file line numberDiff line numberDiff line change
@@ -195,11 +195,7 @@ static int32_t getUniqueId()
195195
wrapperWasCalled = YES;
196196
};
197197

198-
if (RCTTurboModuleBlockCopyEnabled()) {
199-
return [callback copy];
200-
}
201-
202-
return callback;
198+
return [callback copy];
203199
}
204200

205201
namespace facebook {
@@ -651,13 +647,8 @@ static int32_t getUniqueId()
651647
jsInvoker_,
652648
methodNameStr,
653649
^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock) {
654-
RCTPromiseResolveBlock resolveCopy = resolveBlock;
655-
RCTPromiseRejectBlock rejectCopy = rejectBlock;
656-
657-
if (RCTTurboModuleBlockCopyEnabled()) {
658-
resolveCopy = [resolveBlock copy];
659-
rejectCopy = [rejectBlock copy];
660-
}
650+
RCTPromiseResolveBlock resolveCopy = [resolveBlock copy];
651+
RCTPromiseRejectBlock rejectCopy = [rejectBlock copy];
661652

662653
[inv setArgument:(void *)&resolveCopy atIndex:count + 2];
663654
[inv setArgument:(void *)&rejectCopy atIndex:count + 3];

0 commit comments

Comments
 (0)