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

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', () => {