Conversation
If Electron crashes or is force-killed, the Python backend process is left orphaned consuming GPU resources. This adds: - PID file tracking: write backend PID on start, remove on exit - Orphan cleanup: on next launch, check for stale PID file and kill the orphaned process before starting a new one Closes #38 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a PID-file based mechanism to clean up orphaned Python backend processes on Electron startup to prevent GPU-consuming backends from surviving non-graceful app exits (issue #38).
Changes:
- Add PID file creation/removal in
startBackend()and acleanupOrphanedBackend()helper that kills a previously-recorded PID. - Invoke orphan cleanup at the start of the backend startup sequence in the Electron main process.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/desktop/electron/main.ts | Calls orphan-backend cleanup before starting backend dependencies and process. |
| apps/desktop/electron/backend.ts | Implements PID file persistence and orphan process termination logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cleanupOrphanedBackend(); | ||
|
|
There was a problem hiding this comment.
Calling cleanupOrphanedBackend() unconditionally on startup can break multi-instance behavior: if the user launches a second instance while the first is still running, the PID file from the first instance will exist and the second instance will kill the first instance’s backend. If the app is intended to be single-instance, add app.requestSingleInstanceLock() early and exit the secondary instance; otherwise, include a parent-PID / instance identifier in the PID file and only clean up when the owning Electron process is no longer running.
| // Track the PID so orphaned processes can be cleaned up on next launch | ||
| if (child.pid != null) { | ||
| writePidFile(child.pid); | ||
| } | ||
| child.on("exit", () => removePidFile()); | ||
|
|
There was a problem hiding this comment.
New PID file behavior (write on spawn, remove on backend exit, and orphan cleanup) isn’t covered by the existing backend.test.ts suite. Adding unit tests here would help prevent regressions (mock electron.app.getPath, fs reads/writes, and process.kill) and ensure the PID file is written/removed and cleanup only kills when appropriate.
| const pid = parseInt(fs.readFileSync(pidFile, "utf-8").trim(), 10); | ||
| if (!Number.isNaN(pid)) { | ||
| // Check if process is still alive | ||
| process.kill(pid, 0); | ||
| // If we get here, process exists — kill it | ||
| console.log(`Killing orphaned backend process (PID ${pid})`); | ||
| process.kill(pid, "SIGTERM"); | ||
| } |
There was a problem hiding this comment.
The PID file stores only a numeric PID, and cleanupOrphanedBackend() will SIGTERM that PID if it exists. If the app is relaunched long after a crash, the OS may have reused that PID for an unrelated process, which could cause this code to terminate the wrong process. Consider storing additional identifying info (e.g., the previous Electron parent PID and verifying it is no longer alive, and/or validating the process command line/executable where possible) before issuing a kill, and only kill when you can confidently confirm it’s the backend you started.
| } catch { | ||
| // Process doesn't exist or we can't kill it — either way, clean up | ||
| } | ||
| removePidFile(); | ||
| }; |
There was a problem hiding this comment.
The blanket catch {} combined with unconditional removePidFile() means the PID file is deleted even when the backend process still exists but cannot be terminated (e.g., process.kill(pid, 0) / SIGTERM throws EPERM). That can leave a GPU-consuming orphan running while preventing subsequent launches from retrying cleanup. Consider inspecting the error code and only deleting the PID file when the process is confirmed gone (ESRCH) or after a successful termination (and log/report failures).
Summary
backend.pidfile when the Python backend starts, removes it on normal exitTest plan
backend.pidis created and removedCloses #38
🤖 Generated with Claude Code