fix(simulator): stop the emulator on Stop, and close its session on every stop path

src/frontend/components/_organisms/workspace-activity-bar/default.tsx

@@ -636,7 +636,42 @@ export const DefaultWorkspaceActivityBar = ({ zoom }: DefaultWorkspaceActivityBa
   const handleSimulatorControl = useCallback(async (): Promise<void> => {
     try {
       if (simulatorRunning) {
-        await debugSession.stopSession()
+        // Two things end here, in this order, and the emulator's end is not
+        // conditional on the debug session's.
+        //
+        // The debug session goes first: it is a CONSUMER of the emulator, so it
+        // has to let go of the transport before the thing on the other end
+        // disappears.
+        //
+        // The emulator goes second, from a `finally`, because stopping it is
+        // this button's job and nothing else's. `stopSession()` deliberately
+        // does not do it (see its docstring: a debug session is not the owner of
+        // the thing it talks to), so with this call missing "Stop" ended the
+        // debug session, logged "Simulator stopped." and left the avr8js loop
+        // running — re-scheduling itself and burning a core for the rest of the
+        // session, unreachable because the button had flipped back to "Start".
+        //
+        // Sequencing it after a plain `await` reintroduced the same leak on the
+        // error path: anything that rejects inside the teardown (today only a
+        // throwing `onDisconnected` subscriber, which is why nothing hits it
+        // yet) skipped straight to the catch below, which only logs. The
+        // emulator kept running, `simulatorRunning` stayed true, and every
+        // retry failed identically — worse than the original bug, because
+        // nothing settled at all. `finally` keeps the order and drops the
+        // condition.
+        try {
+          await debugSession.stopSession()
+        } finally {
+          await simulator.stop()
+        }
+
+        // The session this debug session was riding is gone, so the claim that it
+        // rides one goes with it. The drop handler cannot clear it on this path:
+        // `stopSession()` has already hidden the debugger, so the handler's
+        // `isDebuggerVisible` gate returns before it reaches the reset — leaving
+        // the ref stale-`true` for a session that no longer exists.
+        debugSessionRidesDeviceRef.current = false
+
         setSimulatorRunning(false)
         addLog({ id: crypto.randomUUID(), level: 'info', message: 'Simulator stopped.' })
       } else {
@@ -649,7 +684,7 @@ export const DefaultWorkspaceActivityBar = ({ zoom }: DefaultWorkspaceActivityBa
       pendingSimulatorDebugRef.current = false
       addLog({ id: crypto.randomUUID(), level: 'error', message: `Simulator control error: ${getErrorMessage(error)}` })
     }
-  }, [debugSession, simulatorRunning, addLog])
+  }, [debugSession, simulator, simulatorRunning, addLog])
 
   // ---------------------------------------------------------------------------
   // MD5 verification — runs after debug compilation for non-simulator

src/main/modules/ipc/__tests__/simulator-session.handler.test.ts

@@ -0,0 +1,183 @@
+/**
+ * The simulator's session lifecycle, in the main process.
+ *
+ * What is worth testing HERE is the ownership rule rather than the emulator
+ * itself: starting the emulator opens this target's session and stopping it
+ * closes it, every stop path routes through the one choke point, and the order
+ * is session-then-emulator so the debug client is dropped before the thing it
+ * talks to disappears.
+ *
+ * The paths that used to leak — a window reload and a start that threw — are the
+ * reason this file exists: both left a running emulator behind, and the reload
+ * also left main holding a session the reloaded renderer knew nothing about.
+ */
+
+import MainProcessBridge from '../main'
+
+jest.mock('electron', () => ({
+  app: { getPath: jest.fn(() => '/tmp'), quit: jest.fn() },
+  dialog: {},
+  nativeTheme: { shouldUseDarkColors: false, themeSource: 'system' },
+  shell: { openExternal: jest.fn() },
+}))
+
+jest.mock('@root/backend/editor/ethercat', () => ({ ESIService: jest.fn() }))
+jest.mock('@root/backend/editor/library-manager/desktop-catalog-transport', () => ({
+  createDesktopCatalogTransport: jest.fn(() => ({})),
+}))
+jest.mock('@root/backend/editor/utils/runtime-https-config', () => ({ getRuntimeHttpsOptions: jest.fn(() => ({})) }))
+jest.mock('@root/backend/shared/ethercat/esi-parser-main', () => ({ parseESIDeviceFull: jest.fn() }))
+jest.mock('@root/backend/shared/library/public-catalog-client', () => ({ listPublicLibraries: jest.fn() }))
+jest.mock('../../../../backend/editor/library-manager', () => ({
+  LibraryManagerModule: jest.fn(() => ({ loadEnabledArchives: jest.fn(() => ({ archives: [], missing: [] })) })),
+}))
+jest.mock('../../../../backend/editor/package-manager', () => ({ PackageManagerModule: jest.fn(() => ({})) }))
+jest.mock('../../../../backend/editor/services', () => ({
+  logger: { error: jest.fn(), info: jest.fn(), warn: jest.fn() },
+}))
+jest.mock('../../../../backend/editor/utils', () => ({ getOpenProjectPath: jest.fn(), getProjectPath: jest.fn() }))
+
+const simulatorModule = { loadAndRun: jest.fn(), stop: jest.fn(), isRunning: jest.fn(() => true) }
+jest.mock('../../../../backend/shared/simulator/simulator-module', () => ({
+  SimulatorModule: jest.fn(() => simulatorModule),
+}))
+
+// The handler reads the hex off disk before it starts anything. Stubbing the read
+// to SUCCEED is what lets a test put the throw where the leak actually was —
+// after `loadAndRun` has marked the emulator running. A failing read throws
+// before that and proves nothing about it.
+const readFile = jest.fn<Promise<string>, [string, string?]>()
+jest.mock('fs/promises', () => ({ readFile: (...args: [string, string?]) => readFile(...args) }))
+
+type Bridge = MainProcessBridge
+
+function createBridge(): Bridge {
+  return new MainProcessBridge({
+    ipcMain: {},
+    mainWindow: {
+      isDestroyed: jest.fn(() => false),
+      isMaximized: jest.fn(() => false),
+      webContents: { reload: jest.fn(), send: jest.fn() },
+    },
+    projectService: {},
+    store: { get: jest.fn(() => undefined) },
+    menuBuilder: {},
+    pouService: {},
+    compilerModule: {},
+    hardwareModule: { isSerialPortPresent: jest.fn(() => true) },
+  } as never)
+}
+
+/**
+ * Pretend the session manager holds a link of the given transport, and record
+ * whether it is closed. Returns the recorder for the closing side.
+ */
+function holdLink(bridge: Bridge, transport: 'simulator' | 'serial') {
+  const session = (bridge as unknown as { deviceSession: { getLink: () => unknown; close: () => void } }).deviceSession
+  jest.spyOn(session, 'getLink').mockReturnValue({ transport })
+  return jest.spyOn(session, 'close').mockImplementation(() => undefined)
+}
+
+beforeEach(() => {
+  simulatorModule.loadAndRun.mockReset()
+  simulatorModule.stop.mockReset()
+  simulatorModule.isRunning.mockReset().mockReturnValue(true)
+  readFile.mockReset().mockResolvedValue(':00000001FF')
+})
+
+afterEach(() => {
+  jest.restoreAllMocks()
+})
+
+describe('simulator:stop', () => {
+  it('closes the session and stops the emulator', async () => {
+    const bridge = createBridge()
+    const close = holdLink(bridge, 'simulator')
+
+    await expect(bridge.handleSimulatorStop({} as never)).resolves.toEqual({ success: true })
+
+    expect(close).toHaveBeenCalledTimes(1)
+    expect(simulatorModule.stop).toHaveBeenCalledTimes(1)
+  })
+
+  it('closes the session BEFORE stopping the emulator', async () => {
+    const bridge = createBridge()
+    const order: string[] = []
+    const session = (bridge as unknown as { deviceSession: { getLink: () => unknown; close: () => void } })
+      .deviceSession
+    jest.spyOn(session, 'getLink').mockReturnValue({ transport: 'simulator' })
+    jest.spyOn(session, 'close').mockImplementation(() => {
+      order.push('session-closed')
+    })
+    simulatorModule.stop.mockImplementation(() => order.push('emulator-stopped'))
+
+    await bridge.handleSimulatorStop({} as never)
+
+    expect(order).toEqual(['session-closed', 'emulator-stopped'])
+  })
+
+  it('leaves a non-simulator session alone', async () => {
+    const bridge = createBridge()
+    const close = holdLink(bridge, 'serial')
+
+    await bridge.handleSimulatorStop({} as never)
+
+    // A cabled board's link is not the emulator's to close, but the emulator
+    // still stops.
+    expect(close).not.toHaveBeenCalled()
+    expect(simulatorModule.stop).toHaveBeenCalledTimes(1)
+  })
+})
+
+describe('window:reload', () => {
+  it('takes the session down with the emulator', () => {
+    const bridge = createBridge()
+    const close = holdLink(bridge, 'simulator')
+
+    bridge.handleWindowReload()
+
+    // The reload resets the renderer's store to 'disconnected'; a session left
+    // open here would be one only the main process still believes in.
+    expect(close).toHaveBeenCalledTimes(1)
+    expect(simulatorModule.stop).toHaveBeenCalledTimes(1)
+  })
+})
+
+describe('simulator:load-firmware', () => {
+  it('stops the emulator and closes the session when the start throws AFTER it is running', async () => {
+    const bridge = createBridge()
+    const close = holdLink(bridge, 'simulator')
+
+    // The leak this covers: `loadAndRun` marks the emulator running before it
+    // finishes wiring, so a throw from that point on left one running with no
+    // session behind it. The read must succeed for the throw to land there —
+    // throwing on the read instead would exercise a path where the emulator was
+    // never started, and would stay green even if this leak came back.
+    simulatorModule.loadAndRun.mockImplementation(() => {
+      throw new Error('avr8js refused the hex')
+    })
+
+    const result = await bridge.handleSimulatorLoadFirmware({} as never, '/tmp/simulator.hex')
+
+    expect(readFile).toHaveBeenCalledTimes(1)
+    expect(simulatorModule.loadAndRun).toHaveBeenCalledTimes(1)
+    expect(result).toEqual({ success: false, error: 'avr8js refused the hex' })
+    expect(simulatorModule.stop).toHaveBeenCalledTimes(1)
+    expect(close).toHaveBeenCalledTimes(1)
+  })
+
+  it('stops the emulator and closes the session when the read fails before it starts', async () => {
+    const bridge = createBridge()
+    const close = holdLink(bridge, 'simulator')
+    readFile.mockRejectedValue(new Error('ENOENT'))
+
+    const result = await bridge.handleSimulatorLoadFirmware({} as never, '/nonexistent/simulator.hex')
+
+    // Nothing was started, so there is nothing to leak — but the cleanup still
+    // has to be harmless, because the catch cannot tell the two apart.
+    expect(simulatorModule.loadAndRun).not.toHaveBeenCalled()
+    expect(result.success).toBe(false)
+    expect(simulatorModule.stop).toHaveBeenCalledTimes(1)
+    expect(close).toHaveBeenCalledTimes(1)
+  })
+})

src/main/modules/ipc/main.ts

@@ -1487,7 +1487,7 @@ class MainProcessBridge implements MainIpcModule {
     }
   }
   handleAppQuit = () => {
-    this.simulatorModule.stop()
+    this.stopSimulator()
     if (this.mainWindow) {
       this.mainWindow.destroy()
     }
@@ -1579,7 +1579,10 @@ class MainProcessBridge implements MainIpcModule {
     }
   }
   handleWindowReload = () => {
-    this.simulatorModule.stop()
+    // The reload wipes the renderer's store back to 'disconnected', so the
+    // session has to go with the emulator — otherwise main keeps holding an open
+    // simulator session that the reloaded UI has no idea about.
+    this.stopSimulator()
     this.mainWindow?.webContents.reload()
   }
   handleWindowRebuildMenu = () => {
@@ -2573,8 +2576,7 @@ class MainProcessBridge implements MainIpcModule {
   /** Stops the simulator and notifies the renderer so it can update UI state. */
   private stopSimulatorAndNotify(): void {
     if (this.simulatorModule.isRunning()) {
-      this.closeSimulatorSession()
-      this.simulatorModule.stop()
+      this.stopSimulator()
       this.mainWindow?.webContents.send('simulator:stopped')
     }
   }
@@ -2825,26 +2827,45 @@ class MainProcessBridge implements MainIpcModule {
         this.toDeviceLinkCandidates([{ connectionType: 'simulator', connectionParams: {} }]),
       )
       if (!opened.ok) {
-        this.simulatorModule.stop()
+        this.stopSimulator()
         const reason = opened.attempts.map((attempt) => attempt.error).join('; ')
         return { success: false, error: reason || 'The simulator did not answer its debug protocol' }
       }
       this.debuggerConnectionType = 'simulator'
       return { success: true }
     } catch (error) {
+      // A start that threw part-way still leaves state behind: `loadAndRun`
+      // marks the emulator running before it finishes wiring, so a throw after
+      // that point leaked a running emulator with no session — and no button to
+      // reach it, because the renderer never learned it had started. Web ends up
+      // in the right place through its worker, which cleans up and reports
+      // 'stopped' when `loadAndRun` throws; this is the editor's counterpart.
+      this.stopSimulator()
       return { success: false, error: getErrorMessage(error) }
     }
   }
 
   /**
    * Stop the emulator entirely — the simulator's Stop button means "stop the
-   * simulator", not "stop the program it is running". The session closes first so
-   * the client is dropped before the thing it talks to disappears.
+   * simulator", not "stop the program it is running".
    */
   handleSimulatorStop = (_event: IpcMainInvokeEvent): Promise<{ success: boolean }> => {
+    this.stopSimulator()
+    return Promise.resolve({ success: true })
+  }
+
+  /**
+   * The one way the emulator stops: session first, then the emulator, so the
+   * debug client is dropped before the thing it talks to disappears.
+   *
+   * Every stop path routes through here on purpose. The session is a consumer of
+   * the emulator, so an emulator that goes away while its session stays open
+   * leaves the renderer gated on a session whose target no longer exists — which
+   * a window reload and a failed start both used to do.
+   */
+  private stopSimulator(): void {
     this.closeSimulatorSession()
     this.simulatorModule.stop()
-    return Promise.resolve({ success: true })
   }
 
   /** Close the session if it is the simulator's. No-op for any other target. */