diff --git a/plugin/appstore/reap.go b/plugin/appstore/reap.go index 73ea4aa..75af70b 100644 --- a/plugin/appstore/reap.go +++ b/plugin/appstore/reap.go @@ -12,6 +12,9 @@ import ( // it is SIGKILLed. const reapGrace = 3 * time.Second +// reapPasses bounds how many times reapStale rescans the process table. +const reapPasses = 3 + // Children run in their own process group (Setpgid), so when the daemon // dies without running its shutdown path — the rx watchdog's os.Exit for // supervisor respawn, SIGKILL, a crash — nothing signals them and they are @@ -28,27 +31,49 @@ const reapGrace = 3 * time.Second // versions that predate this check, and never touches a recycled pid. // Returns the pids it reaped. func (s *supervisor) reapStale(a *installedApp) []int { - pids := findInstances(a.BinaryPath, a.SocketPath) - for _, pid := range pids { - s.logger.Printf("app=%s: reaping stale instance pid=%d left by a previous daemon", a.Manifest.ID, pid) - // Negative pid → the whole process group the child leads; fall - // back to the pid alone if it no longer leads a group. - if syscall.Kill(-pid, syscall.SIGTERM) != nil { - _ = syscall.Kill(pid, syscall.SIGTERM) + var reaped []int + seen := map[int]bool{} + // Several passes: reading a process's argv can fail transiently (e.g. on + // macOS while it is mid-fork), so one scan can miss an instance. + for pass := 0; pass < reapPasses; pass++ { + var pids []int + for _, pid := range findInstances(a.BinaryPath, a.SocketPath) { + if !seen[pid] { + seen[pid] = true + pids = append(pids, pid) + } } - } - deadline := time.Now().Add(reapGrace) - for _, pid := range pids { - for time.Now().Before(deadline) && syscall.Kill(pid, 0) == nil { - time.Sleep(50 * time.Millisecond) + if len(pids) == 0 { + if pass > 0 || len(reaped) > 0 { + break + } + // Nothing on the first scan: one quick re-check covers a + // transient argv read failure. + time.Sleep(20 * time.Millisecond) + continue } - if syscall.Kill(pid, 0) == nil { - if syscall.Kill(-pid, syscall.SIGKILL) != nil { - _ = syscall.Kill(pid, syscall.SIGKILL) + for _, pid := range pids { + s.logger.Printf("app=%s: reaping stale instance pid=%d left by a previous daemon", a.Manifest.ID, pid) + // Negative pid → the whole process group the child leads; fall + // back to the pid alone if it no longer leads a group. + if syscall.Kill(-pid, syscall.SIGTERM) != nil { + _ = syscall.Kill(pid, syscall.SIGTERM) + } + } + deadline := time.Now().Add(reapGrace) + for _, pid := range pids { + for time.Now().Before(deadline) && syscall.Kill(pid, 0) == nil { + time.Sleep(50 * time.Millisecond) + } + if syscall.Kill(pid, 0) == nil { + if syscall.Kill(-pid, syscall.SIGKILL) != nil { + _ = syscall.Kill(pid, syscall.SIGKILL) + } } } + reaped = append(reaped, pids...) } - return pids + return reaped } // findInstances returns the pids (other than this process) whose argv diff --git a/plugin/appstore/rlimit_other.go b/plugin/appstore/rlimit_other.go index 54ad666..23738f6 100644 --- a/plugin/appstore/rlimit_other.go +++ b/plugin/appstore/rlimit_other.go @@ -2,7 +2,12 @@ package appstore -import "log" +import ( + "log" + "sync" +) + +var resourceLimitWarning sync.Once // applyChildResourceLimits is the non-Linux build's no-op. macOS's // equivalent (setrlimit) affects the calling process, not children; @@ -14,8 +19,7 @@ import "log" // The addrSpaceLimit parameter is accepted for signature parity with the // Linux build and ignored here. func applyChildResourceLimits(pid int, addrSpaceLimit uint64, logger *log.Logger) { - // Single startup log line would be nicer than per-spawn, but - // inlining here keeps the supervisor call-site identical to the - // Linux build. Cheap log line at info level — easy to grep. - logger.Printf("resource limits not enforced for pid=%d on this platform (linux-only); requested addr-space cap=%d ignored", pid, addrSpaceLimit) + resourceLimitWarning.Do(func() { + logger.Print("resource limits not enforced on this platform (linux-only); child file-descriptor and address-space caps are ignored") + }) } diff --git a/plugin/appstore/rlimit_other_test.go b/plugin/appstore/rlimit_other_test.go new file mode 100644 index 0000000..085c596 --- /dev/null +++ b/plugin/appstore/rlimit_other_test.go @@ -0,0 +1,26 @@ +//go:build !linux + +package appstore + +import ( + "bytes" + "log" + "strings" + "sync" + "testing" +) + +func TestResourceLimitWarningOnce(t *testing.T) { + resourceLimitWarning = sync.Once{} + var output bytes.Buffer + logger := log.New(&output, "", 0) + var wg sync.WaitGroup + for i := 0; i < 20; i++ { + wg.Add(1) + go func(pid int) { defer wg.Done(); applyChildResourceLimits(pid, 4<<30, logger) }(i) + } + wg.Wait() + if got := strings.Count(output.String(), "resource limits not enforced"); got != 1 { + t.Fatalf("warnings=%d: %s", got, output.String()) + } +}