From 8533d28f4504f5738159a7066740ce9859eef42b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20Norte?= <rubennorte@meta.com>
Date: Thu, 17 Jul 2025 12:18:52 -0700
Subject: [PATCH] Fix incorrect locking and attempts check in ShadowTree
 experiment (#52681)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/52681

Changelog: [internal]

In the original change I made in D78418504 / https://github.com/facebook/react-native/pull/52645 I made 2 mistakes:
1. Used a lock that would try to re-lock on itself without it being recursive (which would cause a deadlock). I didn't see that because when testing I didn't hit the case where we'd exhaust the options.
2. The `attemps` variable wasn't incremented, so we never left the loop in case of exhaustion.

This propagates a flag to `tryCommit` to indicate we've already locked on the commitMutex_ so we don't need to lock again in that case and increases the counter, fixing the issue.

Reviewed By: cortinico

Differential Revision: D78497509

fbshipit-source-id: 546ccd0c84aed5416ce1aef47d79419b4fe06f66
---
 .../react/renderer/mounting/ShadowTree.cpp       | 16 ++++++++++++----
 .../react/renderer/mounting/ShadowTree.h         |  3 ++-
 2 files changed, 14 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 079b55c171f..c803f80e027 100644
--- a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp
+++ b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp
@@ -251,11 +251,12 @@ CommitStatus ShadowTree::commit(
       if (status != CommitStatus::Failed) {
         return status;
       }
+      attempts++;
     }
 
     {
       std::unique_lock lock(commitMutex_);
-      return tryCommit(transaction, commitOptions);
+      return tryCommit(transaction, commitOptions, true);
     }
   } else {
     while (true) {
@@ -275,7 +276,8 @@ CommitStatus ShadowTree::commit(
 
 CommitStatus ShadowTree::tryCommit(
     const ShadowTreeCommitTransaction& transaction,
-    const CommitOptions& commitOptions) const {
+    const CommitOptions& commitOptions,
+    bool hasLocked) const {
   TraceSection s("ShadowTree::commit");
 
   auto telemetry = TransactionTelemetry{};
@@ -287,7 +289,10 @@ CommitStatus ShadowTree::tryCommit(
 
   {
     // Reading `currentRevision_` in shared manner.
-    std::shared_lock lock(commitMutex_);
+    std::shared_lock lock(commitMutex_, std::defer_lock);
+    if (!hasLocked) {
+      lock.lock();
+    }
     commitMode = commitMode_;
     oldRevision = currentRevision_;
   }
@@ -328,7 +333,10 @@ CommitStatus ShadowTree::tryCommit(
 
   {
     // Updating `currentRevision_` in unique manner if it hasn't changed.
-    std::unique_lock lock(commitMutex_);
+    std::unique_lock lock(commitMutex_, std::defer_lock);
+    if (!hasLocked) {
+      lock.lock();
+    }
 
     if (currentRevision_.number != oldRevision.number) {
       return CommitStatus::Failed;
diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.h b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.h
index 87b6a7afa47..6824ae9389c 100644
--- a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.h
+++ b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.h
@@ -111,7 +111,8 @@ class ShadowTree final {
    */
   CommitStatus tryCommit(
       const ShadowTreeCommitTransaction& transaction,
-      const CommitOptions& commitOptions) const;
+      const CommitOptions& commitOptions,
+      bool hasLocked = false) const;
 
   /*
    * Calls `tryCommit` in a loop until it finishes successfully.