fix(@angular/build): prevent concurrent stylesheet bundling esbuild context leaks

packages/angular/build/src/tools/esbuild/bundler-context.ts

@@ -59,6 +59,8 @@ export class BundlerContext {
   #esbuildContext?: BuildContext<{ metafile: true; write: false }>;
   #esbuildOptions?: BuildOptions & { metafile: true; write: false };
   #esbuildResult?: BundleContextResult;
+  #activeBundlePromise?: Promise<BundleContextResult>;
+  #disposed = false;
   #optionsFactory: BundlerOptionsFactory<BuildOptions & { metafile: true; write: false }>;
   #shouldCacheResult: boolean;
   #loadCache?: MemoryLoadResultCache;
@@ -177,7 +179,18 @@ export class BundlerContext {
       return this.#esbuildResult;
     }
 
-    const result = await this.#performBundle();
+    if (!force && this.#activeBundlePromise) {
+      return this.#activeBundlePromise;
+    }
+
+    const bundlePromise = this.#performBundle().finally(() => {
+      if (this.#activeBundlePromise === bundlePromise) {
+        this.#activeBundlePromise = undefined;
+      }
+    });
+    this.#activeBundlePromise = bundlePromise;
+
+    const result = await bundlePromise;
     if (this.#shouldCacheResult) {
       this.#esbuildResult = result;
     }
@@ -207,7 +220,12 @@ export class BundlerContext {
       } else {
         // Create a build context and perform the build.
         // Context creation does not perform a build.
-        this.#esbuildContext = await context(this.#esbuildOptions);
+        const esbuildContext = await context(this.#esbuildOptions);
+        if (this.#disposed) {
+          await esbuildContext.dispose();
+          throw new Error('BundlerContext was disposed during build.');
+        }
+        this.#esbuildContext = esbuildContext;
         result = await this.#esbuildContext.rebuild();
       }
     } catch (failure) {
@@ -446,9 +464,11 @@ export class BundlerContext {
    * @returns A promise that resolves when disposal is complete.
    */
   async dispose(): Promise<void> {
+    this.#disposed = true;
     try {
       this.#esbuildOptions = undefined;
       this.#esbuildResult = undefined;
+      this.#activeBundlePromise = undefined;
       this.#loadCache = undefined;
       await this.#esbuildContext?.dispose();
     } finally {

packages/angular/build/src/tools/esbuild/cache.ts

@@ -44,11 +44,26 @@ export interface CacheStore<V> {
  * to use a cache.
  */
 export class Cache<V, S extends CacheStore<V> = CacheStore<V>> {
+  // In-flight creator promises to deduplicate concurrent requests for the same key.
+  readonly #requests = new Map<string, Promise<V>>();
+  // Track how many writes occurred for a key to detect mutations during await gaps.
+  readonly #writeCounts = new Map<string, number>();
+  // Count the number of active, pending getOrCreate operations per key to avoid memory leaks.
+  readonly #pendingGets = new Map<string, number>();
+
   constructor(
     protected readonly store: S,
     readonly namespace?: string,
   ) {}
 
+  #incrementWrite(key: string) {
+    // Only track write counts if there is a pending getOrCreate operation active for the key.
+    // This ensures that write counts are not leaked when no concurrent gets are running.
+    if (this.#pendingGets.has(key)) {
+      this.#writeCounts.set(key, (this.#writeCounts.get(key) || 0) + 1);
+    }
+  }
+
   /**
    * Prefixes a key with the cache namespace if present.
    * @param key A key string to prefix.
@@ -72,14 +87,75 @@ export class Cache<V, S extends CacheStore<V> = CacheStore<V>> {
    */
   async getOrCreate(key: string, creator: () => V | Promise<V>): Promise<V> {
     const namespacedKey = this.withNamespace(key);
-    let value = await this.store.get(namespacedKey);
 
-    if (value === undefined) {
-      value = await creator();
-      await this.store.set(namespacedKey, value);
+    // 1. If another call is already running the creator for this key, share its promise.
+    let activeRequest = this.#requests.get(namespacedKey);
+    if (activeRequest !== undefined) {
+      return activeRequest;
     }
 
-    return value;
+    // Increment pending gets count to enable write-tracking for this key.
+    const currentPending = this.#pendingGets.get(namespacedKey) || 0;
+    this.#pendingGets.set(namespacedKey, currentPending + 1);
+
+    try {
+      const startWriteCount = this.#writeCounts.get(namespacedKey) || 0;
+
+      // 2. Query the backing store. Since store.get can be async, we yield to the event loop.
+      const value = await this.store.get(namespacedKey);
+
+      // If a write (e.g. put) occurred during the store.get await gap, we must abort
+      // the current execution and restart to ensure we return the newly written value.
+      if ((this.#writeCounts.get(namespacedKey) || 0) !== startWriteCount) {
+        return this.getOrCreate(key, creator);
+      }
+
+      if (value !== undefined) {
+        return value;
+      }
+
+      // 3. Recheck active request after the await gap in case another concurrent call
+      // initiated a creator during the store.get wait.
+      activeRequest = this.#requests.get(namespacedKey);
+      if (activeRequest !== undefined) {
+        return activeRequest;
+      }
+
+      // 4. Run the creator to produce the new value, and store its promise in #requests.
+      activeRequest = Promise.resolve(creator()).then(
+        async (newValue) => {
+          // Ensure this request is still the active one before writing back to the store
+          // (prevents overwriting newer data if put() was called before resolution).
+          if (this.#requests.get(namespacedKey) === activeRequest) {
+            this.#incrementWrite(namespacedKey);
+            await this.store.set(namespacedKey, newValue);
+            this.#requests.delete(namespacedKey);
+          }
+
+          return newValue;
+        },
+        (error) => {
+          // Clean up the active request if the creator fails.
+          if (this.#requests.get(namespacedKey) === activeRequest) {
+            this.#requests.delete(namespacedKey);
+          }
+          throw error;
+        },
+      );
+
+      this.#requests.set(namespacedKey, activeRequest);
+
+      return activeRequest;
+    } finally {
+      // Clean up write counts and pending gets once all concurrent gets for this key finish.
+      const current = this.#pendingGets.get(namespacedKey) || 0;
+      if (current <= 1) {
+        this.#pendingGets.delete(namespacedKey);
+        this.#writeCounts.delete(namespacedKey);
+      } else {
+        this.#pendingGets.set(namespacedKey, current - 1);
+      }
+    }
   }
 
   /**
@@ -100,7 +176,19 @@ export class Cache<V, S extends CacheStore<V> = CacheStore<V>> {
    * @param value A value to put in the cache.
    */
   async put(key: string, value: V): Promise<void> {
-    await this.store.set(this.withNamespace(key), value);
+    const namespacedKey = this.withNamespace(key);
+    this.#requests.delete(namespacedKey);
+    this.#incrementWrite(namespacedKey);
+    await this.store.set(namespacedKey, value);
+  }
+
+  /**
+   * Clears the base class internal state (requests, write counts, and pending gets).
+   */
+  protected clearInternal(): void {
+    this.#requests.clear();
+    this.#writeCounts.clear();
+    this.#pendingGets.clear();
   }
 }
 
@@ -116,6 +204,7 @@ export class MemoryCache<V> extends Cache<V, Map<string, V>> {
    * Removes all entries from the cache instance.
    */
   clear() {
+    this.clearInternal();
     this.store.clear();
   }
 

packages/angular/build/src/tools/esbuild/cache_spec.ts

@@ -0,0 +1,144 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.dev/license
+ */
+
+import { MemoryCache } from './cache';
+
+describe('MemoryCache', () => {
+  let cache: MemoryCache<string>;
+
+  beforeEach(() => {
+    cache = new MemoryCache<string>();
+  });
+
+  it('should return cached value on subsequent getOrCreate calls', async () => {
+    let callCount = 0;
+    const creator = () => {
+      callCount++;
+
+      return 'value';
+    };
+
+    const val1 = await cache.getOrCreate('key', creator);
+    const val2 = await cache.getOrCreate('key', creator);
+
+    expect(val1).toBe('value');
+    expect(val2).toBe('value');
+    expect(callCount).toBe(1);
+  });
+
+  it('should call creator only once for concurrent getOrCreate calls with the same key', async () => {
+    let callCount = 0;
+    let resolveCreator!: (value: string) => void;
+    const promise = new Promise<string>((resolve) => {
+      resolveCreator = resolve;
+    });
+
+    const creator = () => {
+      callCount++;
+
+      return promise;
+    };
+
+    const p1 = cache.getOrCreate('key', creator);
+    const p2 = cache.getOrCreate('key', creator);
+
+    resolveCreator('concurrent-value');
+
+    const [val1, val2] = await Promise.all([p1, p2]);
+
+    expect(val1).toBe('concurrent-value');
+    expect(val2).toBe('concurrent-value');
+    expect(callCount).toBe(1);
+  });
+
+  it('should call creator multiple times for concurrent getOrCreate calls with different keys', async () => {
+    let callCount = 0;
+    const creator = (val: string) => {
+      callCount++;
+
+      return Promise.resolve(val);
+    };
+
+    const p1 = cache.getOrCreate('key1', () => creator('value1'));
+    const p2 = cache.getOrCreate('key2', () => creator('value2'));
+
+    const [val1, val2] = await Promise.all([p1, p2]);
+
+    expect(val1).toBe('value1');
+    expect(val2).toBe('value2');
+    expect(callCount).toBe(2);
+  });
+
+  it('should clean up active request if creator throws/rejects', async () => {
+    let callCount = 0;
+    let rejectCreator!: (err: Error) => void;
+    const promise = new Promise<string>((_, reject) => {
+      rejectCreator = reject;
+    });
+
+    const creator = () => {
+      callCount++;
+
+      return promise;
+    };
+
+    const p1 = cache.getOrCreate('key', creator);
+    const p2 = cache.getOrCreate('key', creator);
+
+    rejectCreator(new Error('creator error'));
+
+    await expectAsync(p1).toBeRejectedWithError('creator error');
+    await expectAsync(p2).toBeRejectedWithError('creator error');
+
+    // Subsequent call should trigger the creator again
+    const p3 = cache.getOrCreate('key', () => {
+      callCount++;
+
+      return Promise.resolve('new-value');
+    });
+    const val3 = await p3;
+    expect(val3).toBe('new-value');
+    expect(callCount).toBe(2);
+  });
+
+  it('should override/clear active requests when put is called', async () => {
+    let resolveCreator!: (value: string) => void;
+    const promise = new Promise<string>((resolve) => {
+      resolveCreator = resolve;
+    });
+
+    let creatorStarted!: (value: void) => void;
+    const creatorStartedPromise = new Promise<void>((resolve) => {
+      creatorStarted = resolve;
+    });
+
+    const creator = () => {
+      creatorStarted();
+
+      return promise;
+    };
+
+    const p1 = cache.getOrCreate('key', creator);
+
+    // Wait for the creator to be called so that the active request is created
+    await creatorStartedPromise;
+
+    // Call put before the creator promise resolves
+    await cache.put('key', 'override-value');
+
+    resolveCreator('original-value');
+
+    const val1 = await p1;
+    // p1 was already returned, so it resolves to original-value
+    expect(val1).toBe('original-value');
+
+    // Subsequent getOrCreate should return the put/overridden value, not the resolved original-value
+    const val2 = await cache.getOrCreate('key', () => 'should-not-run');
+    expect(val2).toBe('override-value');
+  });
+});