From 5f503b8427541f3e30b36c7702949c8332264156 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20M=C4=85ka?= Date: Fri, 4 Aug 2023 07:33:37 -0700 Subject: [PATCH] Restore checking shadow tree commit cancellation after commit hook execution (#38715) Summary: Hello! This PR is a fix for one merged some time ago (https://github.com/facebook/react-native/pull/36216). In the PR check for `nullptr` value of `newRootShadowNode` just after performing commit hooks was overlooked. This PR restores previous behaviour of conditional commit cancellation after commit hook execution. ## Changelog: [INTERNAL] [FIXED] - Restore checking shadow tree commit cancellation after commit hook execution Pull Request resolved: https://github.com/facebook/react-native/pull/38715 Test Plan: Just register a commit hook that return `nullptr`. In that case current code crashes due to `nullptr` dereference. Reviewed By: sammy-SC Differential Revision: D47972245 Pulled By: ryancat fbshipit-source-id: 7599ad11ed4b2dcaf25e53f676ec4530e37410d5 --- .../react/renderer/mounting/ShadowTree.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp index 0b9106bbb5d2b0..5278e34fc99dd6 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp @@ -355,6 +355,11 @@ CommitStatus ShadowTree::tryCommit( newRootShadowNode = delegate_.shadowTreeWillCommit( *this, oldRootShadowNode, newRootShadowNode); + if (!newRootShadowNode || + (commitOptions.shouldYield && commitOptions.shouldYield())) { + return CommitStatus::Cancelled; + } + // Layout nodes. std::vector affectedLayoutableNodes{}; affectedLayoutableNodes.reserve(1024); @@ -372,17 +377,16 @@ CommitStatus ShadowTree::tryCommit( // Updating `currentRevision_` in unique manner if it hasn't changed. std::unique_lock lock(commitMutex_); + if (commitOptions.shouldYield && commitOptions.shouldYield()) { + return CommitStatus::Cancelled; + } + if (currentRevision_.number != oldRevision.number) { return CommitStatus::Failed; } auto newRevisionNumber = oldRevision.number + 1; - if (!newRootShadowNode || - (commitOptions.shouldYield && commitOptions.shouldYield())) { - return CommitStatus::Cancelled; - } - { std::lock_guard dispatchLock(EventEmitter::DispatchMutex());