Skip to content

Commit

Permalink
Copy all blocks generated by TurboModules
Browse files Browse the repository at this point in the history
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 (71da291), D5589246 (2a6965d)

Changelog: [Internal]

Reviewed By: fkgozali

Differential Revision: D23764329

fbshipit-source-id: e71360977bdff74dc570bd40f0139792860f811f
  • Loading branch information
RSNara authored and facebook-github-bot committed Sep 17, 2020
1 parent ee38751 commit 9b76e21
Showing 1 changed file with 12 additions and 8 deletions.
20 changes: 12 additions & 8 deletions ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.mm
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ static int32_t getUniqueId()
{
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");
}
Expand All @@ -194,6 +194,8 @@ static int32_t getUniqueId()

wrapperWasCalled = YES;
};

return [callback copy];
}

namespace facebook {
Expand Down Expand Up @@ -331,12 +333,11 @@ static int32_t getUniqueId()
bool wasMethodSync = isMethodSync(returnType);

void (^block)() = ^{
if (!weakModule) {
id<RCTTurboModule> strongModule = weakModule;
if (!strongModule) {
return;
}

id<RCTTurboModule> strongModule = weakModule;

if (wasMethodSync) {
TurboModulePerfLogger::syncMethodCallExecutionStart(moduleName, methodNameStr.c_str());
} else {
Expand Down Expand Up @@ -635,10 +636,13 @@ static int32_t getUniqueId()
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);
})
Expand Down

0 comments on commit 9b76e21

Please sign in to comment.