From 90618d22a07be3cc4f49eb6aef57ef69331098e5 Mon Sep 17 00:00:00 2001 From: Krzysztof Magiera Date: Fri, 25 Mar 2022 14:30:25 +0100 Subject: [PATCH 1/2] Fix dynamic_cast (RTTI) by adding key function to ShadowNodeWrapper and related classes --- ReactCommon/react/renderer/uimanager/UIManager.cpp | 14 ++++++++++++++ ReactCommon/react/renderer/uimanager/primitives.h | 8 ++++++++ 2 files changed, 22 insertions(+) diff --git a/ReactCommon/react/renderer/uimanager/UIManager.cpp b/ReactCommon/react/renderer/uimanager/UIManager.cpp index 0d2857b49415..c0742434a44f 100644 --- a/ReactCommon/react/renderer/uimanager/UIManager.cpp +++ b/ReactCommon/react/renderer/uimanager/UIManager.cpp @@ -22,6 +22,20 @@ namespace facebook::react { +ShadowNodeWrapper::~ShadowNodeWrapper() { + // implementation is kept empty as there is nothing necessary to do in destrutor + // however, it still needs to exist in order to act as a "key function" for + // the SHadowNodeWrapper class -- this allow for RTTI to work properly across + // the library boundaries (i.e. dynamic_cast that is used by isHostObject method) +} + +ShadowNodeListWrapper::~ShadowNodeListWrapper() { + // implementation is kept empty as there is nothing necessary to do in destrutor + // however, it still needs to exist in order to act as a "key function" for + // the SHadowNodeListWrapper class -- this allow for RTTI to work properly across + // the library boundaries (i.e. dynamic_cast that is used by isHostObject method) +} + static std::unique_ptr constructLeakCheckerIfNeeded( RuntimeExecutor const &runtimeExecutor) { #ifdef REACT_NATIVE_DEBUG diff --git a/ReactCommon/react/renderer/uimanager/primitives.h b/ReactCommon/react/renderer/uimanager/primitives.h index 101155f91afe..ea548586f50a 100644 --- a/ReactCommon/react/renderer/uimanager/primitives.h +++ b/ReactCommon/react/renderer/uimanager/primitives.h @@ -30,6 +30,10 @@ struct ShadowNodeWrapper : public jsi::HostObject { ShadowNodeWrapper(SharedShadowNode shadowNode) : shadowNode(std::move(shadowNode)) {} + // The below method needs to be out-of-line in order for the class to have + // at least one "key function" (see https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable) + virtual ~ShadowNodeWrapper(); + ShadowNode::Shared shadowNode; }; @@ -37,6 +41,10 @@ struct ShadowNodeListWrapper : public jsi::HostObject { ShadowNodeListWrapper(SharedShadowNodeUnsharedList shadowNodeList) : shadowNodeList(shadowNodeList) {} + // The below method needs to be out-of-line in order for the class to have + // at least one "key function" (see https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable) + virtual ~ShadowNodeListWrapper(); + SharedShadowNodeUnsharedList shadowNodeList; }; From e5f833015957eb13bce40d26dc25be0b90d0548e Mon Sep 17 00:00:00 2001 From: Krzysztof Magiera Date: Fri, 25 Mar 2022 16:56:03 +0100 Subject: [PATCH 2/2] Add override for destructors to address review comments --- ReactCommon/react/renderer/uimanager/UIManager.cpp | 4 ++-- ReactCommon/react/renderer/uimanager/primitives.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ReactCommon/react/renderer/uimanager/UIManager.cpp b/ReactCommon/react/renderer/uimanager/UIManager.cpp index c0742434a44f..0416836946a7 100644 --- a/ReactCommon/react/renderer/uimanager/UIManager.cpp +++ b/ReactCommon/react/renderer/uimanager/UIManager.cpp @@ -25,14 +25,14 @@ namespace facebook::react { ShadowNodeWrapper::~ShadowNodeWrapper() { // implementation is kept empty as there is nothing necessary to do in destrutor // however, it still needs to exist in order to act as a "key function" for - // the SHadowNodeWrapper class -- this allow for RTTI to work properly across + // the ShadowNodeWrapper class -- this allow for RTTI to work properly across // the library boundaries (i.e. dynamic_cast that is used by isHostObject method) } ShadowNodeListWrapper::~ShadowNodeListWrapper() { // implementation is kept empty as there is nothing necessary to do in destrutor // however, it still needs to exist in order to act as a "key function" for - // the SHadowNodeListWrapper class -- this allow for RTTI to work properly across + // the ShadowNodeListWrapper class -- this allow for RTTI to work properly across // the library boundaries (i.e. dynamic_cast that is used by isHostObject method) } diff --git a/ReactCommon/react/renderer/uimanager/primitives.h b/ReactCommon/react/renderer/uimanager/primitives.h index ea548586f50a..42128932e856 100644 --- a/ReactCommon/react/renderer/uimanager/primitives.h +++ b/ReactCommon/react/renderer/uimanager/primitives.h @@ -32,7 +32,7 @@ struct ShadowNodeWrapper : public jsi::HostObject { // The below method needs to be out-of-line in order for the class to have // at least one "key function" (see https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable) - virtual ~ShadowNodeWrapper(); + ~ShadowNodeWrapper() override; ShadowNode::Shared shadowNode; }; @@ -43,7 +43,7 @@ struct ShadowNodeListWrapper : public jsi::HostObject { // The below method needs to be out-of-line in order for the class to have // at least one "key function" (see https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable) - virtual ~ShadowNodeListWrapper(); + ~ShadowNodeListWrapper() override; SharedShadowNodeUnsharedList shadowNodeList; };