From c86e1f1932adba4bda92f166e61bc26fd0bf0c98 Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Fri, 3 Jul 2026 09:20:49 +0100 Subject: [PATCH 1/6] fix: release source token scraper before retry to prevent deadlock --- src/deploy/functions/release/fabricator.ts | 42 ++++++++++++------- .../release/sourceTokenScraper.spec.ts | 26 ++++++++++++ .../functions/release/sourceTokenScraper.ts | 19 ++++++++- 3 files changed, 72 insertions(+), 15 deletions(-) diff --git a/src/deploy/functions/release/fabricator.ts b/src/deploy/functions/release/fabricator.ts index be42e9a0e3a..1c1512d281e 100644 --- a/src/deploy/functions/release/fabricator.ts +++ b/src/deploy/functions/release/fabricator.ts @@ -422,13 +422,20 @@ export class Fabricator { if (experiments.isEnabled("functionsv2deployoptimizations")) { apiFunction.buildConfig.sourceToken = await scraper.getToken(); } - const op: { name: string } = await gcfV2.createFunction(apiFunction); - return await poller.pollOperation({ - ...gcfV2PollerOptions, - pollerName: `create-${endpoint.codebase}-${endpoint.region}-${endpoint.id}`, - operationResourceName: op.name, - onPoll: scraper.poller, - }); + try { + const op: { name: string } = await gcfV2.createFunction(apiFunction); + return await poller.pollOperation({ + ...gcfV2PollerOptions, + pollerName: `create-${endpoint.codebase}-${endpoint.region}-${endpoint.id}`, + operationResourceName: op.name, + onPoll: scraper.poller, + }); + } catch (err) { + // Release the scraper so the next retry (or concurrent waiters) are + // not stuck awaiting a token promise that will never be fulfilled. + scraper.abort(); + throw err; + } }) .catch(async (err: any) => { // Abort waiting on source token so other concurrent calls don't get stuck @@ -571,13 +578,20 @@ export class Fabricator { if (experiments.isEnabled("functionsv2deployoptimizations")) { apiFunction.buildConfig.sourceToken = await scraper.getToken(); } - const op: { name: string } = await gcfV2.updateFunction(apiFunction); - return await poller.pollOperation({ - ...gcfV2PollerOptions, - pollerName: `update-${endpoint.codebase}-${endpoint.region}-${endpoint.id}`, - operationResourceName: op.name, - onPoll: scraper.poller, - }); + try { + const op: { name: string } = await gcfV2.updateFunction(apiFunction); + return await poller.pollOperation({ + ...gcfV2PollerOptions, + pollerName: `update-${endpoint.codebase}-${endpoint.region}-${endpoint.id}`, + operationResourceName: op.name, + onPoll: scraper.poller, + }); + } catch (err) { + // Release the scraper so the next retry (or concurrent waiters) are + // not stuck awaiting a token promise that will never be fulfilled. + scraper.abort(); + throw err; + } }, { retryCodes: [...DEFAULT_RETRY_CODES, CLOUD_RUN_RESOURCE_EXHAUSTED_CODE] }, ) diff --git a/src/deploy/functions/release/sourceTokenScraper.spec.ts b/src/deploy/functions/release/sourceTokenScraper.spec.ts index e3cd4d329f1..aad641ea322 100644 --- a/src/deploy/functions/release/sourceTokenScraper.spec.ts +++ b/src/deploy/functions/release/sourceTokenScraper.spec.ts @@ -80,6 +80,32 @@ describe("SourceTokenScraper", () => { await expect(scraper.getToken()).to.eventually.equal("magic token"); }); + it("abort before a concurrent waiter re-arms the scraper so the retry can proceed", async () => { + const scraper = new SourceTokenScraper(); + // First caller transitions NONE → FETCHING. + await expect(scraper.getToken()).to.eventually.be.undefined; + + // Simulate a failed deploy: abort() is called before the retry fires. + scraper.abort(); + + // The retry's getToken() should resolve immediately (promise already + // settled with {aborted: true}) and return undefined, not deadlock. + const secondToken = scraper.getToken(); + const timeout = new Promise((_, reject) => + setTimeout(() => reject(new Error("deadlock: getToken() did not resolve")), 50), + ); + await expect(Promise.race([secondToken, timeout])).to.eventually.be.undefined; + + // After the re-arm, a subsequent poller call delivers the token normally. + scraper.poller({ + metadata: { + sourceToken: "retry token", + target: "projects/p/locations/l/functions/f", + }, + }); + await expect(scraper.getToken()).to.eventually.equal("retry token"); + }); + it("concurrent requests for source token", async () => { const scraper = new SourceTokenScraper(); diff --git a/src/deploy/functions/release/sourceTokenScraper.ts b/src/deploy/functions/release/sourceTokenScraper.ts index 6cdfc1c368d..635e488da4d 100644 --- a/src/deploy/functions/release/sourceTokenScraper.ts +++ b/src/deploy/functions/release/sourceTokenScraper.ts @@ -2,6 +2,10 @@ import { FirebaseError } from "../../../error"; import { assertExhaustive } from "../../../functional"; import { logger } from "../../../logger"; +// How long a concurrent getToken() call will wait for the first deploy's poller +// to produce a token before giving up and proceeding without one. +const SOURCE_TOKEN_FETCH_TIMEOUT_MS = 10 * 60 * 1000; // 10 minutes + type TokenFetchState = "NONE" | "FETCHING" | "VALID"; interface TokenFetchResult { token?: string; @@ -35,7 +39,20 @@ export class SourceTokenScraper { this.fetchState = "FETCHING"; return undefined; } else if (this.fetchState === "FETCHING") { - const tokenResult = await this.promise; + // Race the token promise against a deadline so a lost token producer + // (e.g. a failed deploy that didn't call abort()) degrades to a + // token-less deploy rather than hanging the process forever. + let timeoutHandle: ReturnType | undefined; + const timeoutPromise = new Promise((resolve) => { + timeoutHandle = setTimeout(() => { + logger.warn( + "Timed out waiting for a source token. Proceeding without one, which may slow the deploy.", + ); + resolve({ aborted: true }); + }, SOURCE_TOKEN_FETCH_TIMEOUT_MS); + }); + const tokenResult = await Promise.race([this.promise, timeoutPromise]); + clearTimeout(timeoutHandle); if (tokenResult.aborted) { this.promise = new Promise((resolve) => (this.resolve = resolve)); return undefined; From d3faa3229098f10b5eba0fc5bd9c8d529c809969 Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Fri, 3 Jul 2026 10:23:15 +0100 Subject: [PATCH 2/6] test: add additional test --- .../functions/release/fabricator.spec.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/deploy/functions/release/fabricator.spec.ts b/src/deploy/functions/release/fabricator.spec.ts index 20fda610483..241f2b5d272 100644 --- a/src/deploy/functions/release/fabricator.spec.ts +++ b/src/deploy/functions/release/fabricator.spec.ts @@ -881,6 +881,30 @@ describe("Fabricator", () => { ); }); + it("does not deadlock when updateFunction receives a retryable 409 on the first attempt", async () => { + // Regression test for: a 409 retry re-entering getToken() while the + // scraper is still FETCHING would await a promise that only the poller + // could resolve — but the poller never ran because the request failed. + // The fix calls scraper.abort() inside the closure catch so the promise + // is settled before the executor's backoff fires and the closure re-runs. + const retryingFab = new fabricator.Fabricator({ + ...ctorArgs, + functionExecutor: new executor.QueueExecutor({ retries: 1, backoff: 10, maxBackoff: 10 }), + }); + + const err409 = new Error("unable to queue the operation"); + (err409 as any).status = 409; + gcfv2.updateFunction.onFirstCall().rejects(err409); + gcfv2.updateFunction.resolves({ name: "op", done: false }); + poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); + + const ep = endpoint({ httpsTrigger: {} }, { platform: "gcfv2" }); + await expect( + retryingFab.updateV2Function(ep, new scraper.SourceTokenScraper()), + ).to.eventually.be.undefined; + expect(gcfv2.updateFunction).to.have.been.calledTwice; + }); + it("throws on set invoker failure", async () => { gcfv2.updateFunction.resolves({ name: "op", done: false }); poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); From b1f865a3cc6bd3abb155379baaefe4560d53a46c Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Fri, 3 Jul 2026 11:10:45 +0100 Subject: [PATCH 3/6] chore: fix linting --- src/deploy/functions/release/fabricator.spec.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/deploy/functions/release/fabricator.spec.ts b/src/deploy/functions/release/fabricator.spec.ts index 241f2b5d272..a01d5309a11 100644 --- a/src/deploy/functions/release/fabricator.spec.ts +++ b/src/deploy/functions/release/fabricator.spec.ts @@ -899,9 +899,8 @@ describe("Fabricator", () => { poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); const ep = endpoint({ httpsTrigger: {} }, { platform: "gcfv2" }); - await expect( - retryingFab.updateV2Function(ep, new scraper.SourceTokenScraper()), - ).to.eventually.be.undefined; + await expect(retryingFab.updateV2Function(ep, new scraper.SourceTokenScraper())).to.eventually + .be.undefined; expect(gcfv2.updateFunction).to.have.been.calledTwice; }); From 5b745c4735bb9cd4d336254694d802cc33f2ed71 Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Fri, 3 Jul 2026 14:29:58 +0100 Subject: [PATCH 4/6] fix: remove double-abort and extend srcaper fix to v1 functions --- .../functions/release/fabricator.spec.ts | 36 ++++++++++++++++ src/deploy/functions/release/fabricator.ts | 42 +++++++++++-------- .../functions/release/sourceTokenScraper.ts | 19 +++++---- 3 files changed, 70 insertions(+), 27 deletions(-) diff --git a/src/deploy/functions/release/fabricator.spec.ts b/src/deploy/functions/release/fabricator.spec.ts index a01d5309a11..3696f50ce08 100644 --- a/src/deploy/functions/release/fabricator.spec.ts +++ b/src/deploy/functions/release/fabricator.spec.ts @@ -339,6 +339,24 @@ describe("Fabricator", () => { await fab.createV1Function(ep1, new scraper.SourceTokenScraper()); expect(gcf.setInvokerCreate).to.not.have.been.called; }); + + it("does not deadlock when createFunction receives a retryable 409 on the first attempt", async () => { + const retryingFab = new fabricator.Fabricator({ + ...ctorArgs, + functionExecutor: new executor.QueueExecutor({ retries: 1, backoff: 10, maxBackoff: 10 }), + }); + + const err409 = new Error("unable to queue the operation"); + (err409 as any).status = 409; + gcf.createFunction.onFirstCall().rejects(err409); + gcf.createFunction.resolves({ name: "op", type: "create", done: false }); + poller.pollOperation.resolves(); + + const ep = endpoint({ scheduleTrigger: {} }); + await expect(retryingFab.createV1Function(ep, new scraper.SourceTokenScraper())).to.eventually + .be.undefined; + expect(gcf.createFunction).to.have.been.calledTwice; + }); }); describe("updateV1Function", () => { @@ -427,6 +445,24 @@ describe("Fabricator", () => { await fab.updateV1Function(ep, new scraper.SourceTokenScraper()); expect(gcf.setInvokerUpdate).to.not.have.been.called; }); + + it("does not deadlock when updateFunction receives a retryable 409 on the first attempt", async () => { + const retryingFab = new fabricator.Fabricator({ + ...ctorArgs, + functionExecutor: new executor.QueueExecutor({ retries: 1, backoff: 10, maxBackoff: 10 }), + }); + + const err409 = new Error("unable to queue the operation"); + (err409 as any).status = 409; + gcf.updateFunction.onFirstCall().rejects(err409); + gcf.updateFunction.resolves({ name: "op", type: "update", done: false }); + poller.pollOperation.resolves(); + + const ep = endpoint({ httpsTrigger: {} }); + await expect(retryingFab.updateV1Function(ep, new scraper.SourceTokenScraper())).to.eventually + .be.undefined; + expect(gcf.updateFunction).to.have.been.calledTwice; + }); }); describe("deleteV1Function", () => { diff --git a/src/deploy/functions/release/fabricator.ts b/src/deploy/functions/release/fabricator.ts index 1c1512d281e..01e35bb4b62 100644 --- a/src/deploy/functions/release/fabricator.ts +++ b/src/deploy/functions/release/fabricator.ts @@ -293,13 +293,18 @@ export class Fabricator { .run(async () => { // try to get the source token right before deploying apiFunction.sourceToken = await scraper.getToken(); - const op: { name: string } = await gcf.createFunction(apiFunction); - return poller.pollOperation({ - ...gcfV1PollerOptions, - pollerName: `create-${endpoint.codebase}-${endpoint.region}-${endpoint.id}`, - operationResourceName: op.name, - onPoll: scraper.poller, - }); + try { + const op: { name: string } = await gcf.createFunction(apiFunction); + return poller.pollOperation({ + ...gcfV1PollerOptions, + pollerName: `create-${endpoint.codebase}-${endpoint.region}-${endpoint.id}`, + operationResourceName: op.name, + onPoll: scraper.poller, + }); + } catch (err) { + scraper.abort(); + throw err; + } }) .catch(rethrowAs(endpoint, "create")); @@ -438,9 +443,6 @@ export class Fabricator { } }) .catch(async (err: any) => { - // Abort waiting on source token so other concurrent calls don't get stuck - scraper.abort(); - // If the createFunction call returns RPC error code RESOURCE_EXHAUSTED (8), // we have exhausted the underlying Cloud Run API quota. To retry, we need to // first delete the GCF function resource, then call createFunction again. @@ -527,13 +529,18 @@ export class Fabricator { const resultFunction = await this.functionExecutor .run(async () => { apiFunction.sourceToken = await scraper.getToken(); - const op: { name: string } = await gcf.updateFunction(apiFunction); - return await poller.pollOperation({ - ...gcfV1PollerOptions, - pollerName: `update-${endpoint.codebase}-${endpoint.region}-${endpoint.id}`, - operationResourceName: op.name, - onPoll: scraper.poller, - }); + try { + const op: { name: string } = await gcf.updateFunction(apiFunction); + return await poller.pollOperation({ + ...gcfV1PollerOptions, + pollerName: `update-${endpoint.codebase}-${endpoint.region}-${endpoint.id}`, + operationResourceName: op.name, + onPoll: scraper.poller, + }); + } catch (err) { + scraper.abort(); + throw err; + } }) .catch(rethrowAs(endpoint, "update")); @@ -596,7 +603,6 @@ export class Fabricator { { retryCodes: [...DEFAULT_RETRY_CODES, CLOUD_RUN_RESOURCE_EXHAUSTED_CODE] }, ) .catch((err: any) => { - scraper.abort(); logger.error((err as Error).message); throw new reporter.DeploymentError(endpoint, "update", err); }); diff --git a/src/deploy/functions/release/sourceTokenScraper.ts b/src/deploy/functions/release/sourceTokenScraper.ts index 635e488da4d..88e1636ba95 100644 --- a/src/deploy/functions/release/sourceTokenScraper.ts +++ b/src/deploy/functions/release/sourceTokenScraper.ts @@ -1,6 +1,7 @@ import { FirebaseError } from "../../../error"; import { assertExhaustive } from "../../../functional"; import { logger } from "../../../logger"; +import { timeoutFallback } from "../../../timeout"; // How long a concurrent getToken() call will wait for the first deploy's poller // to produce a token before giving up and proceeding without one. @@ -10,6 +11,7 @@ type TokenFetchState = "NONE" | "FETCHING" | "VALID"; interface TokenFetchResult { token?: string; aborted: boolean; + timedOut?: boolean; } /** @@ -42,18 +44,17 @@ export class SourceTokenScraper { // Race the token promise against a deadline so a lost token producer // (e.g. a failed deploy that didn't call abort()) degrades to a // token-less deploy rather than hanging the process forever. - let timeoutHandle: ReturnType | undefined; - const timeoutPromise = new Promise((resolve) => { - timeoutHandle = setTimeout(() => { + const tokenResult: TokenFetchResult = await timeoutFallback( + this.promise, + { aborted: true, timedOut: true }, + SOURCE_TOKEN_FETCH_TIMEOUT_MS, + ); + if (tokenResult.aborted) { + if (tokenResult.timedOut) { logger.warn( "Timed out waiting for a source token. Proceeding without one, which may slow the deploy.", ); - resolve({ aborted: true }); - }, SOURCE_TOKEN_FETCH_TIMEOUT_MS); - }); - const tokenResult = await Promise.race([this.promise, timeoutPromise]); - clearTimeout(timeoutHandle); - if (tokenResult.aborted) { + } this.promise = new Promise((resolve) => (this.resolve = resolve)); return undefined; } From 3a7418f8793ce4b9343c21accfc34aea9764c012 Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Thu, 16 Jul 2026 11:50:09 +0100 Subject: [PATCH 5/6] fix: clear timer when race settles --- src/timeout.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/timeout.ts b/src/timeout.ts index acf3e172c68..41b312c4038 100644 --- a/src/timeout.ts +++ b/src/timeout.ts @@ -6,10 +6,17 @@ export async function timeoutFallback( value: V, timeoutMillis = 2000, ): Promise { - return Promise.race([ - promise, - new Promise((resolve) => setTimeout(() => resolve(value), timeoutMillis)), - ]); + let timer: NodeJS.Timeout | undefined; + try { + return await Promise.race([ + promise, + new Promise((resolve) => { + timer = setTimeout(() => resolve(value), timeoutMillis); + }), + ]); + } finally { + clearTimeout(timer); + } } export async function timeoutError( From 048e93c670d9edd0b7448a3d67b81f200571ab09 Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Thu, 16 Jul 2026 11:50:58 +0100 Subject: [PATCH 6/6] fix: increase SOURCE_TOKEN_FETCH_TIMEOUT_MS to match masterTimeout --- src/deploy/functions/release/sourceTokenScraper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deploy/functions/release/sourceTokenScraper.ts b/src/deploy/functions/release/sourceTokenScraper.ts index 88e1636ba95..614e8002b26 100644 --- a/src/deploy/functions/release/sourceTokenScraper.ts +++ b/src/deploy/functions/release/sourceTokenScraper.ts @@ -5,7 +5,7 @@ import { timeoutFallback } from "../../../timeout"; // How long a concurrent getToken() call will wait for the first deploy's poller // to produce a token before giving up and proceeding without one. -const SOURCE_TOKEN_FETCH_TIMEOUT_MS = 10 * 60 * 1000; // 10 minutes +const SOURCE_TOKEN_FETCH_TIMEOUT_MS = 25 * 60 * 1000; // 25 minutes, matches fabricator.ts masterTimeout type TokenFetchState = "NONE" | "FETCHING" | "VALID"; interface TokenFetchResult {