From 0ec1d21c2f8454c9bd4d6578ebc7a970a15dea7f Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Mon, 26 Nov 2018 13:48:56 -0800 Subject: [PATCH] Fabric: Fixed a retain cycle between ParagraphShadowNode, LocalData and AttributedString Summary: Consider this: * ParagraphShadowNode retains LocalData, * LocalData contains AttributedString, * AttributedString contains Fragments, * Fragment can contain a pointer to parent shadow node, so it can be the ParagraphShadowNode. In this case it's a retain cycle. We actually don't need to store pointers to not TextShadowNodes, so we don't now. Later, after we fully migrate to ShadowView, we can remove this condition because it will become harmless. Reviewed By: sahrens Differential Revision: D13196885 fbshipit-source-id: d386ce0a067df0a72e6619d62d56038aaf80eccb --- .../components/text/basetext/BaseTextShadowNode.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.cpp b/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.cpp index 7c7036fbc4a..aaa8e83e2da 100644 --- a/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.cpp +++ b/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.cpp @@ -29,7 +29,15 @@ AttributedString BaseTextShadowNode::getAttributedString( auto fragment = AttributedString::Fragment{}; fragment.string = rawTextShadowNode->getProps()->text; fragment.textAttributes = textAttributes; - fragment.parentShadowNode = parentNode; + + // Storing a retaining pointer to `ParagraphShadowNode` inside + // `attributedString` causes a retain cycle (besides that fact that we + // don't need it at all). Storing a `ShadowView` instance instead of + // `ShadowNode` should properly fix this problem. + fragment.parentShadowNode = + std::dynamic_pointer_cast(parentNode) + ? parentNode + : nullptr; attributedString.appendFragment(fragment); continue; }