Fix memory leak convertJSIFunctionToCallback

Summary:
changelog: [internal]

### When does leak happen?
Leak happens anytime a callback isn't executed inside native module, it will never get cleaned up.
Imagine a native module with method that takes onSuccess and onFail callbacks. Only one of them will be called at any time and the other one will leak.

### Why does it leak?
It leaks because when `CallbackWrapper` is created using `CallbackWrapper::createWeak`. Inside `CallbackWrapper::createWeak`, the newly created object is inserted into `LongLivedObjectCollection`. This object collection will keep it alive until `CallbackWrapper::destroy` is called, which isn't called in case closure isn't executed.

### Solution
Introduce class RCTBlockGuard which ties cleanup of resources to lifetime of the block.

Reviewed By: RSNara

Differential Revision: D26664173

fbshipit-source-id: 9348f7c39eb317cf1e8e5d59e77a378e5e04f3eb
This commit is contained in:
Samuel Susla
2021-02-27 04:55:52 -08:00
committed by Facebook GitHub Bot
parent 9f120efcf4
commit f7d006e60c
5 changed files with 82 additions and 1 deletions
+4
View File
@@ -160,6 +160,10 @@ RCT_EXTERN void RCTEnableTurboModuleEagerInit(BOOL enabled);
RCT_EXTERN BOOL RCTTurboModuleSharedMutexInitEnabled(void);
RCT_EXTERN void RCTEnableTurboModuleSharedMutexInit(BOOL enabled);
// Turn on TurboModule shared mutex initialization
RCT_EXTERN BOOL RCTTurboModuleBlockGuardEnabled(void);
RCT_EXTERN void RCTEnableTurboModuleBlockGuard(BOOL enabled);
/**
* Async batched bridge used to communicate with the JavaScript application.
*/
+11
View File
@@ -135,6 +135,17 @@ void RCTEnableTurboModuleSharedMutexInit(BOOL enabled)
turboModuleSharedMutexInitEnabled = enabled;
}
static BOOL turboModuleBlockGuardEnabled = NO;
BOOL RCTTurboModuleBlockGuardEnabled(void)
{
return turboModuleBlockGuardEnabled;
}
void RCTEnableTurboModuleBlockGuard(BOOL enabled)
{
turboModuleBlockGuardEnabled = enabled;
}
@interface RCTBridge () <RCTReloadListener>
@end
@@ -0,0 +1,24 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
#import <Foundation/Foundation.h>
NS_ASSUME_NONNULL_BEGIN
/**
* RCTBlockGuard is designed to be used with obj-c blocks to assist with manual deallocation of C++ resources
* tied to lifetime of a block. If C++ resources needs to be manually released at the end of block or when the block
* is deallocated, place the clean up code inside constructor and make sure the instace of the class is references in
* the block.
*/
@interface RCTBlockGuard : NSObject
- (instancetype)initWithCleanup:(void (^)(void))cleanup;
@end
NS_ASSUME_NONNULL_END
@@ -0,0 +1,28 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
#import "RCTBlockGuard.h"
@implementation RCTBlockGuard {
void (^_cleanup)(void);
}
- (instancetype)initWithCleanup:(void (^)(void))cleanup
{
if (self = [super init]) {
_cleanup = cleanup;
}
return self;
}
- (void)dealloc
{
_cleanup();
}
@end
@@ -6,6 +6,7 @@
*/
#import "RCTTurboModule.h"
#import "RCTBlockGuard.h"
#import <objc/message.h>
#import <objc/runtime.h>
@@ -170,6 +171,16 @@ static RCTResponseSenderBlock
convertJSIFunctionToCallback(jsi::Runtime &runtime, const jsi::Function &value, std::shared_ptr<CallInvoker> jsInvoker)
{
auto weakWrapper = CallbackWrapper::createWeak(value.getFunction(runtime), runtime, jsInvoker);
RCTBlockGuard *blockGuard;
if (RCTTurboModuleBlockGuardEnabled()) {
blockGuard = [[RCTBlockGuard alloc] initWithCleanup:^() {
auto strongWrapper = weakWrapper.lock();
if (strongWrapper) {
strongWrapper->destroy();
}
}];
}
BOOL __block wrapperWasCalled = NO;
RCTResponseSenderBlock callback = ^(NSArray *responses) {
if (wrapperWasCalled) {
@@ -181,7 +192,7 @@ convertJSIFunctionToCallback(jsi::Runtime &runtime, const jsi::Function &value,
return;
}
strongWrapper->jsInvoker().invokeAsync([weakWrapper, responses]() {
strongWrapper->jsInvoker().invokeAsync([weakWrapper, responses, blockGuard]() {
auto strongWrapper2 = weakWrapper.lock();
if (!strongWrapper2) {
return;
@@ -190,6 +201,9 @@ convertJSIFunctionToCallback(jsi::Runtime &runtime, const jsi::Function &value,
std::vector<jsi::Value> args = convertNSArrayToStdVector(strongWrapper2->runtime(), responses);
strongWrapper2->callback().call(strongWrapper2->runtime(), (const jsi::Value *)args.data(), args.size());
strongWrapper2->destroy();
// Delete the CallbackWrapper when the block gets dealloced without being invoked.
(void)blockGuard;
});
wrapperWasCalled = YES;