fix: drop a stale buffered exit status so a reused pid waits for its own child

Author: mirkobrombin

internal/reaper/pidreuse_test.go

@@ -0,0 +1,55 @@
+package reaper
+
+import (
+	"syscall"
+	"testing"
+	"time"
+)
+
+func exit(code int) syscall.WaitStatus { return syscall.WaitStatus(code << 8) }
+
+// TestWaitDropsStalePIDReuse ensures a buffered status left by an earlier child
+// is not handed to a new child that reused the same pid: Wait must ignore the
+// stale entry, install a fresh waiter, and route the new child's real exit
+// (R-SINIT5).
+func TestWaitDropsStalePIDReuse(t *testing.T) {
+	r := NewRegistry()
+	// An orphan exited earlier with pid 100 and its status was buffered (no waiter).
+	r.Deliver(100, exit(7))
+	// Age it so it predates the reused pid's registration.
+	r.mu.Lock()
+	r.pending[100] = pending{ws: exit(7), at: time.Now().Add(-5 * time.Second)}
+	r.mu.Unlock()
+
+	ch := r.Wait(100) // a new child reused pid 100 and registers interest
+	select {
+	case ws := <-ch:
+		t.Fatalf("Wait returned stale status (exit %d) for a reused pid; must wait for the new child", ws.ExitStatus())
+	default:
+	}
+
+	r.Deliver(100, exit(0)) // the new child's real exit
+	select {
+	case ws := <-ch:
+		if ws.ExitStatus() != 0 {
+			t.Errorf("routed exit %d, want 0 (the new child)", ws.ExitStatus())
+		}
+	default:
+		t.Fatal("the new child's status did not route to the waiter")
+	}
+}
+
+// TestWaitReturnsFreshBufferedStatus keeps the legitimate register-vs-exit race
+// working: a status buffered just before Wait is returned immediately.
+func TestWaitReturnsFreshBufferedStatus(t *testing.T) {
+	r := NewRegistry()
+	r.Deliver(200, exit(3))
+	select {
+	case ws := <-r.Wait(200):
+		if ws.ExitStatus() != 3 {
+			t.Errorf("got exit %d, want 3", ws.ExitStatus())
+		}
+	default:
+		t.Fatal("a freshly buffered status should be returned immediately")
+	}
+}

internal/reaper/reaper.go

@@ -34,17 +34,27 @@ func NewRegistry() *Registry {
 	}
 }
 
+// staleWait bounds how old a buffered status may be to satisfy a Wait. A caller
+// registers interest immediately after spawning, so a status legitimately
+// buffered in the register-vs-exit race is only microseconds old; anything older
+// belongs to an earlier child whose pid has since been reused (R-SINIT5), and
+// must not be handed to the new child's waiter.
+const staleWait = time.Second
+
 // Wait registers interest in pid and returns a one-shot channel for its exit
 // status. If the child already exited (its status was delivered before this
-// call -- the tiny register-vs-exit window), the buffered status is returned
-// immediately.
+// call -- the tiny register-vs-exit window), the freshly buffered status is
+// returned immediately. A buffered status older than staleWait is a leftover
+// from a prior child with the same (reused) pid: it is dropped and a fresh
+// waiter installed so the new child's real exit routes here.
 func (r *Registry) Wait(pid int) <-chan syscall.WaitStatus {
 	ch := make(chan syscall.WaitStatus, 1)
 	r.mu.Lock()
-	if p, ok := r.pending[pid]; ok {
+	if p, ok := r.pending[pid]; ok && time.Since(p.at) < staleWait {
 		ch <- p.ws
 		delete(r.pending, pid)
 	} else {
+		delete(r.pending, pid) // drop any stale leftover for a reused pid
 		r.waiters[pid] = ch
 	}
 	r.mu.Unlock()