[rush] Avoid redundant downloads when local processes race for the same build cache entry (#5883)

* Avoid redundant downloads when local processes race for the same build cache entry

When useDirectFileTransfersForBuildCache is enabled, multiple local Rush
processes (e.g. parallel "rush build" invocations on the same machine or CI
agent) restoring the same cache entry would each independently download it
from the cloud cache provider.

Use LockFile to have later processes wait for the first one to finish
downloading, mirroring the pattern already used for installing the package
manager (see InstallHelpers.ensureLocalPackageManagerAsync). After acquiring
the lock (or failing/timing out), recheck whether the final cache entry path
already exists before downloading, so a process that waited can skip the
network round-trip entirely if a peer already populated it.

This is strictly a best-effort optimization: if the lock cannot be acquired
(for example, it's held by a process on a different machine sharing a
network cache folder, where LockFile's stale-holder detection cannot see
across machines) we fall back to downloading independently, exactly as
before. Correctness never depends on the lock, since downloads always land
in a uniquely-named temp file that is atomically renamed into place.

The lock's resource name is derived by hashing the cache ID (sha1-hex),
since cache IDs can contain characters (e.g. "+", "_", "/") that
LockFile.getLockFilePath's resource name validation rejects, depending on
the configured cacheEntryNamePattern.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8d807794-b652-4bf8-bc4b-8ee4b2039f92

* Add change file

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8d807794-b652-4bf8-bc4b-8ee4b2039f92

* fixup! Avoid redundant downloads when local processes race for the same build cache entry

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: iclanton

common/changes/@microsoft/rush/iclanton-build-cache-download-lock_2026-07-18-00-51-14.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "comment": "Avoid redundant downloads when multiple local Rush processes race to restore the same build cache entry (when useDirectFileTransfersForBuildCache is enabled).",
+      "type": "none",
+      "packageName": "@microsoft/rush"
+    }
+  ],
+  "packageName": "@microsoft/rush",
+  "email": "198982749+Copilot@users.noreply.github.com"
+}

libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts

@@ -4,7 +4,7 @@
 import * as path from 'node:path';
 import * as crypto from 'node:crypto';
 
-import { FileSystem, type FolderItem, InternalError, Async } from '@rushstack/node-core-library';
+import { FileSystem, type FolderItem, InternalError, Async, LockFile } from '@rushstack/node-core-library';
 import type { ITerminal } from '@rushstack/terminal';
 
 import type { RushConfigurationProject } from '../../api/RushConfigurationProject';
@@ -15,6 +15,16 @@ import { TarExecutable } from '../../utilities/TarExecutable';
 import { EnvironmentVariableNames } from '../../api/EnvironmentConfiguration';
 import type { IBaseOperationExecutionResult } from '../operations/IOperationExecutionResult';
 
+/**
+ * How long to wait to acquire the per-cache-entry download lock (see
+ * {@link OperationBuildCache._getDirectFileTransferLockResourceName}) before giving up and
+ * downloading independently. This is strictly a best-effort optimization to avoid redundant
+ * downloads when multiple local Rush processes race to restore the same cache entry; it is not
+ * required for correctness, since downloads always land in a uniquely-named temp file that is
+ * atomically renamed into place.
+ */
+const DIRECT_FILE_TRANSFER_LOCK_MAX_WAIT_MS: number = 5 * 60 * 1000; // Five minutes
+
 /**
  * @internal
  */
@@ -66,6 +76,13 @@ interface IPathsToCache {
   outputFilePaths: string[];
 }
 
+function _getDirectFileTransferLockResourceName(cacheId: string): string {
+  // LockFile resource names must match /^[a-zA-Z0-9][a-zA-Z0-9-.]+[a-zA-Z0-9]$/, but cacheId may
+  // contain other characters (e.g. "/") depending on the configured cacheEntryNamePattern, so hash
+  // it into a fixed-length, lock-file-safe token.
+  return crypto.createHash('sha1').update(cacheId).digest('hex');
+}
+
 /**
  * @internal
  */
@@ -188,43 +205,79 @@ export class OperationBuildCache {
         // Use file-based path to avoid loading the entire cache entry into memory.
         // The provider downloads directly to a temp file that is atomically moved into place.
         const targetPath: string = this._localBuildCacheProvider.getCacheEntryPath(cacheId);
-        const tempTargetPath: string = OperationBuildCache._getTempLocalCacheEntryPath(targetPath);
+
+        // If multiple local Rush processes race to restore the same cache entry (e.g. parallel
+        // "rush build" invocations on the same machine or CI agent), avoid redundant downloads by
+        // having later processes wait for the first one to finish, mirroring the pattern used for
+        // installing the package manager (see InstallHelpers.ensureLocalPackageManagerAsync). This
+        // is strictly a best-effort optimization: if the lock cannot be acquired (for example,
+        // because it is held by a process on a different machine sharing a network cache folder,
+        // where the lock's stale-holder detection cannot see across machines) we simply fall back
+        // to downloading independently below. Correctness never depends on the lock, because
+        // downloads always land in a uniquely-named temp file that is atomically renamed into
+        // place.
+        let downloadLock: LockFile | undefined;
         try {
-          const downloadedToTempFile: boolean =
-            await this._cloudBuildCacheProvider.tryDownloadCacheEntryToFileAsync(
-              terminal,
-              cacheId,
-              tempTargetPath
-            );
-          if (downloadedToTempFile) {
-            await Async.runWithRetriesAsync({
-              action: () =>
-                FileSystem.moveAsync({
-                  sourcePath: tempTargetPath,
-                  destinationPath: targetPath,
-                  overwrite: true
-                }),
-              maxRetries: 2,
-              retryDelayMs: 500
-            });
+          downloadLock = await LockFile.acquireAsync(
+            path.dirname(targetPath),
+            _getDirectFileTransferLockResourceName(cacheId),
+            DIRECT_FILE_TRANSFER_LOCK_MAX_WAIT_MS
+          );
+        } catch (e) {
+          terminal.writeVerboseLine(
+            `Unable to acquire the local download lock for cache entry "${cacheId}"; downloading independently: ${e}`
+          );
+        }
+
+        try {
+          // Another process may have finished populating the cache entry while we were waiting
+          // for the lock (or the lock may not have been acquired at all).
+          if (await FileSystem.existsAsync(targetPath)) {
             cloudCacheHit = true;
             localCacheEntryPath = targetPath;
             updateLocalCacheSuccess = true;
-          }
-        } catch (e) {
-          terminal.writeVerboseLine(`Failed to download cache entry to local cache: ${e}`);
-          updateLocalCacheSuccess = false;
-        }
+          } else {
+            const tempTargetPath: string = OperationBuildCache._getTempLocalCacheEntryPath(targetPath);
+            try {
+              const downloadedToTempFile: boolean =
+                await this._cloudBuildCacheProvider.tryDownloadCacheEntryToFileAsync(
+                  terminal,
+                  cacheId,
+                  tempTargetPath
+                );
+              if (downloadedToTempFile) {
+                await Async.runWithRetriesAsync({
+                  action: () =>
+                    FileSystem.moveAsync({
+                      sourcePath: tempTargetPath,
+                      destinationPath: targetPath,
+                      overwrite: true
+                    }),
+                  maxRetries: 2,
+                  retryDelayMs: 500
+                });
+                cloudCacheHit = true;
+                localCacheEntryPath = targetPath;
+                updateLocalCacheSuccess = true;
+              }
+            } catch (e) {
+              terminal.writeVerboseLine(`Failed to download cache entry to local cache: ${e}`);
+              updateLocalCacheSuccess = false;
+            }
 
-        if (!cloudCacheHit) {
-          // Clean up any partial file left by the failed or missed download so it isn't
-          // mistaken for a valid cache entry on the next build. Providers may catch errors
-          // internally and return false instead of throwing, leaving a partially written file.
-          try {
-            await FileSystem.deleteFileAsync(tempTargetPath);
-          } catch {
-            // Ignore cleanup errors (file may not have been created)
+            if (!cloudCacheHit) {
+              // Clean up any partial file left by the failed or missed download so it isn't
+              // mistaken for a valid cache entry on the next build. Providers may catch errors
+              // internally and return false instead of throwing, leaving a partially written file.
+              try {
+                await FileSystem.deleteFileAsync(tempTargetPath);
+              } catch {
+                // Ignore cleanup errors (file may not have been created)
+              }
+            }
           }
+        } finally {
+          downloadLock?.release();
         }
       } else {
         const cacheEntryBuffer: Buffer | undefined =

libraries/rush-lib/src/logic/buildCache/test/OperationBuildCache.test.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
 // See LICENSE in the project root for license information.
 
-import { FileSystem, type FolderItem } from '@rushstack/node-core-library';
+import { FileSystem, type FolderItem, LockFile } from '@rushstack/node-core-library';
 import { StringBufferTerminalProvider, Terminal, type ITerminal } from '@rushstack/terminal';
 
 import type { BuildCacheConfiguration } from '../../../api/BuildCacheConfiguration';
@@ -77,6 +77,18 @@ describe(OperationBuildCache.name, () => {
   });
 
   describe('direct file cloud cache restore', () => {
+    let fakeLockRelease: jest.Mock;
+
+    beforeEach(() => {
+      fakeLockRelease = jest.fn();
+      // By default, simulate an uncontended lock acquisition and no pre-existing cache entry.
+      // Individual tests may override these to exercise the lock-contention/fallback paths.
+      jest
+        .spyOn(LockFile, 'acquireAsync')
+        .mockImplementation(async () => ({ release: fakeLockRelease }) as unknown as LockFile);
+      jest.spyOn(FileSystem, 'existsAsync').mockResolvedValue(false);
+    });
+
     afterEach(() => {
       Reflect.set(OperationBuildCache, '_tarUtilityPromise', undefined);
       jest.restoreAllMocks();
@@ -151,6 +163,12 @@ describe(OperationBuildCache.name, () => {
         })
       );
       expect(deleteFileAsyncSpy).not.toHaveBeenCalled();
+      expect(LockFile.acquireAsync).toHaveBeenCalledWith(
+        '/cache',
+        expect.stringMatching(/^[0-9a-f]{40}$/),
+        expect.any(Number)
+      );
+      expect(fakeLockRelease).toHaveBeenCalledTimes(1);
     });
 
     it('cleans up the temp file when a direct file download misses or fails', async () => {
@@ -174,6 +192,59 @@ describe(OperationBuildCache.name, () => {
       expect(tempPath).toMatch(/^\/cache\/acme-wizard-cache-entry-[0-9a-f]+\.temp$/);
       expect(deleteFileAsyncSpy).toHaveBeenCalledWith(tempPath);
     });
+
+    it('skips downloading when another process already populated the cache entry while waiting for the lock', async () => {
+      const tryDownloadCacheEntryToFileAsync: jest.Mock<
+        Promise<boolean>,
+        [ITerminal, string, string]
+      > = jest.fn();
+      const subject: OperationBuildCache = prepareDirectTransferSubject({
+        tryDownloadCacheEntryToFileAsync
+      });
+      const terminal: Terminal = new Terminal(new StringBufferTerminalProvider());
+      const tryUntarAsync: jest.Mock = jest.fn().mockResolvedValue(0);
+
+      jest.spyOn(FileSystem, 'deleteFolderAsync').mockResolvedValue();
+      // Simulate the cache entry having been fully populated (e.g. by another local process)
+      // by the time we acquired the lock.
+      jest.spyOn(FileSystem, 'existsAsync').mockResolvedValue(true);
+      Reflect.set(OperationBuildCache, '_tarUtilityPromise', Promise.resolve({ tryUntarAsync }));
+
+      const result: boolean = await subject.tryRestoreFromCacheAsync(terminal);
+
+      expect(result).toBe(true);
+      expect(tryDownloadCacheEntryToFileAsync).not.toHaveBeenCalled();
+      expect(tryUntarAsync).toHaveBeenCalledWith(
+        expect.objectContaining({
+          archivePath: '/cache/acme-wizard-cache-entry'
+        })
+      );
+      expect(fakeLockRelease).toHaveBeenCalledTimes(1);
+    });
+
+    it('falls back to downloading independently when the download lock cannot be acquired', async () => {
+      jest.spyOn(LockFile, 'acquireAsync').mockRejectedValue(new Error('Exceeded maximum wait time'));
+
+      const tryDownloadCacheEntryToFileAsync: jest.Mock<Promise<boolean>, [ITerminal, string, string]> = jest
+        .fn()
+        .mockResolvedValue(true);
+      const subject: OperationBuildCache = prepareDirectTransferSubject({
+        tryDownloadCacheEntryToFileAsync
+      });
+      const terminal: Terminal = new Terminal(new StringBufferTerminalProvider());
+      const tryUntarAsync: jest.Mock = jest.fn().mockResolvedValue(0);
+
+      jest.spyOn(FileSystem, 'deleteFolderAsync').mockResolvedValue();
+      jest.spyOn(FileSystem, 'moveAsync').mockResolvedValue();
+      Reflect.set(OperationBuildCache, '_tarUtilityPromise', Promise.resolve({ tryUntarAsync }));
+
+      const result: boolean = await subject.tryRestoreFromCacheAsync(terminal);
+
+      expect(result).toBe(true);
+      expect(tryDownloadCacheEntryToFileAsync).toHaveBeenCalledTimes(1);
+      // No lock instance was returned, so there is nothing to release.
+      expect(fakeLockRelease).not.toHaveBeenCalled();
+    });
   });
 
   describe('AppleDouble file exclusion', () => {