diff --git a/packages/angular/build/src/tools/esbuild/bundler-context.ts b/packages/angular/build/src/tools/esbuild/bundler-context.ts index dd2764c8c608..7fe28e5a51d4 100644 --- a/packages/angular/build/src/tools/esbuild/bundler-context.ts +++ b/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; + #disposed = false; #optionsFactory: BundlerOptionsFactory; #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 { + this.#disposed = true; try { this.#esbuildOptions = undefined; this.#esbuildResult = undefined; + this.#activeBundlePromise = undefined; this.#loadCache = undefined; await this.#esbuildContext?.dispose(); } finally { diff --git a/packages/angular/build/src/tools/esbuild/cache.ts b/packages/angular/build/src/tools/esbuild/cache.ts index 5bd0dc84d73f..0634ec5268e8 100644 --- a/packages/angular/build/src/tools/esbuild/cache.ts +++ b/packages/angular/build/src/tools/esbuild/cache.ts @@ -44,11 +44,26 @@ export interface CacheStore { * to use a cache. */ export class Cache = CacheStore> { + // In-flight creator promises to deduplicate concurrent requests for the same key. + readonly #requests = new Map>(); + // Track how many writes occurred for a key to detect mutations during await gaps. + readonly #writeCounts = new Map(); + // Count the number of active, pending getOrCreate operations per key to avoid memory leaks. + readonly #pendingGets = new Map(); + 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 = CacheStore> { */ async getOrCreate(key: string, creator: () => V | Promise): Promise { 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 = CacheStore> { * @param value A value to put in the cache. */ async put(key: string, value: V): Promise { - 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 extends Cache> { * Removes all entries from the cache instance. */ clear() { + this.clearInternal(); this.store.clear(); } diff --git a/packages/angular/build/src/tools/esbuild/cache_spec.ts b/packages/angular/build/src/tools/esbuild/cache_spec.ts new file mode 100644 index 000000000000..a30852836eff --- /dev/null +++ b/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; + + beforeEach(() => { + cache = new MemoryCache(); + }); + + 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((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((_, 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((resolve) => { + resolveCreator = resolve; + }); + + let creatorStarted!: (value: void) => void; + const creatorStartedPromise = new Promise((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'); + }); +});