From 31937ee80522229e7d178bccffe68015019aed3d Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Tue, 4 Aug 2026 03:50:58 +0200 Subject: [PATCH 1/2] fix(electron): drop the PID-file instance lock that could brick startup The app took two single-instance locks: Electron's own, and a home-made one, a directory in os.tmpdir() holding the owner's PID. A hard kill or a crash leaves that PID file behind; the staleness check then asks whether that PID is still alive, and once Windows recycles the number for an unrelated process the answer is yes, forever. The app then quits with exit code 0 before printing a single line: no window, no error dialog. Seen in the wild on 1.8.0: the lock held PID 21220, which by then belonged to another Electron app. Every launch exited in ~1.1s. app.requestSingleInstanceLock() already does this job, is released by the OS even when the process dies badly, and has no PID to confuse. The extra lock only added the failure mode, so it goes. Verified with the poisoned lock in place: the previous build quits before any startup log, this one boots (userData, global shortcut, renderer), and a second launch still quits instead of opening a duplicate. --- electron/main.ts | 7 +- electron/singleInstanceLock.test.ts | 52 -------------- electron/singleInstanceLock.ts | 104 ---------------------------- 3 files changed, 1 insertion(+), 162 deletions(-) delete mode 100644 electron/singleInstanceLock.test.ts delete mode 100644 electron/singleInstanceLock.ts diff --git a/electron/main.ts b/electron/main.ts index 7549121f3f..5c13884073 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -23,7 +23,6 @@ import { import { mainT, setMainLocale } from "./i18n"; import { getSelectedDesktopSource, registerIpcHandlers } from "./ipc/handlers"; import { installMainProcessErrorGuards } from "./main-process-errors"; -import { acquireStableInstanceLock } from "./singleInstanceLock"; import { registerSttIpc } from "./stt"; import { createCountdownOverlayWindow, @@ -134,9 +133,7 @@ function showMainWindow() { // CLI runs skip the single-instance lock so `openscreen export/record` works // while the GUI app is open (they share nothing but the recordings directory). -const stableInstanceLock = cliCommand ? null : acquireStableInstanceLock(); -const hasElectronSingleInstanceLock = cliCommand ? false : app.requestSingleInstanceLock(); -const hasSingleInstanceLock = Boolean(stableInstanceLock && hasElectronSingleInstanceLock); +const hasSingleInstanceLock = cliCommand ? false : app.requestSingleInstanceLock(); if (cliCommand) { runCli(cliCommand); @@ -145,7 +142,6 @@ if (cliCommand) { showMainWindow(); }); } else { - stableInstanceLock?.release(); app.quit(); } @@ -518,7 +514,6 @@ app.on("activate", () => { app.on("will-quit", () => { unregisterAllGlobalShortcuts(); - stableInstanceLock?.release(); }); const appReady = !cliCommand && hasSingleInstanceLock ? app.whenReady() : null; diff --git a/electron/singleInstanceLock.test.ts b/electron/singleInstanceLock.test.ts deleted file mode 100644 index 4df35c72b3..0000000000 --- a/electron/singleInstanceLock.test.ts +++ /dev/null @@ -1,52 +0,0 @@ -import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; -import { acquireStableInstanceLock } from "./singleInstanceLock"; - -const testDirs: string[] = []; - -function createTestLockDir() { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openscreen-lock-test-")); - testDirs.push(dir); - return path.join(dir, "app.lock"); -} - -afterEach(() => { - for (const dir of testDirs.splice(0)) { - fs.rmSync(dir, { recursive: true, force: true }); - } -}); - -describe("acquireStableInstanceLock", () => { - it("prevents a second lock while the owning process is still running", () => { - const lockDir = createTestLockDir(); - const firstLock = acquireStableInstanceLock({ lockDir }); - - expect(firstLock).not.toBeNull(); - expect(acquireStableInstanceLock({ lockDir })).toBeNull(); - - firstLock?.release(); - }); - - it("reclaims a stale lock when its process is gone", () => { - const lockDir = createTestLockDir(); - fs.mkdirSync(lockDir); - fs.writeFileSync(path.join(lockDir, "pid"), "99999999\n"); - - const lock = acquireStableInstanceLock({ lockDir }); - - expect(lock).not.toBeNull(); - expect(fs.readFileSync(path.join(lockDir, "pid"), "utf8")).toBe(`${process.pid}\n`); - - lock?.release(); - }); - - it("does not remove a fresh empty lock directory", () => { - const lockDir = createTestLockDir(); - fs.mkdirSync(lockDir); - - expect(acquireStableInstanceLock({ lockDir })).toBeNull(); - expect(fs.existsSync(lockDir)).toBe(true); - }); -}); diff --git a/electron/singleInstanceLock.ts b/electron/singleInstanceLock.ts deleted file mode 100644 index d1a0a01afc..0000000000 --- a/electron/singleInstanceLock.ts +++ /dev/null @@ -1,104 +0,0 @@ -import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; - -const LOCK_DIR_PREFIX = "openscreen-single-instance"; -const PID_FILE_NAME = "pid"; -const EMPTY_LOCK_STALE_MS = 30_000; - -export type StableInstanceLock = { - lockDir: string; - release: () => void; -}; - -type LockOptions = { - lockDir?: string; - pid?: number; - now?: () => number; -}; - -function isProcessRunning(pid: number): boolean { - try { - process.kill(pid, 0); - return true; - } catch (error) { - return (error as NodeJS.ErrnoException).code === "EPERM"; - } -} - -function readLockPid(lockDir: string): number | null { - try { - const rawPid = fs.readFileSync(path.join(lockDir, PID_FILE_NAME), "utf8").trim(); - const pid = Number(rawPid); - return Number.isInteger(pid) && pid > 0 ? pid : null; - } catch { - return null; - } -} - -function isEmptyLockStale(lockDir: string, now: () => number): boolean { - try { - const stat = fs.statSync(lockDir); - return now() - stat.mtimeMs > EMPTY_LOCK_STALE_MS; - } catch { - return false; - } -} - -function releaseLock(lockDir: string, pid: number) { - if (readLockPid(lockDir) !== pid) { - return; - } - fs.rmSync(lockDir, { recursive: true, force: true }); -} - -function getCurrentUserLockKey() { - if (typeof process.getuid === "function") { - return `uid-${process.getuid()}`; - } - - try { - const username = os.userInfo().username.replace(/[^a-zA-Z0-9._-]/g, "_"); - return username || "default"; - } catch { - return "default"; - } -} - -export function getStableInstanceLockDir() { - return path.join(os.tmpdir(), `${LOCK_DIR_PREFIX}-${getCurrentUserLockKey()}.lock`); -} - -export function acquireStableInstanceLock(options: LockOptions = {}): StableInstanceLock | null { - const lockDir = options.lockDir ?? getStableInstanceLockDir(); - const pid = options.pid ?? process.pid; - const now = options.now ?? Date.now; - - for (let attempt = 0; attempt < 2; attempt += 1) { - try { - fs.mkdirSync(lockDir, { mode: 0o700 }); - fs.writeFileSync(path.join(lockDir, PID_FILE_NAME), `${pid}\n`, { flag: "wx" }); - return { - lockDir, - release: () => releaseLock(lockDir, pid), - }; - } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (code !== "EEXIST") { - throw error; - } - - const existingPid = readLockPid(lockDir); - if (existingPid && isProcessRunning(existingPid)) { - return null; - } - if (!existingPid && !isEmptyLockStale(lockDir, now)) { - return null; - } - - fs.rmSync(lockDir, { recursive: true, force: true }); - } - } - - return null; -} From 5fff1b309b3f32060db9177864fdd86b56cd20df Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Tue, 4 Aug 2026 14:34:04 +0200 Subject: [PATCH 2/2] docs: sync the single-instance notes with the Electron-only lock --- AGENTS.md | 2 +- technical-documentation/architecture/export-pipeline.md | 8 +++++--- .../engineering/rendering-performance.md | 2 +- technical-documentation/testing/manual-e2e-checklist.md | 2 +- tests/e2e/gif-export.spec.ts | 7 +++---- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8864fdf576..bfa79b4984 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,7 +50,7 @@ Unit/browser tests can't exercise real capture (native screen recording, a physi **Launch the app** - Normal: `npm run dev` — Vite serves the renderer and `vite-plugin-electron` opens the Electron window. The main process logs `Global shortcut registered: CommandOrControl+Shift+O` when ready (Ctrl/Cmd+Shift+O toggles the HUD). -- The app is single-instance: a lock dir at `%TEMP%/openscreen-single-instance-.lock` (macOS: `$TMPDIR`). If a stale Electron process holds it, a new launch quits silently (exit 0, no window). Kill leftover `electron` processes and delete that lock dir before relaunching. +- The app is single-instance through `app.requestSingleInstanceLock()`, which keys on the `userData` path. If a leftover Electron process still holds it, a new launch quits silently (exit 0, no window) — kill leftover `electron` processes before relaunching. The lock is held by the OS and dies with the process, so there is nothing to clean up on disk. A dev build and the installed `Openscreen` resolve different `userData` paths and can run side by side. - **From a git worktree** (no `node_modules`/native binaries): junction/symlink `node_modules` from the main checkout (deps are usually identical — check `package-lock.json`), and copy the prebuilt native capture binaries from `electron/native/bin//` (gitignored — rebuilding needs the full VS/Xcode toolchain). Then `npm run dev` works normally. **Granting access** diff --git a/technical-documentation/architecture/export-pipeline.md b/technical-documentation/architecture/export-pipeline.md index e8e0b2067b..0068fdf4cc 100644 --- a/technical-documentation/architecture/export-pipeline.md +++ b/technical-documentation/architecture/export-pipeline.md @@ -144,9 +144,11 @@ Each cost hours and each produced a confident, wrong conclusion. against the new renderer. It read as "export IPC not registered" once and as "the bench flag does nothing" once. The bench now refuses to run against one. -9. **The installed app (`openscreen.exe`) holds the same single-instance - lock as the dev build.** A launch exits 0 and reports nothing — - silently. +9. **A second instance of the same build quits silently.** The lock keys + on the `userData` path, so another dev build already running makes a + launch exit 0 and report nothing. The installed app + (`openscreen.exe`) resolves a different `userData` path and does not + conflict. ## A truncated project file is unopenable, not partially readable diff --git a/technical-documentation/engineering/rendering-performance.md b/technical-documentation/engineering/rendering-performance.md index af5a7189d6..64d48f88ac 100644 --- a/technical-documentation/engineering/rendering-performance.md +++ b/technical-documentation/engineering/rendering-performance.md @@ -379,7 +379,7 @@ The first frames of a 4-second export cost 358/113/28/350 ms — 10.3 ms/frame o - Electron cannot transfer an ArrayBuffer renderer→main. The transfer list takes `MessagePort[]`; transferring a buffer silently drops the whole message ([electron#34905](https://github.com/electron/electron/issues/34905)) — it works renderer→renderer. - `Buffer.from(typedArray)` copies. Wrapping (`Buffer.from(buf.buffer, byteOffset, byteLength)`) measured +31 %. - A stale `dist-electron` bundle runs the *previous* main process against the new renderer. It read as "export IPC not registered" once and as "the bench flag does nothing" once. The bench now refuses to run against one. -- The installed app (`openscreen.exe`) holds the same single-instance lock as the dev build. A launch exits 0 and reports nothing — silently. +- A second instance of the same build quits silently: the lock keys on the `userData` path, so another dev build already running makes a launch exit 0 and report nothing. The installed app (`openscreen.exe`) resolves a different `userData` path and does not conflict. ## What the numbers mean diff --git a/technical-documentation/testing/manual-e2e-checklist.md b/technical-documentation/testing/manual-e2e-checklist.md index 80a47ed218..f0cce73693 100644 --- a/technical-documentation/testing/manual-e2e-checklist.md +++ b/technical-documentation/testing/manual-e2e-checklist.md @@ -7,7 +7,7 @@ Sections marked **v1.8.0** cover what this release changed: chat-driven editing ## How to run this 1. Drive the real Electron app with computer-use, not a browser shim. Start a dev build with `npm run dev`, or launch the packaged build under test. -2. The app is single-instance. If a stale process holds the lock, stop the leftover Electron/OpenScreen process and remove the per-user lock directory before relaunching; a second launch can exit successfully without opening a window. +2. The app is single-instance per `userData` path. If a leftover Electron/OpenScreen process still holds the lock, stop that process before relaunching; a second launch can exit successfully without opening a window. The lock is held by the OS and is released when the process dies, so there is nothing to delete on disk. 3. From a worktree, link or junction `node_modules` to the main checkout and provide the prebuilt native capture binaries for the platform before starting the dev build. 4. Grant computer-use access to the process name that actually owns the window: `electron.exe` or `Electron.app` for a dev build, and `Openscreen.exe` or `Openscreen.app` for a packaged build. Do not grant access only to the installed app name when testing a dev build. 5. Read [AGENTS.md](../../AGENTS.md) for the computer-use mechanics, screenshot permissions, tray interaction, and cleanup procedure. Read one check, perform it, observe the result, then continue; close each modal or popover with `Esc` before the next check. diff --git a/tests/e2e/gif-export.spec.ts b/tests/e2e/gif-export.spec.ts index 1ff2b74b54..887681270f 100644 --- a/tests/e2e/gif-export.spec.ts +++ b/tests/e2e/gif-export.spec.ts @@ -27,10 +27,9 @@ async function launchApp(userDataDir: string, tmpDir: string) { env: { ...process.env, ELECTRON_USER_DATA_DIR: userDataDir, - // `acquireStableInstanceLock` keys its lock directory off `os.tmpdir()` and - // the *user id* — not off userData — so a dev instance running from any - // worktree would otherwise make this launch quit instantly (exit code 0, no - // window, no stderr). Giving the app its own temp dir gives it its own lock. + // Keep this run's scratch files out of the shared temp dir so a dev instance + // cannot collide with them. The single-instance lock keys on userData, which + // `--user-data-dir` above already makes private to this launch. TMPDIR: tmpDir, TMP: tmpDir, TEMP: tmpDir,