Summary
pidAliveProbe in internal/daemon/reaper.go:374-383 hand-rolls a liveness check
using p.Signal(syscall.Signal(0)). On Windows, os.Process.signal returns
syscall.EWINDOWS for any signal other than Kill
($GOROOT/src/os/exec_windows.go), so the probe reports false for every process
— including healthy, running ones.
Consequence: on Windows the watcher registry sweep
(Reaper.sweepWatchers -> watchreg.Sweep) classifies every live grafel watch
process as dead and drops its registry entry. The daemon then loses track of
watchers it is actually running.
Why it wasn't caught
internal/daemon is excluded from the always-on windows.yml tripwire, and the
full 3-OS matrix in .github/workflows/test.yml only runs on version tags and the
ci:full label — so this path is effectively untested on Windows in normal CI.
The repo already has the right primitive
internal/process provides a platform-correct implementation that is already used
elsewhere (internal/daemon/pidfile.go:188):
internal/process/isalive_windows.go:24
internal/process/isalive_unix.go:18
pidAliveProbe should delegate to process.IsAlive rather than reimplementing the
unix-only Signal(0) idiom.
Suggested fix
Replace the body of pidAliveProbe with a call to process.IsAlive, and add a
watchreg-sweep test that runs on Windows (using the existing portable
spawnLiveChild helper at internal/daemon/pidfile_test.go:141-162, which already
branches cmd /C ping -n 31 vs sleep 30) asserting a live watcher entry is
retained.
Provenance
Found during the independent adversarial review of the #5933 fix. It is
pre-existing and out of scope for that PR, but it sits in the same call path:
it is why a naive liveness-based test added in #5933 would fail on Windows.
Relevant to the v0.1.9 release gate, which requires green CI on macOS + Linux +
Windows.
Related
Summary
pidAliveProbeininternal/daemon/reaper.go:374-383hand-rolls a liveness checkusing
p.Signal(syscall.Signal(0)). On Windows,os.Process.signalreturnssyscall.EWINDOWSfor any signal other thanKill(
$GOROOT/src/os/exec_windows.go), so the probe reportsfalsefor every process— including healthy, running ones.
Consequence: on Windows the watcher registry sweep
(
Reaper.sweepWatchers->watchreg.Sweep) classifies every livegrafel watchprocess as dead and drops its registry entry. The daemon then loses track of
watchers it is actually running.
Why it wasn't caught
internal/daemonis excluded from the always-onwindows.ymltripwire, and thefull 3-OS matrix in
.github/workflows/test.ymlonly runs on version tags and theci:fulllabel — so this path is effectively untested on Windows in normal CI.The repo already has the right primitive
internal/processprovides a platform-correct implementation that is already usedelsewhere (
internal/daemon/pidfile.go:188):internal/process/isalive_windows.go:24internal/process/isalive_unix.go:18pidAliveProbeshould delegate toprocess.IsAliverather than reimplementing theunix-only
Signal(0)idiom.Suggested fix
Replace the body of
pidAliveProbewith a call toprocess.IsAlive, and add awatchreg-sweep test that runs on Windows (using the existing portable
spawnLiveChildhelper atinternal/daemon/pidfile_test.go:141-162, which alreadybranches
cmd /C ping -n 31vssleep 30) asserting a live watcher entry isretained.
Provenance
Found during the independent adversarial review of the #5933 fix. It is
pre-existing and out of scope for that PR, but it sits in the same call path:
it is why a naive liveness-based test added in #5933 would fail on Windows.
Relevant to the v0.1.9 release gate, which requires green CI on macOS + Linux +
Windows.
Related