From a1b6f7bd9ecf8f8f3ec809c5d86206e30d4b0f47 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Mon, 3 Aug 2020 10:42:36 -0700 Subject: [PATCH] Fabric: Asserting if the RuntimeExecutor callback in RCTRuntimeExecutorFromBridge is not called Summary: The implementation of RuntimeExecutor must execute all provided callbacks. However, the implementation of RCTRuntimeExecutorFromBridge cannot guarantee it because it relies on Bridge to make the call. In this diff, we wrap the callback into a callable that asserts if it's not being called before destruction. Changelog: [Internal] Fabric-specific internal change. Reviewed By: mdvacca Differential Revision: D22810439 fbshipit-source-id: f11932019ab6ccbab7e65db5919e0c64dcaf37ed --- .../RCTSurfacePresenterBridgeAdapter.mm | 16 +++- .../utils/CalledOnceMovableOnlyFunction.h | 87 +++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 ReactCommon/utils/CalledOnceMovableOnlyFunction.h diff --git a/React/Fabric/RCTSurfacePresenterBridgeAdapter.mm b/React/Fabric/RCTSurfacePresenterBridgeAdapter.mm index a70c494d937..7f359d4b4b9 100644 --- a/React/Fabric/RCTSurfacePresenterBridgeAdapter.mm +++ b/React/Fabric/RCTSurfacePresenterBridgeAdapter.mm @@ -18,6 +18,7 @@ #import #import +#import #import #import @@ -48,7 +49,20 @@ static RuntimeExecutor RCTRuntimeExecutorFromBridge(RCTBridge *bridge) auto bridgeWeakWrapper = wrapManagedObjectWeakly([bridge batchedBridge] ?: bridge); RuntimeExecutor runtimeExecutor = [bridgeWeakWrapper]( - std::function &&callback) { + std::function &&callbackArgument) { + +#ifndef NDEBUG + // Here we wrap callback into a callable that will assert if the `callback` is not called before deallocation + // or called more than once. + // It's useful to see at which exact point in time those assumptions were violated. + auto sharedCallback = std::make_shared>( + [callback = std::move(callbackArgument)](facebook::jsi::Runtime &runtime) { callback(runtime); }); + auto callback = std::function( + [sharedCallback](facebook::jsi::Runtime &runtime) { (*sharedCallback)(runtime); }); +#else + auto callback = std::move(callbackArgument); +#endif + RCTBridge *bridge = unwrapManagedObjectWeakly(bridgeWeakWrapper); RCTAssert(bridge, @"RCTRuntimeExecutorFromBridge: Bridge must not be nil at the moment of scheduling a call."); diff --git a/ReactCommon/utils/CalledOnceMovableOnlyFunction.h b/ReactCommon/utils/CalledOnceMovableOnlyFunction.h new file mode 100644 index 00000000000..b6d31707d13 --- /dev/null +++ b/ReactCommon/utils/CalledOnceMovableOnlyFunction.h @@ -0,0 +1,87 @@ +/* + * 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. + */ + +#include + +namespace facebook { +namespace react { + +/* + * Implements a moveable-only function that asserts if called more than once + * or destroyed before calling. + * Useful for use in debug mode to ensure such guarantees. + */ +template +class CalledOnceMovableOnlyFunction { + using T = ReturnT(ArgumentT...); + + std::function function_; + bool wasCalled_; + bool wasMovedFrom_; + + public: + CalledOnceMovableOnlyFunction(std::function &&function) + : function_(std::move(function)) { + wasCalled_ = false; + wasMovedFrom_ = false; + } + + ~CalledOnceMovableOnlyFunction() { + assert( + (wasCalled_ || wasMovedFrom_) && + "`CalledOnceMovableOnlyFunction` is destroyed before being called."); + } + + /* + * Not copyable. + */ + CalledOnceMovableOnlyFunction(CalledOnceMovableOnlyFunction const &other) = + delete; + CalledOnceMovableOnlyFunction &operator=( + CalledOnceMovableOnlyFunction const &other) = delete; + + /* + * Movable. + */ + CalledOnceMovableOnlyFunction( + CalledOnceMovableOnlyFunction &&other) noexcept { + wasCalled_ = false; + wasMovedFrom_ = false; + other.wasMovedFrom_ = true; + function_ = std::move(other.function_); + }; + + CalledOnceMovableOnlyFunction &operator=( + CalledOnceMovableOnlyFunction &&other) noexcept { + assert( + (wasCalled_ || wasMovedFrom_) && + "`CalledOnceMovableOnlyFunction` is re-assigned before being called."); + wasCalled_ = false; + wasMovedFrom_ = false; + other.wasMovedFrom_ = true; + function_ = std::move(other.function_); + return *this; + } + + /* + * Callable. + */ + ReturnT operator()(ArgumentT... args) { + assert( + !wasMovedFrom_ && + "`CalledOnceMovableOnlyFunction` is called after being moved from."); + assert( + !wasCalled_ && + "`CalledOnceMovableOnlyFunction` is called more than once."); + + wasCalled_ = true; + return function_(args...); + } +}; + +} // namespace react +} // namespace facebook