containers: Make sure state can't go out of sync in the container class with ctx.container

.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.

src/lib/container.ts

@@ -596,7 +596,37 @@ export class Container<Env = Cloudflare.Env> extends DurableObject<Env> {
    * @returns Promise<State>
    */
   async getState(): Promise<State> {
-    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<Env = Cloudflare.Env> extends DurableObject<Env> {
    * @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<void> {
     const portToCheck =
       waitOptions?.portToCheck ??
       this.defaultPort ??
@@ -2037,6 +2070,17 @@ export class Container<Env = Cloudflare.Env> extends DurableObject<Env> {
     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<Env = Cloudflare.Env> extends DurableObject<Env> {
 
   // 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);

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<number>;
+    };
+    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' })
     );