From 54598aee8ed09e52e0d792adc11bd1a95ae8276e Mon Sep 17 00:00:00 2001 From: mrigangha Date: Thu, 7 May 2026 03:32:06 +0000 Subject: [PATCH] Fixed the stale pid issue --- pkg/pid/pidfile.go | 29 +++++++++-- pkg/pid/pidfile_test.go | 106 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 126 insertions(+), 9 deletions(-) diff --git a/pkg/pid/pidfile.go b/pkg/pid/pidfile.go index 682bf1bb9..7201ade03 100644 --- a/pkg/pid/pidfile.go +++ b/pkg/pid/pidfile.go @@ -46,16 +46,35 @@ func generateToken() string { return hex.EncodeToString(b) } -// Does a heath check of the port if already a gateway is running -func isGatewayAlive(port int) bool { - url := fmt.Sprintf("http://localhost:%d/health", port) +// isGatewayAlive performs a health check against the recorded host and port, +// then verifies the reported PID matches expectedPID to confirm the process +// is actually a picoclaw gateway and not a foreign service on the same port. +func isGatewayAlive(host string, port int, expectedPID int) bool { + url := fmt.Sprintf("http://%s:%d/health", host, port) client := &http.Client{Timeout: 2 * time.Second} resp, err := client.Get(url) if err != nil { + // Port not responding — PID belongs to a foreign process return false } defer resp.Body.Close() - return resp.StatusCode == 200 + + if resp.StatusCode != http.StatusOK { + return false + } + + var body struct { + PID int `json:"pid"` + } + if err := json.NewDecoder(resp.Body).Decode(&body); err != nil { + return false + } + fmt.Println("HEllo") + + // Only treat as alive if the reported PID matches the PID file. + // This prevents an unrelated service on the same port from being + // mistaken for a running gateway. + return body.PID == expectedPID } // WritePidFile creates (or overwrites) the PID file atomically. @@ -76,7 +95,7 @@ func WritePidFile(homePath, host string, port int) (*PidFileData, error) { // PID file on a shared volume, the host's PID 1 (init) would // pass the isProcessRunning check, blocking new gateway starts. // Treat recorded PID 1 as always stale. - if data.PID != 1 && isProcessRunning(data.PID) && isGatewayAlive(data.Port) { + if data.PID != 1 && isProcessRunning(data.PID) && isGatewayAlive(data.Host, data.Port, data.PID){ return nil, fmt.Errorf("gateway is already running (PID: %d, version: %s)", data.PID, data.Version) } logger.Warnf("not running (PID: %d) so will remove the pid file: %s", data.PID, pidPath) diff --git a/pkg/pid/pidfile_test.go b/pkg/pid/pidfile_test.go index 2d3c11f63..c8fac1aa5 100644 --- a/pkg/pid/pidfile_test.go +++ b/pkg/pid/pidfile_test.go @@ -1,10 +1,14 @@ package pid import ( - "encoding/json" - "os" - "path/filepath" - "testing" + "encoding/json" + "net" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strconv" + "testing" ) // tmpDir returns a clean temporary directory for a test. @@ -51,6 +55,100 @@ func TestPidFilePath(t *testing.T) { } } +// verifies that an unrelated service on the recorded port is not mistaken for the gateway. +func TestWritePidFileHealthPIDMismatch(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/health", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + // Return PID 99999 — does not match the PID file entry + json.NewEncoder(w).Encode(map[string]any{"pid": 99999, "status": "ok"}) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + host, portStr, _ := net.SplitHostPort(srv.Listener.Addr().String()) + port, _ := strconv.Atoi(portStr) + + dir := tmpDir(t) + foreign := PidFileData{ + PID: os.Getpid(), // real running PID so it reaches isGatewayAlive + Token: "deadbeef12345678deadbeef12345678", + Port: port, + Host: host, + } + raw, _ := json.MarshalIndent(foreign, "", " ") + os.WriteFile(filepath.Join(dir, pidFileName), raw, 0o600) + + // Should succeed — health PID (99999) != PID file PID (os.Getpid()) = not our gateway + data, err := WritePidFile(dir, "127.0.0.1", 18790) + if err != nil { + t.Fatalf("WritePidFile should treat health PID mismatch as stale, got error: %v", err) + } + if data.PID != os.Getpid() { + t.Errorf("PID = %d, want %d", data.PID, os.Getpid()) + } +} + +// verifies that isGatewayAlive uses the host from the PID file instead of hardcoding localhost. +func TestWritePidFileNonLocalhostHost(t *testing.T) { + if !isProcessRunning(os.Getppid()) { + t.Skip("skipping: parent process not running in this environment") + } + + mux := http.NewServeMux() + mux.HandleFunc("/health", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + // Return parent PID — matches the PID file entry + json.NewEncoder(w).Encode(map[string]any{"pid": os.Getppid(), "status": "ok"}) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + host, portStr, _ := net.SplitHostPort(srv.Listener.Addr().String()) + port, _ := strconv.Atoi(portStr) + + dir := tmpDir(t) + foreign := PidFileData{ + PID: os.Getppid(), // parent PID — real, running, but not us + Token: "deadbeef12345678deadbeef12345678", + Port: port, + Host: host, + } + raw, _ := json.MarshalIndent(foreign, "", " ") + os.WriteFile(filepath.Join(dir, pidFileName), raw, 0o600) + + // Should block — PID exists, health responds with matching PID on non-localhost host + _, err := WritePidFile(dir, "127.0.0.1", 18790) + if err == nil { + t.Fatal("WritePidFile should block startup when gateway is genuinely alive on non-localhost host") + } +} + + +//verifies that a foreign process reusing a crashed gateway's PID is treated as stale. +func TestWritePidFileForeignPIDReuse(t *testing.T) { + dir := tmpDir(t) + + // PID 1 (init/systemd) is always running but won't respond on port 19999 + foreign := PidFileData{ + PID: 1, + Token: "deadbeef12345678deadbeef12345678", + Port: 19999, // nothing listening here + Host: "127.0.0.1", + } + raw, _ := json.MarshalIndent(foreign, "", " ") + os.WriteFile(filepath.Join(dir, pidFileName), raw, 0o600) + + // Should succeed — foreign PID reuse should be treated as stale + data, err := WritePidFile(dir, "127.0.0.1", 18790) + if err != nil { + t.Fatalf("WritePidFile should treat foreign PID as stale, got error: %v", err) + } + if data.PID != os.Getpid() { + t.Errorf("PID = %d, want %d", data.PID, os.Getpid()) + } +} + // TestWritePidFile creates a PID file and verifies its contents. func TestWritePidFile(t *testing.T) { dir := tmpDir(t)