From b4388c0ae8d5a198472c0adf1beb1235ad779765 Mon Sep 17 00:00:00 2001 From: Gabi Villalonga Simon Date: Fri, 21 Aug 2026 12:13:27 -0500 Subject: [PATCH] containers: Make sure state can't go out of sync in the container class with ctx.container --- .changeset/keep-start-state-synchronized.md | 5 ++ src/lib/container.ts | 52 ++++++++++++++++++++- src/tests/container.test.ts | 51 ++++++++++++++++++++ 3 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 .changeset/keep-start-state-synchronized.md diff --git a/.changeset/keep-start-state-synchronized.md b/.changeset/keep-start-state-synchronized.md new file mode 100644 index 0000000..dfc3299 --- /dev/null +++ b/.changeset/keep-start-state-synchronized.md @@ -0,0 +1,5 @@ +--- +'@cloudflare/containers': patch +--- + +Prevent an alarm racing with container startup from reporting the running container as stopped or removing its lifecycle alarm. `getState()` now also repairs stale stopped state when the runtime reports that the container is running. diff --git a/src/lib/container.ts b/src/lib/container.ts index b10439d..7fe308c 100644 --- a/src/lib/container.ts +++ b/src/lib/container.ts @@ -596,7 +596,37 @@ export class Container extends DurableObject { * @returns Promise */ async getState(): Promise { - return { ...(await this.state.getState()) }; + let state = await this.state.getState(); + + // The underlying container capability + // is what says if we are really running. + // This is a weird spot, we should not get here, + // however if we have a bug on state management still + // it's better to be defensive here. + // `stopped_with_code` is an edge case: it means the recorded exit belongs + // to a previous process and a new process has already started. Trust the + // runtime and repair the state, but do not call onStart again because the + // startup path owns that hook. + if ( + this.container.running && + (state.status === 'stopped' || state.status === 'stopped_with_code') + ) { + // Runtime state is authoritative here. Replacing `stopped_with_code` + // intentionally drops an exit code from a previous process because a + // newer process is already running; retaining it would report that old + // terminal event as the current process state. + await this.state.setRunning(); + + // Startup owns its monitor until startInFlight settles. Attaching another + // callback here could clear that monitor before startup handles its result. + if (this.startInFlight === undefined) { + this.monitor ??= this.container.monitor(); + this.setupMonitorCallbacks(); + } + state = await this.state.getState(); + } + + return { ...state }; } // ==================================== @@ -782,7 +812,10 @@ export class Container extends DurableObject { * @returns A promise that resolves when the container start command has been issued * @throws Error if no container context is available or if all start attempts fail */ - public async start(startOptions?: ContainerStartConfigOptions, waitOptions?: WaitOptions) { + public async start( + startOptions?: ContainerStartConfigOptions, + waitOptions?: WaitOptions + ): Promise { const portToCheck = waitOptions?.portToCheck ?? this.defaultPort ?? @@ -2037,6 +2070,17 @@ export class Container extends DurableObject { if (!this.container.running) { await this.syncPendingStoppedEvents(); + if (this.startInFlight !== undefined) { + await this.scheduleNextAlarm(); + return; + } + + // A concurrent start may have completed while synchronising. Its alarm + // must remain scheduled so the running container keeps lifecycle checks. + if (this.container.running) { + return; + } + if (resultForMinTime.length == 0) { await this.ctx.storage.deleteAlarm(); } else { @@ -2082,6 +2126,10 @@ export class Container extends DurableObject { // synchronises container state with the container source of truth to process events private async syncPendingStoppedEvents() { + if (this.startInFlight !== undefined) { + return; + } + const state = await this.state.getState(); if (!this.container.running && (state.status === 'healthy' || state.status === 'running')) { await this.callOnStop({ exitCode: 0, reason: 'exit' }, state); diff --git a/src/tests/container.test.ts b/src/tests/container.test.ts index c2e603c..2ddcdab 100644 --- a/src/tests/container.test.ts +++ b/src/tests/container.test.ts @@ -9,6 +9,52 @@ describe('Container', () => { expect(container.sleepAfter).toBe('10m'); }); + test('getState should repair stale stopped state for a running container', async ({ + mockCtx, + container, + }) => { + let resolveMonitor: () => void = () => undefined; + mockCtx.container.monitor.mockReturnValue( + new Promise(resolve => { + resolveMonitor = resolve; + }) + ); + mockCtx.storage.get.mockResolvedValue({ status: 'stopped', lastChange: Date.now() }); + mockCtx.container.running = true; + + await expect(container.getState()).resolves.toEqual( + expect.objectContaining({ status: 'running' }) + ); + expect(mockCtx.storage.put).toHaveBeenCalledWith( + '__CF_CONTAINER_STATE', + expect.objectContaining({ status: 'running' }) + ); + expect(mockCtx.container.monitor).toHaveBeenCalledOnce(); + + mockCtx.container.running = false; + resolveMonitor(); + await vi.waitFor(() => { + expect(mockCtx.storage.put).toHaveBeenCalledWith( + '__CF_CONTAINER_STATE', + expect.objectContaining({ status: 'stopped_with_code', exitCode: 0 }) + ); + }); + }); + + test('getState should not wait for an in-flight start', async ({ mockCtx, container }) => { + mockCtx.storage.get.mockResolvedValue({ status: 'stopped', lastChange: Date.now() }); + mockCtx.container.running = true; + const containerInternals = container as unknown as { + startInFlight: Promise; + }; + containerInternals.startInFlight = new Promise(() => undefined); + + await expect(container.getState()).resolves.toEqual( + expect.objectContaining({ status: 'running' }) + ); + expect(mockCtx.container.monitor).not.toHaveBeenCalled(); + }); + test('should use configured constructor startup options', async ({ mockCtx }) => { const container = new Container( mockCtx as never, @@ -245,10 +291,15 @@ describe('Container', () => { expect(mockCtx.container.start).not.toHaveBeenCalled(); await container.alarm(); + expect(mockCtx.container.start).not.toHaveBeenCalled(); + expect(onStopSpy).not.toHaveBeenCalled(); + expect(mockCtx.storage.deleteAlarm).not.toHaveBeenCalled(); + resumeStart(); await startPromise; expect(onStopSpy).not.toHaveBeenCalled(); + expect(mockCtx.storage.deleteAlarm).not.toHaveBeenCalled(); await expect(container.getState()).resolves.toEqual( expect.objectContaining({ status: 'running' }) );