From 61eff4e566ae4e10523a9953ff23b22bc1b63dd8 Mon Sep 17 00:00:00 2001
From: Fabian Meumertzheim <fabian@meumertzhe.im>
Date: Mon, 10 Aug 2026 13:17:32 +0200
Subject: [PATCH] Unlock rewinding locks on exceptions and add profiler spans

This guards against interrupts and Errors causing locks to be held indefinitely.

Along the way add more profiler spans.
---
 .../RemoteRewoundActionSynchronizer.java      | 23 +++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteRewoundActionSynchronizer.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteRewoundActionSynchronizer.java
index b14263d6ddb3c7..960680d762d69a 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteRewoundActionSynchronizer.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteRewoundActionSynchronizer.java
@@ -149,7 +149,10 @@ private SilentCloseable enterActionPreparationForRewinding(Action action)
     if (localCoarseLock != null) {
       // This is the first time a rewound action has attempted to prepare for its execution.
       // Switch to using the fine locks under the protection of the coarse write lock.
-      localCoarseLock.writeLock().lockInterruptibly();
+      try (SilentCloseable c =
+          Profiler.instance().profile(ProfilerTask.ACTION_LOCK, "action.prepareFirstRewinding")) {
+        localCoarseLock.writeLock().lockInterruptibly();
+      }
       try {
         // Check again under the lock to avoid a race between multiple rewound actions attempting
         // to prepare for execution at the same time.
@@ -166,6 +169,8 @@ private SilentCloseable enterActionPreparationForRewinding(Action action)
                   // (https://github.com/openjdk/jdk/blob/b349f661ea5f14b258191134714a7e712c90ef3e/src/java.base/share/classes/java/util/concurrent/locks/StampedLock.java#L1039),
                   // TODO: Investigate the effect of fair locks on build wall time.
                   .build((ActionLookupData unused) -> new StampedLock().asReadWriteLock());
+          // Must be assigned after fineLocks as lockArtifactsForConsumption relies on a null
+          // coarseLock implying a non-null fineLocks.
           coarseLock = null;
         }
       } finally {
@@ -174,8 +179,18 @@ private SilentCloseable enterActionPreparationForRewinding(Action action)
     }
 
     var writeLock = fineLocks.get(outputKeyFor(action)).writeLock();
-    writeLock.lockInterruptibly();
-    prepareOutputsForRewinding(action);
+    try (SilentCloseable c =
+        Profiler.instance()
+            .profile(ProfilerTask.ACTION_LOCK, "action.awaitRewoundActionConsumers")) {
+      writeLock.lockInterruptibly();
+    }
+    try (SilentCloseable c =
+        Profiler.instance().profile(ProfilerTask.INFO, "action.prepareOutputsForRewinding")) {
+      prepareOutputsForRewinding(action);
+    } catch (Throwable t) {
+      writeLock.unlock();
+      throw t;
+    }
     return writeLock::unlock;
   }
 
@@ -265,7 +280,7 @@ private SilentCloseable lockArtifactsForConsumption(
         readLock.lockInterruptibly();
         locksToUnlockBuilder.add(readLock);
       }
-    } catch (InterruptedException e) {
+    } catch (Throwable e) {
       for (var readLock : locksToUnlockBuilder.build()) {
         readLock.unlock();
       }