From 2b5893e0592fa3d150c04738cba1cd549e70378e Mon Sep 17 00:00:00 2001 From: wangbill Date: Wed, 29 Jul 2026 16:28:31 -0700 Subject: [PATCH] fix: cancel pending timers on continue-as-new in InMemoryOrchestrationBackend Timer IDs are per-execution sequence numbers that restart at 1 after continue-as-new, so a timer left pending from the previous iteration fires a TimerFired event whose ID collides with the new iteration's first task and completes it early. Cancel the instance's pending timers before resetting state. Rebased onto current main. The test is a rewrite of the original: that version used a 60s stale timer against a 50ms real timer, so the stale timer could never fire within the test and it passed with and without the fix. This version inverts the durations (50ms stale vs 1.5s real) and asserts on elapsed time. Verified on current main: without the fix the orchestration completes in 58ms (expected >1000); with the fix it takes 1562ms. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5aaa70df-fae3-4570-aabe-0fc96cb869bc --- .../src/testing/in-memory-backend.ts | 5 +++ .../test/in-memory-backend.spec.ts | 40 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/packages/durabletask-js/src/testing/in-memory-backend.ts b/packages/durabletask-js/src/testing/in-memory-backend.ts index c1748da..b348c55 100644 --- a/packages/durabletask-js/src/testing/in-memory-backend.ts +++ b/packages/durabletask-js/src/testing/in-memory-backend.ts @@ -579,6 +579,11 @@ export class InMemoryOrchestrationBackend { const newInput = completeAction.getResult()?.getValue(); const carryoverEvents = completeAction.getCarryovereventsList(); + // Cancel timers still pending from the previous iteration. Their timer IDs are + // sequence numbers that restart at 1 in the new iteration, so a leaked timer + // would fire a TimerFired event that completes an unrelated task. + this.cancelInstanceTimers(instance.instanceId); + // Reset instance state instance.history = []; instance.input = newInput; diff --git a/packages/durabletask-js/test/in-memory-backend.spec.ts b/packages/durabletask-js/test/in-memory-backend.spec.ts index e2fac55..ed54898 100644 --- a/packages/durabletask-js/test/in-memory-backend.spec.ts +++ b/packages/durabletask-js/test/in-memory-backend.spec.ts @@ -933,4 +933,44 @@ describe("In-Memory Backend", () => { expect(backend.toClientStatus(suspendedInstance!.status)).toEqual(OrchestrationStatus.SUSPENDED); }); }); + + it("should cancel pending timers on continue-as-new", async () => { + // Timer IDs are per-execution sequence numbers that restart at 1 after + // continue-as-new. A timer left pending from iteration 1 therefore fires a + // TimerFired event whose ID collides with iteration 2's first task, completing + // it long before it is due. Iteration 1's timer is deliberately short and + // iteration 2's is long, so a leak shows up as an early completion. + const staleTimerSeconds = 0.05; + const iteration2TimerSeconds = 1.5; + let iteration2TimerFired = false; + + const orchestrator: TOrchestrator = async function* (ctx: OrchestrationContext, input: number): any { + if (input === 1) { + // Created but never awaited, then abandoned by continue-as-new. + ctx.createTimer(staleTimerSeconds); + ctx.continueAsNew(2, false); + } else { + yield ctx.createTimer(iteration2TimerSeconds); + iteration2TimerFired = true; + return "done"; + } + }; + + worker.addOrchestrator(orchestrator); + await worker.start(); + + const startedAt = Date.now(); + const id = await client.scheduleNewOrchestration(orchestrator, 1); + const state = await client.waitForOrchestrationCompletion(id, true, 10); + const elapsedMs = Date.now() - startedAt; + + expect(state).toBeDefined(); + expect(state?.runtimeStatus).toEqual(OrchestrationStatus.COMPLETED); + expect(state?.serializedOutput).toEqual(JSON.stringify("done")); + expect(iteration2TimerFired).toBe(true); + + // Without the cancellation, the stale 50ms timer completes iteration 2's timer + // almost immediately. The margin is deliberately wide to stay reliable in CI. + expect(elapsedMs).toBeGreaterThan(1000); + }); });