Skip to content

fix(electron): drop the PID-file instance lock that could brick startup - #247

Merged
EtienneLescot merged 2 commits into
mainfrom
fix/windows-single-instance-lock
Aug 4, 2026
Merged

fix(electron): drop the PID-file instance lock that could brick startup#247
EtienneLescot merged 2 commits into
mainfrom
fix/windows-single-instance-lock

Conversation

@EtienneLescot

@EtienneLescot EtienneLescot commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Symptom

Install 1.8.0 on Windows, double-click OpenScreen: nothing. No window, no error dialog, no entry in the Windows event log. The process exits with code 0 in ~1.1 s, before printing its first startup line.

Root cause

The app took two single-instance locks. Electron's own, and a home-made one in electron/singleInstanceLock.ts: a directory in os.tmpdir() holding the owner's PID.

That lock is only ever removed by its owner, on will-quit. Kill the app, crash it, or lose it to a reboot and the PID file survives. The staleness check then asks a question that stops being meaningful the moment the OS recycles the number:

const existingPid = readLockPid(lockDir);
if (existingPid && isProcessRunning(existingPid)) {
    return null;   // "still running" -> the app quits
}

isProcessRunning only asks is some process alive with this PID, never is it us. Windows recycles PIDs freely, so sooner or later the number belongs to an unrelated program — and from then on OpenScreen is permanently unlaunchable, silently, until the user finds and deletes a directory in %TEMP% they have no reason to know exists.

Observed on a 1.8.0 install: the lock held PID 21220, which by then belonged to another Electron app running on the machine.

> Start-Process 'C:\Program Files\Openscreen\Openscreen.exe' -PassThru -Wait
exit=0 ms=1136        # stale lock present
                      # after deleting the lock dir: window opens normally

Fix

Delete the home-made lock and keep app.requestSingleInstanceLock(), which already does the job and does it correctly:

  • held by the OS, so it is released even when the process dies badly;
  • keyed on the userData path, not on a PID that can be recycled;
  • already wired to second-instance, so the "focus the existing window" behaviour is unchanged.

One behaviour does change: the deleted lock keyed on os.tmpdir() + the user id, so it was machine-wide; requestSingleInstanceLock() keys on userData. A dev build and the installed Openscreen resolve different userData paths and can now run side by side. Two instances of the same build still cannot — which is the guarantee that matters for end users.

The CLI carve-out (cliCommand ? false : …, so openscreen export/record can run while the GUI is open) is preserved.

Net on the code: +1 / −162. A second commit syncs the four docs and the one test comment that described the deleted lock.

Verification

Built with the poisoned lock still on disk (a pid file naming a live, unrelated process):

before after
startup logs none — quits at the guard RECORDINGS_DIR, User Data Path, Global shortcut registered
window none renderer up

Second launch still quits instead of opening a duplicate, so the single-instance behaviour itself is intact.

Rebased onto main (c1862364). Both typechecks, lint, the docs check and vitest --run electron/ (31 files, 368 passed) are green locally — the CliCaptionsRunner.tsx type errors and the webm-seek-index failures seen on the previous base were fixed on main in the meantime.

For users already stuck on 1.8.0

Deleting %TEMP%\openscreen-single-instance-<user>.lock unblocks the app immediately. After this change the file is never read again.

Summary by CodeRabbit

  • Refactor

    • Removed custom single-instance lock implementation and replaced it with Electron's native mechanism for managing application instances.
  • Documentation

    • Updated development setup instructions, architecture documentation, performance benchmarks, and testing checklists to reflect simplified single-instance lock behavior and improved development build management.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Electron single-instance handling now uses app.requestSingleInstanceLock() for GUI launches. The stable lock module and tests were removed. Documentation and E2E comments now describe userData-scoped locks and process-based cleanup.

Changes

Single-instance lock migration

Layer / File(s) Summary
Electron lock integration
electron/main.ts, electron/singleInstanceLock.ts, electron/singleInstanceLock.test.ts
GUI startup now checks Electron’s lock directly. CLI mode still bypasses the lock. Stable lock acquisition, release handling, implementation, and tests were removed.
Lock documentation and test guidance
AGENTS.md, technical-documentation/..., tests/e2e/gif-export.spec.ts
Documentation now describes locks keyed by userData, separate development and installed-app paths, OS-managed release, and process-based cleanup. E2E comments describe --user-data-dir isolation.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description explains the symptom, root cause, fix, and verification, but it omits the required template sections and classification checkboxes. Add the required Summary, Related issue, Type of change, Release impact, Desktop impact, Screenshots/video, and Testing sections, with applicable checkboxes completed.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies removal of the faulty PID-file instance lock and the startup failure it could cause.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/windows-single-instance-lock

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.
@EtienneLescot
EtienneLescot force-pushed the fix/windows-single-instance-lock branch from de212f2 to 5fff1b3 Compare August 4, 2026 12:37
@EtienneLescot
EtienneLescot merged commit e271c43 into main Aug 4, 2026
17 checks passed
@EtienneLescot
EtienneLescot deleted the fix/windows-single-instance-lock branch August 4, 2026 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant