fix: prevent port conflicts when launching multiple installs concurrently (#94)

* fix: prevent port conflicts when launching multiple installs concurrently

Port reservation and instance detection previously only took effect after
full boot (up to 2 minutes), creating a race window where concurrent
launches could claim the same port.

Main process:
- Add in-memory _pendingPorts map to track ports during boot
- Check pending reservations in port conflict detection
- Pass reserved ports as exclusions to findAvailablePort
- Add TOCTOU re-check before reserving to close async gap
- Broadcast instance-launching/instance-launch-failed events

Renderer:
- Track launchingInstances in sessionStore via new IPC events
- Update useLocalInstanceGuard to warn about booting instances

Tests: 18 new tests across process.test.ts, sessionStore.test.ts,
and useLocalInstanceGuard.test.ts

Amp-Thread-ID: https://ampcode.com/threads/T-019caeaa-8ead-722d-942d-5aee9307865e
Co-authored-by: Amp <amp@ampcode.com>

* fix: retry rename on EPERM/EACCES in safe file writes (Windows)

On Windows, antivirus or file indexer can briefly lock files after a
write, causing EPERM on the atomic rename step. Add retry logic (up to
3 attempts with backoff) to both sync and async writeFileSafe variants.
Clean up leftover .tmp files on permanent failure.

Amp-Thread-ID: https://ampcode.com/threads/T-019caeaa-8ead-722d-942d-5aee9307865e
Co-authored-by: Amp <amp@ampcode.com>

* fix: drop busy-wait retry from sync writeFileSafe, keep async only

The sync busy-wait blocks the event loop for up to 600ms. Instead, rely
on readFileSafe's .bak fallback for recovery. Add a future enhancement
note in DESIGN_PROCESS.md to convert sync callers to async.

Amp-Thread-ID: https://ampcode.com/threads/T-019caeaa-8ead-722d-942d-5aee9307865e
Co-authored-by: Amp <amp@ampcode.com>

---------

Co-authored-by: Amp <amp@ampcode.com>

Author: Kosinkadink

DESIGN_PROCESS.md

@@ -194,3 +194,21 @@ When reviewing code for modularity, check:
 5. **Does a renderer view contain hardcoded conditionals for specific action/source IDs?** Replace with a data-driven pattern (add a property to the schema).
 6. **Does a source repeat information already available from its own definition?** Have the framework (ipc.js) inject it.
 7. **Can a new source be added by only creating a file in `sources/` and registering it in `sources/index.js`?** If not, something is coupled.
+
+## Future Enhancements
+
+### Convert `writeFileSafe` callers to async
+
+The synchronous `writeFileSafe` in `lib/safe-file.ts` cannot safely retry on
+Windows `EPERM`/`EACCES` rename errors (retry would require a busy-wait that
+blocks the event loop). The async variant `writeFileSafeAsync` handles this
+with proper `setTimeout`-based retries.
+
+Callers that should be migrated to `writeFileSafeAsync`:
+- `src/main/settings.ts` — `set()` function
+- `src/main/lib/release-cache.ts` — `save()` function
+- `src/main/lib/models.ts` — `ensureModelPathsConfig()`
+- `src/main/lib/fetch.ts` — `fetchJSON()` cache write
+
+Currently the sync version relies on `readFileSafe`'s `.bak` fallback to
+recover from a failed rename, which is adequate but not ideal.

src/main/lib/ipc.ts

@@ -255,6 +255,15 @@ let _gpuPromise: Promise<GpuInfo | null> | null = null
 
 const _operationAborts = new Map<string, AbortController>()
 const _runningSessions = new Map<string, SessionInfo>()
+const _pendingPorts = new Map<number, string>() // port → installationName
+
+function _reservePort(port: number, installationName: string): void {
+  _pendingPorts.set(port, installationName)
+}
+
+function _releasePort(port: number): void {
+  _pendingPorts.delete(port)
+}
 
 function _broadcastToRenderer(channel: string, data: Record<string, unknown>): void {
   BrowserWindow.getAllWindows().forEach((win) => {
@@ -1400,41 +1409,83 @@ export function register(callbacks: RegisterCallbacks = {}): void {
         setPortArg(launchCmd as LaunchCmd, actionData.portOverride as number)
       }
 
-      const existingPids = await findPidsByPort(launchCmd.port!)
-      if (existingPids.length > 0) {
+      // Check for port conflicts: in-memory pending reservations, OS-level listeners, and disk locks
+      const pendingPortOwner = _pendingPorts.get(launchCmd.port!)
+      const existingPids = pendingPortOwner ? [] : await findPidsByPort(launchCmd.port!)
+      const portOccupied = !!pendingPortOwner || existingPids.length > 0
+
+      if (portOccupied) {
         const defaults = source.getDefaults ? source.getDefaults() : {}
         const portConflictMode = (inst.portConflict as string | undefined) || (defaults.portConflict as string | undefined) || 'auto'
         const userArgs = ((inst.launchArgs as string | undefined) || '').trim()
         const portIsExplicit = /(?:^|\s)--port\b/.test(userArgs)
 
+        const reservedPorts = new Set(_pendingPorts.keys())
         let nextPort: number | null = null
         try {
-          nextPort = await findAvailablePort('127.0.0.1', launchCmd.port! + 1, launchCmd.port! + 1000)
+          nextPort = await findAvailablePort('127.0.0.1', launchCmd.port! + 1, launchCmd.port! + 1000, reservedPorts)
         } catch {}
 
         if (portConflictMode === 'auto' && nextPort && !portIsExplicit) {
           sendProgress('launch', { percent: -1, status: i18n.t('launch.portBusyUsing', { old: launchCmd.port!, new: nextPort }) })
           setPortArg(launchCmd as LaunchCmd, nextPort)
         } else {
-          const lock = readPortLock(launchCmd.port!)
           let message: string
           let isComfy: boolean
-          if (lock) {
-            message = i18n.t('errors.portConflictLauncher', { port: launchCmd.port!, name: lock.installationName })
+          if (pendingPortOwner) {
+            message = i18n.t('errors.portConflictLauncher', { port: launchCmd.port!, name: pendingPortOwner })
             isComfy = true
           } else {
-            const info = await getProcessInfo(existingPids[0]!)
-            isComfy = looksLikeComfyUI(info)
-            const processDesc = info ? info.name : `PID ${existingPids[0]}`
-            message = isComfy
-              ? i18n.t('errors.portConflictComfy', { port: launchCmd.port!, process: processDesc })
-              : i18n.t('errors.portConflictOther', { port: launchCmd.port!, process: processDesc })
+            const lock = readPortLock(launchCmd.port!)
+            if (lock) {
+              message = i18n.t('errors.portConflictLauncher', { port: launchCmd.port!, name: lock.installationName })
+              isComfy = true
+            } else {
+              const info = await getProcessInfo(existingPids[0]!)
+              isComfy = looksLikeComfyUI(info)
+              const processDesc = info ? info.name : `PID ${existingPids[0]}`
+              message = isComfy
+                ? i18n.t('errors.portConflictComfy', { port: launchCmd.port!, process: processDesc })
+                : i18n.t('errors.portConflictOther', { port: launchCmd.port!, process: processDesc })
+            }
           }
           _operationAborts.delete(installationId)
           return { ok: false, message, portConflict: { port: launchCmd.port, pids: existingPids, isComfy, nextPort } }
         }
       }
 
+      // Synchronous re-check: another launch may have reserved this port while we
+      // were awaiting findPidsByPort / findAvailablePort above (TOCTOU gap).
+      const lateConflictOwner = _pendingPorts.get(launchCmd.port!)
+      if (lateConflictOwner) {
+        const defaults = source.getDefaults ? source.getDefaults() : {}
+        const portConflictMode = (inst.portConflict as string | undefined) || (defaults.portConflict as string | undefined) || 'auto'
+        const userArgs = ((inst.launchArgs as string | undefined) || '').trim()
+        const portIsExplicit = /(?:^|\s)--port\b/.test(userArgs)
+
+        const reservedPorts = new Set(_pendingPorts.keys())
+        let nextPort: number | null = null
+        try {
+          nextPort = await findAvailablePort('127.0.0.1', launchCmd.port! + 1, launchCmd.port! + 1000, reservedPorts)
+        } catch {}
+
+        if (portConflictMode === 'auto' && nextPort && !portIsExplicit) {
+          sendProgress('launch', { percent: -1, status: i18n.t('launch.portBusyUsing', { old: launchCmd.port!, new: nextPort }) })
+          setPortArg(launchCmd as LaunchCmd, nextPort)
+        } else {
+          _operationAborts.delete(installationId)
+          return {
+            ok: false,
+            message: i18n.t('errors.portConflictLauncher', { port: launchCmd.port!, name: lateConflictOwner }),
+            portConflict: { port: launchCmd.port, pids: [], isComfy: true, nextPort },
+          }
+        }
+      }
+
+      // Reserve port eagerly before spawning to prevent concurrent launches from claiming it
+      _reservePort(launchCmd.port!, inst.name)
+      _broadcastToRenderer('instance-launching', { installationId, installationName: inst.name })
+
       const sessionPath = createSessionPath()
       const launchEnv = { ...process.env, __COMFY_CLI_SESSION__: sessionPath }
       const sendOutput = (text: string): void => {
@@ -1517,9 +1568,12 @@ export function register(callbacks: RegisterCallbacks = {}): void {
           if (isPortConflict && portRetries < PORT_RETRY_MAX) {
             portRetries++
             try {
-              const retryPort = await findAvailablePort('127.0.0.1', launchCmd.port! + 1, launchCmd.port! + 1000)
+              const reservedPorts = new Set(_pendingPorts.keys())
+              const retryPort = await findAvailablePort('127.0.0.1', launchCmd.port! + 1, launchCmd.port! + 1000, reservedPorts)
               sendOutput(`\nPort ${launchCmd.port} in use, retrying on port ${retryPort}…\n`)
+              _releasePort(launchCmd.port!)
               setPortArg(launchCmd as LaunchCmd, retryPort)
+              _reservePort(launchCmd.port!, inst.name)
               return tryLaunch()
             } catch {}
           }
@@ -1530,12 +1584,16 @@ export function register(callbacks: RegisterCallbacks = {}): void {
 
       const launchResult = await tryLaunch()
       if (!launchResult.ok) {
+        _releasePort(launchCmd.port!)
         _operationAborts.delete(installationId)
+        _broadcastToRenderer('instance-launch-failed', { installationId })
         if (launchResult.cancelled) return { ok: false, cancelled: true }
         return { ok: false, message: launchResult.message }
       }
       let { proc } = launchResult
 
+      // Transition from pending reservation to confirmed session + port lock
+      _pendingPorts.delete(launchCmd.port!)
       _operationAborts.delete(installationId)
       const mode = (inst.launchMode as string | undefined) || 'window'
       _addSession(installationId, { proc, port: launchCmd.port!, mode, installationName: inst.name })

src/main/lib/process.test.ts

@@ -0,0 +1,56 @@
+import { describe, it, expect } from 'vitest'
+import { findAvailablePort } from './process'
+import net from 'net'
+
+describe('findAvailablePort', () => {
+  it('finds an available port in the given range', async () => {
+    const port = await findAvailablePort('127.0.0.1', 49200, 49300)
+
+    expect(port).toBeGreaterThanOrEqual(49200)
+    expect(port).toBeLessThanOrEqual(49300)
+  })
+
+  it('skips ports in the excludePorts set', async () => {
+    const firstPort = await findAvailablePort('127.0.0.1', 49200, 49300)
+
+    const excluded = new Set([firstPort])
+    const result = await findAvailablePort('127.0.0.1', firstPort, 49300, excluded)
+
+    expect(result).not.toBe(firstPort)
+    expect(result).toBeGreaterThanOrEqual(firstPort + 1)
+  })
+
+  it('skips multiple excluded ports', async () => {
+    const base = 49300
+    const excluded = new Set([base, base + 1, base + 2])
+    const result = await findAvailablePort('127.0.0.1', base, base + 100, excluded)
+
+    expect(excluded.has(result)).toBe(false)
+    expect(result).toBeGreaterThanOrEqual(base + 3)
+  })
+
+  it('rejects when all ports in range are excluded', async () => {
+    const base = 49400
+    const excluded = new Set([base, base + 1, base + 2])
+
+    await expect(
+      findAvailablePort('127.0.0.1', base, base + 2, excluded)
+    ).rejects.toThrow('No available ports found')
+  })
+
+  it('skips a port that is actually in use', async () => {
+    const base = 49500
+    // Bind a port to simulate it being in use
+    const server = net.createServer()
+    await new Promise<void>((resolve) => {
+      server.listen(base, '127.0.0.1', () => resolve())
+    })
+
+    try {
+      const result = await findAvailablePort('127.0.0.1', base, base + 100)
+      expect(result).toBeGreaterThan(base)
+    } finally {
+      await new Promise<void>((resolve) => server.close(() => resolve()))
+    }
+  })
+})

src/main/lib/process.ts

@@ -23,7 +23,7 @@ export interface LaunchCmd {
   port: number
 }
 
-interface PortLock {
+export interface PortLock {
   pid: number
   installationName: string
   timestamp: number
@@ -206,13 +206,17 @@ export function setPortArg(launchCmd: LaunchCmd, port: number): void {
   launchCmd.port = port
 }
 
-export function findAvailablePort(host: string, startPort: number, endPort: number): Promise<number> {
+export function findAvailablePort(host: string, startPort: number, endPort: number, excludePorts?: ReadonlySet<number>): Promise<number> {
   return new Promise((resolve, reject) => {
     function tryPort(port: number): void {
       if (port > endPort) {
         reject(new Error(`No available ports found between ${startPort} and ${endPort}`))
         return
       }
+      if (excludePorts && excludePorts.has(port)) {
+        tryPort(port + 1)
+        return
+      }
       const server = net.createServer()
       server.listen(port, host, () => {
         server.once("close", () => resolve(port))

src/main/lib/safe-file.ts

@@ -28,7 +28,12 @@ export function writeFileSafe(filePath: string, data: string, backup: boolean =
   if (backup) {
     try { fs.copyFileSync(filePath, bakPath) } catch {}
   }
-  fs.renameSync(tmpPath, filePath)
+  try {
+    fs.renameSync(tmpPath, filePath)
+  } catch (err) {
+    try { fs.unlinkSync(tmpPath) } catch {}
+    throw err
+  }
 }
 
 /**
@@ -66,7 +71,24 @@ export async function writeFileSafeAsync(filePath: string, data: string, backup:
   if (backup) {
     try { await fs.promises.copyFile(filePath, bakPath) } catch {}
   }
-  await fs.promises.rename(tmpPath, filePath)
+  // On Windows, antivirus or indexer may briefly lock the file after a write,
+  // causing EPERM on rename. Retry a few times with a short delay.
+  const RENAME_RETRIES = 3
+  const RENAME_DELAY_MS = 100
+  for (let attempt = 0; ; attempt++) {
+    try {
+      await fs.promises.rename(tmpPath, filePath)
+      return
+    } catch (err) {
+      const code = (err as NodeJS.ErrnoException).code
+      if ((code === 'EPERM' || code === 'EACCES') && attempt < RENAME_RETRIES) {
+        await new Promise((r) => setTimeout(r, RENAME_DELAY_MS * (attempt + 1)))
+        continue
+      }
+      try { await fs.promises.unlink(tmpPath) } catch {}
+      throw err
+    }
+  }
 }
 
 export async function readFileSafeAsync(filePath: string): Promise<string | null> {

src/preload/index.ts

@@ -106,6 +106,16 @@ const api: ElectronApi = {
     ipcRenderer.on('comfy-exited', handler)
     return () => ipcRenderer.removeListener('comfy-exited', handler)
   },
+  onInstanceLaunching: (callback) => {
+    const handler = (_event: IpcRendererEvent, data: unknown) => callback(data as Parameters<typeof callback>[0])
+    ipcRenderer.on('instance-launching', handler)
+    return () => ipcRenderer.removeListener('instance-launching', handler)
+  },
+  onInstanceLaunchFailed: (callback) => {
+    const handler = (_event: IpcRendererEvent, data: unknown) => callback(data as Parameters<typeof callback>[0])
+    ipcRenderer.on('instance-launch-failed', handler)
+    return () => ipcRenderer.removeListener('instance-launch-failed', handler)
+  },
   onInstanceStarted: (callback) => {
     const handler = (_event: IpcRendererEvent, data: unknown) => callback(data as Parameters<typeof callback>[0])
     ipcRenderer.on('instance-started', handler)