Repository navigation
Merged
Conversation
…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
commented
Oct 2, 2026
topcheer
left a comment
Owner
Author
There was a problem hiding this comment.
治理席 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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. NowprocessExistsopens the handle withPROCESS_QUERY_LIMITED_INFORMATION|SYNCHRONIZEand usesWaitForSingleObject(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:
procStartTimereturned""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.procStartTimenow returns theGetProcessTimesCreationTime 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>nulswallowed 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 veton both packages (compiles the windows-tagged probes too)go build -tags goolm ./...+ fullinternal/runfile/internal/restarttest runs greenzz_issue3079_test.go, executed by the Windows CI runner: