From 57dd48b2464ac04b860f2f69cb4f131990fe4dbd Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Fri, 18 Sep 2020 17:05:10 -0700 Subject: [PATCH] Fabric: Marking all JS function lambdas `noexcept` in UIManagerBinding Summary: Exceptions in C++ work quite differently from exceptions in other languages. To make exceptions actually work **correctly** all the code needs to be written with "exceptions in mind" (e.g., see https://www.stroustrup.com/except.pdf). In short, if the code is not "exceptions ready", throwing an exception causes memory leaks, dangling pointers, and invariant violations all over the place, which will probably cause another crashes down the road (which will be especially hard to investigate and attribute to the original issue). Fabric Core (Layout, Props parsing, ShadowNodes management, and so on) does not use exceptions because in most (all?) the cases the exception is now recoverable. So, if a program detects some internal state invariant violation or missing some resource, *logically* it's fatal. We also don't want to pay code-size and performance tax for exception support, so that's why we don't use them. It's just not the right fit for Fabric Core. This does not mean that exceptions don't happen though. C++ standard library can throw them... sometimes. And if our library is compiled with exceptions enabled (still the case, unfortunately), an exception can bubble to JavaScript code and losing all context down the road. And it's hard to investigate such crashes. To isolate those occasional exceptions inside C++ core we are marking all C++/JS boundaries with `noexcept` that stops the bubbling. I hope that will give us much more informative crash reports. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D23787492 fbshipit-source-id: 0822dbf36fc680c15b02b5cd0f2d87328296b642 --- .../RCTSurfacePresenterBridgeAdapter.mm | 2 +- .../react/renderer/core/EventTarget.cpp | 2 +- ReactCommon/react/renderer/core/EventTarget.h | 2 +- .../react/renderer/uimanager/UIManager.cpp | 4 +- .../react/renderer/uimanager/UIManager.h | 4 +- .../uimanager/UIManagerAnimationDelegate.h | 4 +- .../renderer/uimanager/UIManagerBinding.cpp | 138 +++++++++--------- .../renderer/uimanager/UIManagerBinding.h | 14 +- .../react/renderer/uimanager/primitives.h | 16 +- 9 files changed, 93 insertions(+), 93 deletions(-) diff --git a/React/Fabric/RCTSurfacePresenterBridgeAdapter.mm b/React/Fabric/RCTSurfacePresenterBridgeAdapter.mm index a70c494d937..8c20e934870 100644 --- a/React/Fabric/RCTSurfacePresenterBridgeAdapter.mm +++ b/React/Fabric/RCTSurfacePresenterBridgeAdapter.mm @@ -48,7 +48,7 @@ static RuntimeExecutor RCTRuntimeExecutorFromBridge(RCTBridge *bridge) auto bridgeWeakWrapper = wrapManagedObjectWeakly([bridge batchedBridge] ?: bridge); RuntimeExecutor runtimeExecutor = [bridgeWeakWrapper]( - std::function &&callback) { + std::function &&callback) { RCTBridge *bridge = unwrapManagedObjectWeakly(bridgeWeakWrapper); RCTAssert(bridge, @"RCTRuntimeExecutorFromBridge: Bridge must not be nil at the moment of scheduling a call."); diff --git a/ReactCommon/react/renderer/core/EventTarget.cpp b/ReactCommon/react/renderer/core/EventTarget.cpp index c9f418c2d88..01a5a77bd50 100644 --- a/ReactCommon/react/renderer/core/EventTarget.cpp +++ b/ReactCommon/react/renderer/core/EventTarget.cpp @@ -14,7 +14,7 @@ using Tag = EventTarget::Tag; EventTarget::EventTarget( jsi::Runtime &runtime, - const jsi::Value &instanceHandle, + jsi::Value const &instanceHandle, Tag tag) : weakInstanceHandle_( jsi::WeakObject(runtime, instanceHandle.asObject(runtime))), diff --git a/ReactCommon/react/renderer/core/EventTarget.h b/ReactCommon/react/renderer/core/EventTarget.h index 8bd7a03a199..92f382a96c4 100644 --- a/ReactCommon/react/renderer/core/EventTarget.h +++ b/ReactCommon/react/renderer/core/EventTarget.h @@ -35,7 +35,7 @@ class EventTarget { /* * Constructs an EventTarget from a weak instance handler and a tag. */ - EventTarget(jsi::Runtime &runtime, const jsi::Value &instanceHandle, Tag tag); + EventTarget(jsi::Runtime &runtime, jsi::Value const &instanceHandle, Tag tag); /* * Sets the `enabled` flag that allows creating a strong instance handle from diff --git a/ReactCommon/react/renderer/uimanager/UIManager.cpp b/ReactCommon/react/renderer/uimanager/UIManager.cpp index e4c4cdb8f5b..a8b4c696480 100644 --- a/ReactCommon/react/renderer/uimanager/UIManager.cpp +++ b/ReactCommon/react/renderer/uimanager/UIManager.cpp @@ -311,8 +311,8 @@ void UIManager::dispatchCommand( void UIManager::configureNextLayoutAnimation( jsi::Runtime &runtime, RawValue const &config, - const jsi::Value &successCallback, - const jsi::Value &failureCallback) const { + jsi::Value const &successCallback, + jsi::Value const &failureCallback) const { if (animationDelegate_) { animationDelegate_->uiManagerDidConfigureNextLayoutAnimation( runtime, diff --git a/ReactCommon/react/renderer/uimanager/UIManager.h b/ReactCommon/react/renderer/uimanager/UIManager.h index 4e53389615d..1235cf3532a 100644 --- a/ReactCommon/react/renderer/uimanager/UIManager.h +++ b/ReactCommon/react/renderer/uimanager/UIManager.h @@ -147,8 +147,8 @@ class UIManager final : public ShadowTreeDelegate { void configureNextLayoutAnimation( jsi::Runtime &runtime, RawValue const &config, - const jsi::Value &successCallback, - const jsi::Value &failureCallback) const; + jsi::Value const &successCallback, + jsi::Value const &failureCallback) const; ShadowTreeRegistry const &getShadowTreeRegistry() const; diff --git a/ReactCommon/react/renderer/uimanager/UIManagerAnimationDelegate.h b/ReactCommon/react/renderer/uimanager/UIManagerAnimationDelegate.h index cfbb0c00e55..9e55d1f3862 100644 --- a/ReactCommon/react/renderer/uimanager/UIManagerAnimationDelegate.h +++ b/ReactCommon/react/renderer/uimanager/UIManagerAnimationDelegate.h @@ -26,8 +26,8 @@ class UIManagerAnimationDelegate { virtual void uiManagerDidConfigureNextLayoutAnimation( jsi::Runtime &runtime, RawValue const &config, - const jsi::Value &successCallback, - const jsi::Value &failureCallback) const = 0; + jsi::Value const &successCallback, + jsi::Value const &failureCallback) const = 0; /** * Set ComponentDescriptor registry. diff --git a/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp b/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp index 0633fbcc6e6..024eb3424dc 100644 --- a/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp +++ b/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp @@ -17,7 +17,7 @@ namespace react { static jsi::Object getModule( jsi::Runtime &runtime, - const std::string &moduleName) { + std::string const &moduleName) { auto batchedBridge = runtime.global().getPropertyAsObject(runtime, "__fbBatchedBridge"); auto getCallableModule = @@ -82,8 +82,8 @@ void UIManagerBinding::attach(std::shared_ptr const &uiManager) { void UIManagerBinding::startSurface( jsi::Runtime &runtime, SurfaceId surfaceId, - const std::string &moduleName, - const folly::dynamic &initalProps) const { + std::string const &moduleName, + folly::dynamic const &initalProps) const { folly::dynamic parameters = folly::dynamic::object(); parameters["rootTag"] = surfaceId; parameters["initialProps"] = initalProps; @@ -128,9 +128,9 @@ void UIManagerBinding::stopSurface(jsi::Runtime &runtime, SurfaceId surfaceId) void UIManagerBinding::dispatchEvent( jsi::Runtime &runtime, - const EventTarget *eventTarget, - const std::string &type, - const ValueFactory &payloadFactory) const { + EventTarget const *eventTarget, + std::string const &type, + ValueFactory const &payloadFactory) const { SystraceSection s("UIManagerBinding::dispatchEvent"); auto payload = payloadFactory(runtime); @@ -158,7 +158,7 @@ void UIManagerBinding::dispatchEvent( : jsi::Value::null(); auto &eventHandlerWrapper = - static_cast(*eventHandler_); + static_cast(*eventHandler_); eventHandlerWrapper.callback.call( runtime, @@ -173,7 +173,7 @@ void UIManagerBinding::invalidate() const { jsi::Value UIManagerBinding::get( jsi::Runtime &runtime, - const jsi::PropNameID &name) { + jsi::PropNameID const &name) { auto methodName = name.utf8(runtime); // Convert shared_ptr to a raw ptr @@ -213,9 +213,9 @@ jsi::Value UIManagerBinding::get( 5, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { return valueFromShadowNode( runtime, uiManager->createNode( @@ -235,9 +235,9 @@ jsi::Value UIManagerBinding::get( 1, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { return valueFromShadowNode( runtime, uiManager->cloneNode(shadowNodeFromValue(runtime, arguments[0]))); @@ -251,9 +251,9 @@ jsi::Value UIManagerBinding::get( 2, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { uiManager->setJSResponder( shadowNodeFromValue(runtime, arguments[0]), arguments[1].getBool()); @@ -269,9 +269,9 @@ jsi::Value UIManagerBinding::get( 2, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { auto node = shadowNodeFromValue(runtime, arguments[0]); auto locationX = (Float)arguments[1].getNumber(); auto locationY = (Float)arguments[2].getNumber(); @@ -299,9 +299,9 @@ jsi::Value UIManagerBinding::get( 0, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { uiManager->clearJSResponder(); return jsi::Value::undefined(); @@ -316,9 +316,9 @@ jsi::Value UIManagerBinding::get( 1, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { return valueFromShadowNode( runtime, uiManager->cloneNode( @@ -335,10 +335,10 @@ jsi::Value UIManagerBinding::get( 2, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { - const auto &rawProps = RawProps(runtime, arguments[1]); + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { + auto const &rawProps = RawProps(runtime, arguments[1]); return valueFromShadowNode( runtime, uiManager->cloneNode( @@ -356,10 +356,10 @@ jsi::Value UIManagerBinding::get( 2, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { - const auto &rawProps = RawProps(runtime, arguments[1]); + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { + auto const &rawProps = RawProps(runtime, arguments[1]); return valueFromShadowNode( runtime, uiManager->cloneNode( @@ -376,9 +376,9 @@ jsi::Value UIManagerBinding::get( 2, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { uiManager->appendChild( shadowNodeFromValue(runtime, arguments[0]), shadowNodeFromValue(runtime, arguments[1])); @@ -392,9 +392,9 @@ jsi::Value UIManagerBinding::get( name, 1, [](jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { auto shadowNodeList = std::make_shared(SharedShadowNodeList({})); return valueFromShadowNodeList(runtime, shadowNodeList); @@ -407,9 +407,9 @@ jsi::Value UIManagerBinding::get( name, 2, [](jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { auto shadowNodeList = shadowNodeListFromValue(runtime, arguments[0]); auto shadowNode = shadowNodeFromValue(runtime, arguments[1]); shadowNodeList->push_back(shadowNode); @@ -429,7 +429,7 @@ jsi::Value UIManagerBinding::get( jsi::Runtime &runtime, jsi::Value const &thisValue, jsi::Value const *arguments, - size_t count) -> jsi::Value { + size_t count) noexcept -> jsi::Value { auto surfaceId = surfaceIdFromValue(runtime, arguments[0]); auto shadowNodeList = shadowNodeListFromValue(runtime, arguments[1]); @@ -456,7 +456,7 @@ jsi::Value UIManagerBinding::get( jsi::Runtime &runtime, jsi::Value const &thisValue, jsi::Value const *arguments, - size_t count) -> jsi::Value { + size_t count) noexcept -> jsi::Value { uiManager->completeSurface( surfaceIdFromValue(runtime, arguments[0]), shadowNodeListFromValue(runtime, arguments[1])); @@ -473,9 +473,9 @@ jsi::Value UIManagerBinding::get( 1, [this]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { auto eventHandler = arguments[0].getObject(runtime).getFunction(runtime); eventHandler_ = @@ -491,9 +491,9 @@ jsi::Value UIManagerBinding::get( 2, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { auto layoutMetrics = uiManager->getRelativeLayoutMetrics( *shadowNodeFromValue(runtime, arguments[0]), shadowNodeFromValue(runtime, arguments[1]).get(), @@ -515,9 +515,9 @@ jsi::Value UIManagerBinding::get( 3, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { uiManager->dispatchCommand( shadowNodeFromValue(runtime, arguments[0]), stringFromValue(runtime, arguments[1]), @@ -535,9 +535,9 @@ jsi::Value UIManagerBinding::get( 4, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { auto layoutMetrics = uiManager->getRelativeLayoutMetrics( *shadowNodeFromValue(runtime, arguments[0]), shadowNodeFromValue(runtime, arguments[1]).get(), @@ -571,9 +571,9 @@ jsi::Value UIManagerBinding::get( 2, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { auto layoutMetrics = uiManager->getRelativeLayoutMetrics( *shadowNodeFromValue(runtime, arguments[0]), nullptr, @@ -606,9 +606,9 @@ jsi::Value UIManagerBinding::get( 2, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { auto layoutMetrics = uiManager->getRelativeLayoutMetrics( *shadowNodeFromValue(runtime, arguments[0]), nullptr, @@ -641,9 +641,9 @@ jsi::Value UIManagerBinding::get( 2, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { uiManager->setNativeProps( *shadowNodeFromValue(runtime, arguments[0]), RawProps(runtime, arguments[1])); @@ -659,9 +659,9 @@ jsi::Value UIManagerBinding::get( 3, [uiManager]( jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { uiManager->configureNextLayoutAnimation( runtime, // TODO: pass in JSI value instead of folly::dynamic to RawValue diff --git a/ReactCommon/react/renderer/uimanager/UIManagerBinding.h b/ReactCommon/react/renderer/uimanager/UIManagerBinding.h index d4e133f59d3..d5616310716 100644 --- a/ReactCommon/react/renderer/uimanager/UIManagerBinding.h +++ b/ReactCommon/react/renderer/uimanager/UIManagerBinding.h @@ -47,8 +47,8 @@ class UIManagerBinding : public jsi::HostObject { void startSurface( jsi::Runtime &runtime, SurfaceId surfaceId, - const std::string &moduleName, - const folly::dynamic &initalProps) const; + std::string const &moduleName, + folly::dynamic const &initalProps) const; /* * Stops React Native Surface with given id. @@ -62,9 +62,9 @@ class UIManagerBinding : public jsi::HostObject { */ void dispatchEvent( jsi::Runtime &runtime, - const EventTarget *eventTarget, - const std::string &type, - const ValueFactory &payloadFactory) const; + EventTarget const *eventTarget, + std::string const &type, + ValueFactory const &payloadFactory) const; /* * Invalidates the binding and underlying UIManager. @@ -78,11 +78,11 @@ class UIManagerBinding : public jsi::HostObject { /* * `jsi::HostObject` specific overloads. */ - jsi::Value get(jsi::Runtime &runtime, const jsi::PropNameID &name) override; + jsi::Value get(jsi::Runtime &runtime, jsi::PropNameID const &name) override; private: std::shared_ptr uiManager_; - std::unique_ptr eventHandler_; + std::unique_ptr eventHandler_; }; } // namespace react diff --git a/ReactCommon/react/renderer/uimanager/primitives.h b/ReactCommon/react/renderer/uimanager/primitives.h index cd9dde4a662..9fb2ea92786 100644 --- a/ReactCommon/react/renderer/uimanager/primitives.h +++ b/ReactCommon/react/renderer/uimanager/primitives.h @@ -42,7 +42,7 @@ struct ShadowNodeListWrapper : public jsi::HostObject { inline static ShadowNode::Shared shadowNodeFromValue( jsi::Runtime &runtime, - const jsi::Value &value) { + jsi::Value const &value) { return value.getObject(runtime) .getHostObject(runtime) ->shadowNode; @@ -57,7 +57,7 @@ inline static jsi::Value valueFromShadowNode( inline static SharedShadowNodeUnsharedList shadowNodeListFromValue( jsi::Runtime &runtime, - const jsi::Value &value) { + jsi::Value const &value) { return value.getObject(runtime) .getHostObject(runtime) ->shadowNodeList; @@ -72,31 +72,31 @@ inline static jsi::Value valueFromShadowNodeList( inline static SharedEventTarget eventTargetFromValue( jsi::Runtime &runtime, - const jsi::Value &eventTargetValue, - const jsi::Value &tagValue) { + jsi::Value const &eventTargetValue, + jsi::Value const &tagValue) { return std::make_shared( runtime, eventTargetValue, tagValue.getNumber()); } -inline static Tag tagFromValue(jsi::Runtime &runtime, const jsi::Value &value) { +inline static Tag tagFromValue(jsi::Runtime &runtime, jsi::Value const &value) { return (Tag)value.getNumber(); } inline static SurfaceId surfaceIdFromValue( jsi::Runtime &runtime, - const jsi::Value &value) { + jsi::Value const &value) { return (SurfaceId)value.getNumber(); } inline static std::string stringFromValue( jsi::Runtime &runtime, - const jsi::Value &value) { + jsi::Value const &value) { return value.getString(runtime).utf8(runtime); } inline static folly::dynamic commandArgsFromValue( jsi::Runtime &runtime, - const jsi::Value &value) { + jsi::Value const &value) { return jsi::dynamicFromValue(runtime, value); }