Skip to content

fix(daemon): treat an unopenable Windows process as alive - #577

Open
columbia11 wants to merge 7 commits into
Tencent:mainfrom
columbia11:fix/windows-pid-alive-access-denied
Open

columbia11 wants to merge 7 commits into
Tencent:mainfrom
columbia11:fix/windows-pid-alive-access-denied

Conversation

@columbia11

Copy link
Copy Markdown
Contributor

Problem

lockfile::pid_alive is documented to return true "if a process with that pid exists". The Unix branch does that and explicitly treats EPERM as alive. The Windows branch returns false for every OpenProcess(PROCESS_SYNCHRONIZE) failure, including ERROR_ACCESS_DENIED, which means the process exists but this caller may not open it.

Measured on Windows 11 from an unelevated PowerShell:

Process Exists OpenProcess(SYNCHRONIZE) pid_alive before
System (pid 4), wininit, lsass, csrss, services yes fails with 5 (ERROR_ACCESS_DENIED) false
explorer, MsMpEng yes opens true
pid 999999 no fails with 87 (ERROR_INVALID_PARAMETER) false

A process started from an elevated terminal is not openable from an unelevated one in the same way. pid_alive feeds daemon stop cleanup (ensure!(!pid_alive(expected.pid)) before removing daemon.json), daemon_info::read_valid, the post-update wait for the predecessor process, and update-leftover cleanup. In each of these, a live but unopenable process was treated as gone.

Fix

When OpenProcess fails, return GetLastError() == ERROR_ACCESS_DENIED, matching the Unix EPERM case. A nonexistent pid still fails with ERROR_INVALID_PARAMETER and stays false. pid 0 and u32::MAX are unchanged and still covered by the existing tests.

Tests

  • New existing_process_that_cannot_be_opened_is_alive: pid_alive(4) (the System process). Unelevated callers get ERROR_ACCESS_DENIED; elevated callers such as the CI runner can open it. It passes either way after the fix.
  • cargo test -p bsk --lib daemon::lockfile::tests on Windows 11, unelevated: 4 passed. With the old return false restored, the new test fails (3 passed, 1 failed).
  • rustfmt --check: clean.
  • I didn't reproduce an elevated daemon with an unelevated CLI end to end. The table above is the OS behavior this relies on.

I used Claude Code to find this, write the fix and test, and run the checks above.

🤖 Generated with Claude Code

columbia11 and others added 6 commits November 4, 2026 11:15
pid_alive is documented to return true when a process with that pid exists,
and the Unix branch treats EPERM as alive. On Windows every OpenProcess
failure returned false, including ERROR_ACCESS_DENIED, which means the
process exists but this caller may not open it (for example a SYSTEM or
elevated process). A pid with no process fails with ERROR_INVALID_PARAMETER.

Return true for ERROR_ACCESS_DENIED, matching the Unix EPERM case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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