Skip to content

Commit

Permalink
Restore checking shadow tree commit cancellation after commit hook ex…
Browse files Browse the repository at this point in the history
…ecution (#38715)

Summary:
Hello! This PR is a fix for one merged some time ago (#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: #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
  • Loading branch information
michalmaka authored and Luna Wei committed Aug 10, 2023
1 parent 4f8c87c commit 5f503b8
Showing 1 changed file with 9 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<LayoutableShadowNode const *> affectedLayoutableNodes{};
affectedLayoutableNodes.reserve(1024);
Expand All @@ -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<std::mutex> dispatchLock(EventEmitter::DispatchMutex());

Expand Down

0 comments on commit 5f503b8

Please sign in to comment.