Skip to content

[code-review] IsProcessAlive treats signal-0 EPERM as dead on Unix - asymmetric with the Windows ACCESS_DENIED->alive fix (#1723/#1535); false-death deletes PID files and forks a second daemon or triggers concurrent crash recovery (#1490 same lesion, Unix side) - one-line errors.Is fix #2190

Description

@topcheer

文件和行号

internal/util/process.go L26-28(IsProcessAlive)

问题:Unix 侧 signal-0 的 EPERM 被判为"已死"——与 Windows 侧 ACCESS_DENIED→alive 的修复不对称,误判死可致双 daemon/并发恢复

证据链(sa-151 初审中 → sa-152 独立复核确认中低):

  1. process.go:26-28:if proc.Signal(syscall.Signal(0)) != nil { return false }——Unix kill(pid,0) 的 EPERM 语义 = 进程存在但无信号权限(POSIX)。当前把 EPERM 与 ESRCH 同等判死
  2. 平台不对称坐实:Windows 孪生 process_windows_helper.go:27-38([business-logic] #1535 fixed the Unix EPERM case but the Windows helper returns true for ALL OpenProcess errors - a dead PID gets ERROR_INVALID_PARAMETER(87), not ACCESS_DENIED(5), so a crashed daemon is judged alive and EnsureDaemonSlot is permanently wedged; the existing TestIsProcessAlive_NonExistent would go red on Windows, proving the Verified claim never ran Windows tests #1723/[code-review] both platform aliveness checks treat permission errors as death: unix Signal(0) returns EPERM for a live root-owned daemon and windows OpenProcess returns ERROR_ACCESS_DENIED for elevated processes - CheckExistingDaemon deletes the PID file either way and a second daemon gets forked; TerminalFollowDisplay.OnError never clears roundBuf though the bridge-side of the same leak was already fixed in #552-D; Windows daemon is the repo's only long-lived child missing CREATE_NO_WINDOW (dies on Ctrl+C); darwin argv-parse failure falls back to the pre-#31 whole-blob-with-ENV match #1535)专门把 ACCESS_DENIED 判 alive,注释记载"误判死会删 PID 文件并 fork 第二个 daemon",仅 ERROR_INVALID_PARAMETER 判死,"when in doubt, alive"
  3. 调用点后果(均读源码核实):
  4. 可达性:需跨用户/提权分裂(sudo daemon + 非 root 检查、或 root 拥有的状态目录被非 root 读)——非常规但真实;EPERM 下 isZombieUnix 读不了他户 /proc 返回 false(保守判活),与新语义自洽

触发场景

root/sudo 启动 ggcode daemon → 非 root 用户的 ggcode 调 IsProcessAlive → EPERM → 判死 → 删 PID 文件 + fork 第二实例;或 run journal 被误判 crash 走并发恢复。

预期行为 vs 实际行为

  • 预期:EPERM ⟹ 存在(与 Windows "when in doubt, alive" 对齐)
  • 实际:EPERM ⟹ 判死

修复建议(一行)

if err := proc.Signal(syscall.Signal(0)); err != nil && !errors.Is(err, syscall.EPERM) {
    return false
}

严重程度

medium-low(三先例背书应修:#1490/#1535/#1723 均为同病灶 Windows 侧事故/修复;Unix 侧触发罕见但后果是双实例/并发恢复竞争)

(R72 轮:sa-151 初审 → sa-152 独立复核【确认中低——降半级理由:Unix 跨用户场景罕见】。同轮 P2(symlink 回退删文件)被复核证伪为不可达(debug.go:882 前置 Remove 断危害链)降为加固项不立案;P3(shell 探测 sync.Once 永久缓存失败)低不立案。全程只读。查重:#1723/#1535/#1405 已关——本条是 Unix 侧对称缺口新角度)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions