Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions watch/process_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
package watch

import (
"os"
"os/exec"
"strconv"
"strings"
"syscall"
)

func processCommandLine(pid int) (string, error) {
Expand All @@ -15,3 +17,23 @@ func processCommandLine(pid int) (string, error) {
}
return strings.TrimSpace(string(out)), nil
}

// processAlive reports whether a process with the given PID is currently
// running. FindProcess always succeeds on Unix, so liveness is probed with
// signal 0.
func processAlive(pid int) bool {
if pid <= 0 {
return false
}
proc, err := os.FindProcess(pid)
if err != nil {
return false
}
return proc.Signal(syscall.Signal(0)) == nil
}

// terminateProcess asks the daemon to shut down. On Unix SIGTERM lets it clean
// up gracefully; its error (e.g. ESRCH/EPERM) is returned unchanged.
func terminateProcess(proc *os.Process) error {
return proc.Signal(syscall.SIGTERM)
}
34 changes: 33 additions & 1 deletion watch/process_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,41 @@

package watch

import "errors"
import (
"errors"
"os"
"syscall"
)

func processCommandLine(pid int) (string, error) {
_ = pid
return "", errors.New("process command line lookup not supported on windows")
}

// processAlive reports whether a process with the given PID is currently
// running. os.Process.Signal(0) is unsupported on Windows — Signal returns an
// error for any signal other than Kill — so IsRunning cannot use it there.
// Instead we open the process and check its exit code: a live process reports
// STILL_ACTIVE.
func processAlive(pid int) bool {
if pid <= 0 {
return false
}
const stillActive = 259 // STILL_ACTIVE
h, err := syscall.OpenProcess(syscall.PROCESS_QUERY_INFORMATION, false, uint32(pid))
if err != nil {
return false
}
defer syscall.CloseHandle(h)
var code uint32
if err := syscall.GetExitCodeProcess(h, &code); err != nil {
return false
}
return code == stillActive

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Require daemon ownership before accepting Windows PIDs

When .codemap/watch.pid is stale and Windows has reused that PID for an unrelated live process, this returns true even though processCommandLine is still unsupported on Windows and no repo/daemon ownership is verified. runWatchSubcommand("start") checks watchIsRunning(absRoot) before spawning, so that stale file makes codemap watch start refuse to start a real daemon, while status can report running/stale state until the user deletes the PID file. Fresh evidence: Stop is now non-destructive, but start/status still rely only on this liveness result.

Useful? React with 👍 / 👎.

}

// terminateProcess stops the daemon. Windows has no SIGTERM, so Kill (which
// maps to TerminateProcess) is the correct mechanism.
func terminateProcess(proc *os.Process) error {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Verify ownership before killing Windows PIDs

When .codemap/watch.pid is stale and its PID has been reused on Windows, the new liveness check lets runWatchSubcommand("stop") proceed for any active process, and this Kill terminates it even though processCommandLine still cannot verify that the PID belongs to a codemap daemon for this root. This is a new Windows failure mode from making IsRunning succeed there, so please verify daemon ownership before killing or refuse to stop when ownership cannot be established.

Useful? React with 👍 / 👎.

return proc.Kill()
}
15 changes: 5 additions & 10 deletions watch/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"os"
"path/filepath"
"strings"
"syscall"
"time"
)

Expand Down Expand Up @@ -93,14 +92,9 @@ func IsRunning(root string) bool {
if err != nil {
return false
}
// Check if process exists by sending signal 0
proc, err := os.FindProcess(pid)
if err != nil {
return false
}
// On Unix, FindProcess always succeeds, so send signal 0 to check
err = proc.Signal(syscall.Signal(0))
return err == nil
// Liveness is checked in a platform-specific way: Signal(0) on Unix is
// unsupported on Windows, so processAlive queries the OS directly there.
return processAlive(pid)
}

// Stop sends SIGTERM to the daemon process
Expand All @@ -113,7 +107,8 @@ func Stop(root string) error {
if err != nil {
return err
}
if err := proc.Signal(syscall.SIGTERM); err != nil {
// terminateProcess is platform-specific: SIGTERM on Unix, Kill on Windows.
if err := terminateProcess(proc); err != nil {
return err
}
// Clean up PID file
Expand Down
22 changes: 22 additions & 0 deletions watch/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package watch
import (
"encoding/json"
"os"
"os/exec"
"path/filepath"
"testing"
"time"
Expand Down Expand Up @@ -144,3 +145,24 @@ func TestWriteInitialStateWritesReadableState(t *testing.T) {
t.Fatalf("unexpected recent events in state: %#v", got.RecentEvents)
}
}

func TestProcessAliveDetectsLiveAndDeadPIDs(t *testing.T) {
if !processAlive(os.Getpid()) {
t.Fatal("current process should be reported alive")
}
if processAlive(0) {
t.Fatal("pid 0 should not be reported alive")
}
// Spawn a short-lived process, wait for it to exit, then confirm it is dead.
// Re-exec this test binary with a run filter that matches nothing so it
// exits immediately — self-contained, no dependency on an external tool.
cmd := exec.Command(os.Args[0], "-test.run=^$")
if err := cmd.Start(); err != nil {
t.Skipf("cannot spawn helper process: %v", err)
}
pid := cmd.Process.Pid
_ = cmd.Wait()
Comment on lines +156 to +164
if processAlive(pid) {
t.Fatalf("exited process %d should not be reported alive", pid)
}
}
Loading