Windows ConPTY pidAlive returns true for terminated processes (OpenProcess without WaitForSingleObject)

[i-trytoohard]

Summary

pidAlive in backend/internal/adapters/runtime/conpty/pidalive_windows.go probes process liveness by opening the process handle and returning true if OpenProcess succeeds — without ever calling WaitForSingleObject. On Windows a terminated process's kernel object persists until all handles to it are closed, so OpenProcess succeeds for recently-dead processes and pidAlive reports them as alive.

Root cause

The current implementation (pidalive_windows.go:14-21):

func pidAlive(pid int) bool {
    h, err := windows.OpenProcess(windows.SYNCHRONIZE, false, uint32(pid))
    if err != nil {
        return false
    }
    _ = windows.CloseHandle(h)
    return true // ← only checks that the process OBJECT exists, not that it's running
}

It requests SYNCHRONIZE access — which is the access right needed to call WaitForSingleObject on the handle — but never waits. The permission is effectively dead code; the probe degenerates to "does the kernel object still exist?", which is true for any terminated-but-not-fully-reaped process.

The correct pattern already exists in this repo at backend/internal/processalive/process_windows.go::Alive, which is the shared package for exactly this check:

func Alive(pid int) bool {
    handle, err := windows.OpenProcess(windows.SYNCHRONIZE, false, uint32(pid))
    if err != nil {
        return errors.Is(err, windows.ERROR_ACCESS_DENIED) // access denied => exists
    }
    defer windows.CloseHandle(handle)
    status, err := windows.WaitForSingleObject(handle, 0)
    if err != nil {
        return false
    }
    return status == uint32(windows.WAIT_TIMEOUT) // WAIT_TIMEOUT = still running
}

WAIT_OBJECT_0 (signaled) = terminated; WAIT_TIMEOUT = still running. The conpty pidAlive is also weaker than its own Unix sibling (pidalive_unix.go), which explicitly treats zombies as dead.

Two additional gaps vs. processalive.Alive:

• No pid <= 0 guard.
• ERROR_ACCESS_DENIED is treated as dead instead of alive (the process exists, just not openable with the requested rights).

Impact

pidAlive is consumed by the conpty runtime shutdown path (runtime.go:118-127):

// Poll up to ~500ms (20 x 25ms) for the pty-host pid to exit.
deadline := time.Now().Add(500 * time.Millisecond)
for time.Now().Before(deadline) {
    if !pidAlive(sess.pid) {
        break
    }
    time.Sleep(25 * time.Millisecond)
}

The intent is "ask the host to shut down gracefully, detect its exit early, otherwise force-kill after ~500ms." With the bug, the early-exit break is effectively dead on Windows — the loop spins the full 20 iterations on every session teardown even when the host exits in a few milliseconds. Sessions still terminate correctly (the unconditional force-kill after the loop handles it), so this is a bounded latency/correctness degradation, not a hang — hence P2.

Suggested fix

Delegate to the existing, tested processalive.Alive:

//go:build windows

package conpty

import (
    "fmt"
    "os"

    "github.com/aoagents/ao/backend/internal/processalive"
)

func pidAlive(pid int) bool {
    return processalive.Alive(pid)
}

…or, if the conpty package wants to keep its own probe for test-injection reasons (pidAlive is a package-level var in ptyregistry), inline the WaitForSingleObject(handle, 0) + WAIT_TIMEOUT check and add the pid <= 0 / ERROR_ACCESS_DENIED handling from processalive.Alive.

Either way, do not rely on OpenProcess success alone.

Environment

• Windows ConPTY runtime adapter (//go:build windows)
• Reproducible on any Windows host using the ConPTY runtime; not observable on macOS/Linux (separate pidalive_unix.go).

---

Reported by Neo in Discord #bug-triage.