Skip to content

Commit

Permalink
Roll out TurboModule block copy
Browse files Browse the repository at this point in the history
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
  • Loading branch information
RSNara authored and facebook-github-bot committed Dec 18, 2020
1 parent 5c4f145 commit 5275895
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 27 deletions.
4 changes: 0 additions & 4 deletions React/Base/RCTBridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 0 additions & 11 deletions React/Base/RCTBridge.m
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
15 changes: 3 additions & 12 deletions ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,7 @@ static int32_t getUniqueId()
wrapperWasCalled = YES;
};

if (RCTTurboModuleBlockCopyEnabled()) {
return [callback copy];
}

return callback;
return [callback copy];
}

namespace facebook {
Expand Down Expand Up @@ -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];
Expand Down

0 comments on commit 5275895

Please sign in to comment.