Skip to content

fix(runfile,restart): Windows liveness wait-state check, creation-time recycle token, restart cd warning (#3079) - #3080

Merged
topcheer merged 2 commits into
mainfrom
fix-3079
Oct 2, 2026
Merged

topcheer merged 2 commits into
mainfrom
fix-3079

Conversation

@topcheer

@topcheer topcheer commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Fixes #3079 (V2 + V3; V1 was cross-adjudicated as not-real in the issue itself — findstr/PID substring with /FI precise filtering).

V2a — STILL_ACTIVE=259 exit-code ambiguity (medium)

Confirmed verbatim: return exitCode == 259. A process whose real exit code was exactly 259 was judged alive forever. Now processExists opens the handle with PROCESS_QUERY_LIMITED_INFORMATION|SYNCHRONIZE and uses WaitForSingleObject(handle, 0): WAIT_TIMEOUT = alive, WAIT_OBJECT_0 = terminated — unambiguous regardless of exit code. The 259 heuristic survives only as the degraded path when the wait right is unavailable (some protected processes grant query but not synchronize). #1290's ERROR_ACCESS_DENIED→alive semantics preserved unchanged.

V2b — PID-recycle fallback fully disabled on Windows (medium)

Confirmed: procStartTime returned "" on Windows, and runfile.go's gate (pf.StartTime != "" at read, pf.StartTime = procStartTime(...) at write) skipped on both sides — the #1624 ghost-port-file recycle cleanup degraded to plain liveness exactly as the code comment admits. procStartTime now returns the GetProcessTimes CreationTime FILETIME as an opaque per-process-start token. New port files record it at write and the reader comparison activates; legacy files with no stored token keep the pre-existing liveness rule (no behavior cliff).

V3 — restart script cd failure silent (low)

Confirmed: cd /d "%WORK_DIR%" 2>nul swallowed the failure when the work dir was deleted during the restart wait, and a relative %BINARY% then resolved from the wrong cwd with no explanation. The script now echoes a warning naming the failed work dir and the effective cwd (%CD%).

Verification

  • CGO_ENABLED=0 GOOS=windows go build + go vet on both packages (compiles the windows-tagged probes too)
  • darwin go build -tags goolm ./... + full internal/runfile / internal/restart test runs green
  • 3 new windows-tagged probes in zz_issue3079_test.go, executed by the Windows CI runner:
    • a child genuinely exited with code 259 is reported dead (the core V2a regression)
    • creation-time token: non-empty for self, stable across calls, plain decimal, empty for a dead pid
    • self alive via the wait-state check, pid 0 dead
  • The existing [code-review] runfile processExists Windows 侧 ACCESS_DENIED 误判进程死亡:#799 修复不对称致活实例 port 文件被删 #1290 windows suite (ACCESS_DENIED semantics) is untouched and must stay green on the runner

…e recycle token, restart cd warning (#3079)

V2a: processExists judged alive purely by GetExitCodeProcess==259 -
a process whose REAL exit code happened to be exactly 259 stayed
alive forever (STILL_ACTIVE ambiguity). Now opens the handle with
PROCESS_QUERY_LIMITED_INFORMATION|SYNCHRONIZE and uses
WaitForSingleObject(handle, 0): WAIT_TIMEOUT=alive,
WAIT_OBJECT_0=terminated, regardless of exit code. The legacy 259
heuristic remains only as the degraded path when the wait right is
unavailable; #1290 ACCESS_DENIED-alive semantics preserved.
V2b: procStartTime returned "" on Windows, fully disabling the
#1624 PID-recycle fallback (runfile.go skips the gate on an empty
token on either side). Now returns the GetProcessTimes CreationTime
FILETIME as the identity token - new port files record it and the
reader comparison activates; legacy files with no token keep the
pre-existing liveness rule.
V3: the restart batch script cd failure was silenced by 2>nul; it
now echoes a warning naming the failed work dir and the effective
cwd, so a deleted work dir no longer produces an unexplained launch
failure.

Verified: CGO_ENABLED=0 GOOS=windows go build + go vet on both
packages (covers the windows-tagged probes); darwin go build
-tags goolm ./... + runfile/restart package tests; 3 new
zz_issue3079 probes run on the Windows CI runner (exit-code-259
child reported dead, creation-time token non-empty/stable/plain and
empty for dead pids, self alive).

@topcheer topcheer left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

治理席 diff 级复核(#3079 让渡承接):

生产代码:通过

  • processExists:PROCESS_QUERY_LIMITED_INFORMATION|SYNCHRONIZE 双权限单次打开,wait-state 判定(WAIT_OBJECT_0=terminated/WAIT_TIMEOUT=alive)语义正确且与 internal/util/process_windows_helper.go #552-A 先例对齐;#1290 ACCESS_DENIED→true 语义保留;降级路径(wait 不可用回退 259 启发式)保守方向正确(误判活优于误判死)。顺带清除 var _ = os.Getpid 死引用,干净。
  • procStartTime:raw FILETIME 64 位 token——opaque 但满足字符串比较契约(同进程稳定、PID 回收后必变);写入侧 runfile.go L68-69 同函数调用,单点修复激活两侧,#1624 兜底在 Windows 复活。
  • restart cd 告警:errorlevel 语义正确,最小改动。

本地验证(git archive fix-3079):darwin 两包 build/vet/test 全绿;GOOS=windows 全仓库交叉编译+vet 绿(探针文件通过编译级验证)。

一个阻塞项:探针 CI 执行性缺口
zz_issue3079_test.go 头注释称 "Runs only on Windows CI",但①ci.yml Test job 仅 ubuntu-latest;②windows-process.yml 的 paths 过滤只含 internal/util/process*.go,本 PR 改的 internal/runfile/* 不触发该 workflow。当前探针在 CI 永远不会被执行——75 行回归测试零执行覆盖,259 歧义修复无法被真机哨兵守护。

建议(同 PR 补齐):windows-process.yml 的 push/PR paths 增加 internal/runfile/process*.go 与 internal/runfile/zz_issue3079_test.go(该 workflow 定位即 Windows 进程语义验证,扩展名正言顺;runner 已是 windows-latest,跑 go test ./internal/runfile/ 即可)。补齐后本复核转为无阻塞。

…rt (#3079)

PR review blocker: the 75 lines of windows-tagged probes landed in this
PR would never execute - ci.yml Test runs ubuntu-only and
windows-process.yml paths matched only internal/util. The workflow now
also triggers on internal/runfile/*.go and internal/restart/*.go and
runs their full package suites on the windows runner, so the #3079
probes (exit-code-259 child, creation-time token) and the #1290
ACCESS_DENIED sentinel execute on a real Windows kernel.
@topcheer
topcheer merged commit 2714adf into main Oct 2, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant