From eb455198aac1bbaad5fa120a22fe30dd60064f80 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 2 Oct 2026 20:23:08 +0800 Subject: [PATCH 1/2] fix(runfile,restart): Windows liveness wait-state check, creation-time 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). --- internal/restart/restart_windows.go | 6 ++- internal/runfile/process_windows.go | 57 ++++++++++++++++---- internal/runfile/zz_issue3079_test.go | 75 +++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 12 deletions(-) create mode 100644 internal/runfile/zz_issue3079_test.go diff --git a/internal/restart/restart_windows.go b/internal/restart/restart_windows.go index 9dee739bb..724520544 100644 --- a/internal/restart/restart_windows.go +++ b/internal/restart/restart_windows.go @@ -46,8 +46,12 @@ echo [ggcode restart] process %PARENT_PID% exited REM 2. Brief pause ping -n 1 127.0.0.1 >nul -REM 3. cd to original working directory +REM 3. cd to original working directory. #3079: warn on failure instead of +REM silently launching from the wrong cwd when the work dir was deleted +REM during the restart wait - the user sees why the binary path resolved +REM differently instead of an unexplained launch failure. cd /d "%WORK_DIR%" 2>nul +if errorlevel 1 echo [ggcode restart] WARNING: could not cd to "%WORK_DIR%" - launching from "%CD%" REM 4. Self-delete: the classic 'start /b del' races the parent cmd's open REM file handle and loses 100% of the time (#798). The (goto) 2>nul idiom diff --git a/internal/runfile/process_windows.go b/internal/runfile/process_windows.go index 41709257c..3aaefeae8 100644 --- a/internal/runfile/process_windows.go +++ b/internal/runfile/process_windows.go @@ -3,18 +3,27 @@ package runfile import ( - "os" + "fmt" "syscall" ) // processExists checks if a process with the given PID is running. // On Windows, syscall.Kill is not available; we use OpenProcess instead. +// +// #3079: the legacy GetExitCodeProcess==259 check was ambiguous - 259 is +// STILL_ACTIVE, but a process whose REAL exit code happened to be exactly +// 259 stayed "alive" forever. The wait state of the handle is unambiguous: +// WaitForSingleObject(handle, 0) returns WAIT_TIMEOUT while the process +// lives and WAIT_OBJECT_0 once it terminated, whatever its exit code was. func processExists(pid int) bool { if pid <= 0 { return false } - const PROCESS_QUERY_LIMITED_INFORMATION = 0x1000 - handle, err := syscall.OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid)) + const ( + PROCESS_QUERY_LIMITED_INFORMATION = 0x1000 + SYNCHRONIZE = 0x00100000 + ) + handle, err := syscall.OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION|SYNCHRONIZE, false, uint32(pid)) if err != nil { // #1290: mirror the Unix #799 EPERM semantics. ERROR_ACCESS_DENIED // (cross-user, protected/elevated process under UAC integrity-level @@ -27,17 +36,43 @@ func processExists(pid int) bool { return false } defer syscall.CloseHandle(handle) + + // Preferred: wait-state check, immune to the STILL_ACTIVE=259 exit-code + // ambiguity (#3079). + if event, waitErr := syscall.WaitForSingleObject(handle, 0); waitErr == nil { + // WAIT_OBJECT_0 (signaled) = terminated; WAIT_TIMEOUT = still alive. + return event == syscall.WAIT_TIMEOUT + } + // Degraded path (wait right unavailable, e.g. some protected processes + // grant query but not synchronize): fall back to the legacy heuristic + // rather than misreporting a live process as dead. var exitCode uint32 - err = syscall.GetExitCodeProcess(handle, &exitCode) - if err != nil { + if err := syscall.GetExitCodeProcess(handle, &exitCode); err != nil { return false } return exitCode == 259 // STILL_ACTIVE } -// PID is a convenience helper for the current process. -var _ = os.Getpid - -// procStartTime is the unix identity token (#1624 case C); Windows has no -// /proc, so the identity check degrades to signal-0 liveness. -func procStartTime(pid int) string { return "" } +// procStartTime returns the process-creation identity token (#1624 case C). +// Windows has no /proc; the token is the raw GetProcessTimes CreationTime +// FILETIME (opaque but unique per process start), which is all the +// PID-recycle check needs. A non-empty token here (re)activates the recycle +// fallback on Windows, which previously returned "" and degraded to plain +// liveness - a recycled PID kept its ghost port file until the number was +// reused by yet another process (#3079). +func procStartTime(pid int) string { + if pid <= 0 { + return "" + } + const PROCESS_QUERY_LIMITED_INFORMATION = 0x1000 + handle, err := syscall.OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid)) + if err != nil { + return "" + } + defer syscall.CloseHandle(handle) + var create, exit, kernel, user syscall.Filetime + if err := syscall.GetProcessTimes(handle, &create, &exit, &kernel, &user); err != nil { + return "" + } + return fmt.Sprintf("%d", uint64(create.HighDateTime)<<32|uint64(create.LowDateTime)) +} diff --git a/internal/runfile/zz_issue3079_test.go b/internal/runfile/zz_issue3079_test.go new file mode 100644 index 000000000..bd0742ded --- /dev/null +++ b/internal/runfile/zz_issue3079_test.go @@ -0,0 +1,75 @@ +//go:build windows + +package runfile + +// Regression probes for #3079 (process_windows.go): +// V2a: exit code 259 is STILL_ACTIVE, but a process whose REAL exit code +// was exactly 259 was judged alive forever (GetExitCodeProcess==259 +// ambiguity); the wait-state check is unambiguous. +// V2b: procStartTime returned "" on Windows, fully disabling the #1624 +// PID-recycle fallback (runfile.go's gate is skipped when either +// side has no token). +// +// Runs only on Windows CI; GOOS=windows compile/vet gates the rest. + +import ( + "os" + "testing" +) + +// TestIssue3079_ExitCode259IsDead is THE V2a regression probe: a process +// that genuinely exited with code 259 must be reported dead. +func TestIssue3079_ExitCode259IsDead(t *testing.T) { + p, err := os.StartProcess(`C:\Windows\System32\cmd.exe`, []string{`/c`, `exit 259`}, &os.ProcAttr{}) + if err != nil { + t.Skipf("cannot spawn cmd.exe: %v", err) + } + ws, err := p.Wait() + if err != nil { + t.Fatalf("wait: %v", err) + } + if ws.ExitCode() != 259 { + t.Skipf("child exit code = %d, want 259 (cannot build the ambiguity case)", ws.ExitCode()) + } + if processExists(p.Pid) { + t.Fatal("process that exited with code 259 reported alive (V2a: STILL_ACTIVE ambiguity)") + } +} + +// TestIssue3079_ProcStartTimeToken covers V2b: non-empty for a live pid, +// stable across calls, empty for a dead pid. +func TestIssue3079_ProcStartTimeToken(t *testing.T) { + self := os.Getpid() + tok1 := procStartTime(self) + if tok1 == "" { + t.Fatal("procStartTime(self) empty - PID-recycle fallback still disabled (V2b)") + } + if tok2 := procStartTime(self); tok1 != tok2 { + t.Fatalf("creation-time token unstable across calls: %q vs %q", tok1, tok2) + } + for _, r := range tok1 { + if r < '0' || r > '9' { + t.Fatalf("token not a plain decimal number: %q", tok1) + } + } + + p, err := os.StartProcess(`C:\Windows\System32\cmd.exe`, []string{`/c`, `exit`}, &os.ProcAttr{}) + if err != nil { + t.Skipf("cannot spawn cmd.exe: %v", err) + } + _, _ = p.Wait() + if got := procStartTime(p.Pid); got != "" { + t.Fatalf("procStartTime(dead pid) = %q, want empty (V2b)", got) + } +} + +// TestIssue3079_SelfAlive pins the basics after the rewrite (companion of +// the #1290 suite). +func TestIssue3079_SelfAlive(t *testing.T) { + if !processExists(os.Getpid()) { + t.Fatal("current process must be reported alive via wait-state check") + } + if processExists(0) { + t.Fatal("pid 0 must be dead") + } +} From abc1249c2be93d524aab435362c4331ae482a370 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 2 Oct 2026 20:32:44 +0800 Subject: [PATCH 2/2] ci(windows): extend windows-process paths and suites to runfile/restart (#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. --- .github/workflows/windows-process.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.github/workflows/windows-process.yml b/.github/workflows/windows-process.yml index 9e0232cdd..f0f6cc3e4 100644 --- a/.github/workflows/windows-process.yml +++ b/.github/workflows/windows-process.yml @@ -16,10 +16,14 @@ on: branches: [main] paths: - "internal/util/process*.go" + - "internal/runfile/*.go" + - "internal/restart/*.go" - ".github/workflows/windows-process.yml" pull_request: paths: - "internal/util/process*.go" + - "internal/runfile/*.go" + - "internal/restart/*.go" - ".github/workflows/windows-process.yml" workflow_dispatch: @@ -40,3 +44,12 @@ jobs: - name: Process liveness tests shell: pwsh run: go test -tags goolm -count=1 -v -run "IsProcessAlive" ./internal/util/ + - name: Runfile/restart Windows suites (#3079) + # Same real-machine policy as above: processExists wait-state semantics + # (#3079/#1290) and the restart script generator only behave as written + # on a real Windows kernel. Full package suites - the windows-tagged + # probes (exit-code-259 child, creation-time token, cd warning) were + # previously never executed anywhere (ci.yml Test is ubuntu-only and + # this workflow's paths only matched internal/util). + shell: pwsh + run: go test -tags goolm -count=1 ./internal/runfile/ ./internal/restart/