fix: Force-kill process on shutdown timeout to prevent hang (#730)

Author: charlesvien

apps/twig/src/main/index.ts

@@ -83,27 +83,45 @@ app.on("window-all-closed", () => {
 });
 
 app.on("before-quit", async (event) => {
-  const lifecycleService = container.get<AppLifecycleService>(
-    MAIN_TOKENS.AppLifecycleService,
-  );
+  let lifecycleService: AppLifecycleService;
+  try {
+    lifecycleService = container.get<AppLifecycleService>(
+      MAIN_TOKENS.AppLifecycleService,
+    );
+  } catch {
+    // Container already torn down (e.g. second quit during shutdown), let Electron quit
+    return;
+  }
 
   // If quitting to install an update, don't block and let the updater handle it
   // we already gracefully shutdown the app in the updates service when the update is ready
   if (lifecycleService.isQuittingForUpdate) {
     return;
   }
 
+  // If shutdown is already in progress, force-kill immediately
+  if (lifecycleService.isShuttingDown) {
+    lifecycleService.forceExit();
+  }
+
   event.preventDefault();
   await lifecycleService.shutdownAndExit();
 });
 
-const handleShutdownSignal = async (_signal: string) => {
+const handleShutdownSignal = async (signal: string) => {
+  log.info(`Received ${signal}, starting shutdown`);
   try {
     const lifecycleService = container.get<AppLifecycleService>(
       MAIN_TOKENS.AppLifecycleService,
     );
+    if (lifecycleService.isShuttingDown) {
+      log.warn(`${signal} received during shutdown, forcing exit`);
+      process.exit(1);
+    }
     await lifecycleService.shutdown();
-  } catch (_err) {}
+  } catch (_err) {
+    // Container torn down or shutdown failed
+  }
   process.exit(0);
 };
 

apps/twig/src/main/services/app-lifecycle/service.test.ts

@@ -1,17 +1,23 @@
-import { beforeEach, describe, expect, it, vi } from "vitest";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { AppLifecycleService } from "./service.js";
 
-const { mockApp, mockContainer, mockTrackAppEvent, mockShutdownPostHog } =
-  vi.hoisted(() => ({
-    mockApp: {
-      exit: vi.fn(),
-    },
-    mockContainer: {
-      unbindAll: vi.fn(() => Promise.resolve()),
-    },
-    mockTrackAppEvent: vi.fn(),
-    mockShutdownPostHog: vi.fn(() => Promise.resolve()),
-  }));
+const {
+  mockApp,
+  mockContainer,
+  mockTrackAppEvent,
+  mockShutdownPostHog,
+  mockProcessExit,
+} = vi.hoisted(() => ({
+  mockApp: {
+    exit: vi.fn(),
+  },
+  mockContainer: {
+    unbindAll: vi.fn(() => Promise.resolve()),
+  },
+  mockTrackAppEvent: vi.fn(),
+  mockShutdownPostHog: vi.fn(() => Promise.resolve()),
+  mockProcessExit: vi.fn() as unknown as (code?: number) => never,
+}));
 
 vi.mock("electron", () => ({
   app: mockApp,
@@ -45,12 +51,20 @@ vi.mock("../../../types/analytics.js", () => ({
 
 describe("AppLifecycleService", () => {
   let service: AppLifecycleService;
+  const originalProcessExit = process.exit;
 
   beforeEach(() => {
     vi.clearAllMocks();
+    vi.useFakeTimers();
+    process.exit = mockProcessExit;
     service = new AppLifecycleService();
   });
 
+  afterEach(() => {
+    vi.useRealTimers();
+    process.exit = originalProcessExit;
+  });
+
   describe("isQuittingForUpdate", () => {
     it("returns false by default", () => {
       expect(service.isQuittingForUpdate).toBe(false);
@@ -62,19 +76,38 @@ describe("AppLifecycleService", () => {
     });
   });
 
+  describe("isShuttingDown", () => {
+    it("returns false by default", () => {
+      expect(service.isShuttingDown).toBe(false);
+    });
+
+    it("returns true after shutdown is called", async () => {
+      const shutdownPromise = service.shutdown();
+      expect(service.isShuttingDown).toBe(true);
+      await vi.runAllTimersAsync();
+      await shutdownPromise;
+    });
+  });
+
   describe("shutdown", () => {
     it("unbinds all container services", async () => {
-      await service.shutdown();
+      const promise = service.shutdown();
+      await vi.runAllTimersAsync();
+      await promise;
       expect(mockContainer.unbindAll).toHaveBeenCalled();
     });
 
     it("tracks app quit event", async () => {
-      await service.shutdown();
+      const promise = service.shutdown();
+      await vi.runAllTimersAsync();
+      await promise;
       expect(mockTrackAppEvent).toHaveBeenCalledWith("app_quit");
     });
 
     it("shuts down PostHog", async () => {
-      await service.shutdown();
+      const promise = service.shutdown();
+      await vi.runAllTimersAsync();
+      await promise;
       expect(mockShutdownPostHog).toHaveBeenCalled();
     });
 
@@ -91,7 +124,9 @@ describe("AppLifecycleService", () => {
         callOrder.push("shutdownPostHog");
       });
 
-      await service.shutdown();
+      const promise = service.shutdown();
+      await vi.runAllTimersAsync();
+      await promise;
 
       expect(callOrder).toEqual([
         "unbindAll",
@@ -103,7 +138,9 @@ describe("AppLifecycleService", () => {
     it("continues shutdown if container unbind fails", async () => {
       mockContainer.unbindAll.mockRejectedValue(new Error("unbind failed"));
 
-      await service.shutdown();
+      const promise = service.shutdown();
+      await vi.runAllTimersAsync();
+      await promise;
 
       expect(mockTrackAppEvent).toHaveBeenCalled();
       expect(mockShutdownPostHog).toHaveBeenCalled();
@@ -112,7 +149,28 @@ describe("AppLifecycleService", () => {
     it("continues shutdown if PostHog shutdown fails", async () => {
       mockShutdownPostHog.mockRejectedValue(new Error("posthog failed"));
 
-      await expect(service.shutdown()).resolves.toBeUndefined();
+      const promise = service.shutdown();
+      await vi.runAllTimersAsync();
+      await expect(promise).resolves.toBeUndefined();
+    });
+
+    it("force-exits on re-entrant shutdown call", async () => {
+      const promise = service.shutdown();
+      service.shutdown();
+      expect(mockProcessExit).toHaveBeenCalledWith(1);
+      await vi.runAllTimersAsync();
+      await promise;
+    });
+
+    it("force-exits when shutdown times out", async () => {
+      mockContainer.unbindAll.mockReturnValue(new Promise(() => {}));
+
+      const promise = service.shutdown();
+
+      await vi.advanceTimersByTimeAsync(3000);
+      await promise;
+
+      expect(mockProcessExit).toHaveBeenCalledWith(1);
     });
   });
 
@@ -127,14 +185,18 @@ describe("AppLifecycleService", () => {
         callOrder.push("exit");
       });
 
-      await service.shutdownAndExit();
+      const promise = service.shutdownAndExit();
+      await vi.runAllTimersAsync();
+      await promise;
 
       expect(callOrder[0]).toBe("unbindAll");
       expect(callOrder[callOrder.length - 1]).toBe("exit");
     });
 
     it("exits with code 0", async () => {
-      await service.shutdownAndExit();
+      const promise = service.shutdownAndExit();
+      await vi.runAllTimersAsync();
+      await promise;
       expect(mockApp.exit).toHaveBeenCalledWith(0);
     });
   });

apps/twig/src/main/services/app-lifecycle/service.ts

@@ -11,48 +11,72 @@ const log = logger.scope("app-lifecycle");
 @injectable()
 export class AppLifecycleService {
   private _isQuittingForUpdate = false;
+  private _isShuttingDown = false;
   private static readonly SHUTDOWN_TIMEOUT_MS = 3000;
 
   get isQuittingForUpdate(): boolean {
     return this._isQuittingForUpdate;
   }
 
+  get isShuttingDown(): boolean {
+    return this._isShuttingDown;
+  }
+
   setQuittingForUpdate(): void {
     this._isQuittingForUpdate = true;
   }
 
+  forceExit(): never {
+    log.warn("Force-killing process");
+    process.exit(1);
+  }
+
   async shutdown(): Promise<void> {
-    // Race shutdown against timeout to prevent app from hanging forever
+    if (this._isShuttingDown) {
+      log.warn("Shutdown already in progress, forcing exit");
+      this.forceExit();
+    }
+
+    this._isShuttingDown = true;
+
     const result = await withTimeout(
       this.doShutdown(),
       AppLifecycleService.SHUTDOWN_TIMEOUT_MS,
     );
 
     if (result.result === "timeout") {
-      log.warn("Shutdown timeout reached, proceeding anyway", {
+      log.warn("Shutdown timeout reached, forcing exit", {
         timeoutMs: AppLifecycleService.SHUTDOWN_TIMEOUT_MS,
       });
+      this.forceExit();
     }
   }
 
   private async doShutdown(): Promise<void> {
+    log.info("Shutdown started: unbinding container");
     try {
       await container.unbindAll();
+      log.info("Container unbound successfully");
     } catch (error) {
       log.error("Failed to unbind container", error);
     }
 
     trackAppEvent(ANALYTICS_EVENTS.APP_QUIT);
 
+    log.info("Shutting down PostHog");
     try {
       await shutdownPostHog();
+      log.info("PostHog shutdown complete");
     } catch (error) {
       log.error("Failed to shutdown PostHog", error);
     }
+
+    log.info("Graceful shutdown complete");
   }
 
   async shutdownAndExit(): Promise<void> {
     await this.shutdown();
+    log.info("Calling app.exit(0)");
     app.exit(0);
   }
 }