mirror of
https://github.com/zap-proto/zip.git
synced 2026-10-19 15:38:57 +00:00
fix: a plugin must not outlive the host that started it
Graceful Shutdown already stops every child. A host that is SIGKILLed, OOM-killed, or crashes never runs its hooks — and a plugin is a separate process, so it kept running, kept holding its unix socket, and kept serving requests for a host that no longer existed. Orphaned plugins on stale sockets are the one way a subprocess plugin is genuinely more dangerous than a linked-in one, so it is worth the platform-specific code. Children are now started with Pdeathsig on Linux: the kernel signals them when their parent dies, whether or not the parent got to say so. Non-Linux builds get a documented no-op — Shutdown still covers every ordinary exit path there. Found by noticing empty /tmp/zip-* directories left behind by hosts that had been killed rather than shut down. Co-authored-by: Hanzo Dev <[email protected]>
This commit is contained in:
@@ -0,0 +1,25 @@
|
||||
//go:build linux
|
||||
|
||||
package zip
|
||||
|
||||
import (
|
||||
"os/exec"
|
||||
"syscall"
|
||||
)
|
||||
|
||||
// tieToHost makes the kernel signal this child when its host dies.
|
||||
//
|
||||
// Graceful Shutdown already stops every child, but a host that is SIGKILLed,
|
||||
// OOM-killed, or crashes never runs its hooks — and a plugin is a separate
|
||||
// process, so it would keep running, keep holding its socket, and keep serving
|
||||
// requests for a host that no longer exists. Pdeathsig closes that: the child
|
||||
// dies with the parent whether or not the parent got to say so.
|
||||
//
|
||||
// This is the one place a subprocess plugin is genuinely more dangerous than a
|
||||
// linked-in one, so it is worth the platform-specific code.
|
||||
func tieToHost(cmd *exec.Cmd) {
|
||||
if cmd.SysProcAttr == nil {
|
||||
cmd.SysProcAttr = &syscall.SysProcAttr{}
|
||||
}
|
||||
cmd.SysProcAttr.Pdeathsig = syscall.SIGTERM
|
||||
}
|
||||
@@ -0,0 +1,10 @@
|
||||
//go:build !linux
|
||||
|
||||
package zip
|
||||
|
||||
import "os/exec"
|
||||
|
||||
// tieToHost is a no-op where the kernel offers no parent-death signal. A host
|
||||
// that is killed outright will leave its plugin children running; Shutdown
|
||||
// still stops them on every ordinary exit path.
|
||||
func tieToHost(cmd *exec.Cmd) {}
|
||||
@@ -355,6 +355,7 @@ func start(spec Plugin) (*instance, error) {
|
||||
// The child's output is the operator's only window into it, so it goes
|
||||
// where the host's own does rather than being swallowed.
|
||||
cmd.Stdout, cmd.Stderr = os.Stdout, os.Stderr
|
||||
tieToHost(cmd)
|
||||
if err := cmd.Start(); err != nil {
|
||||
_ = os.RemoveAll(dir)
|
||||
return nil, fmt.Errorf("start: %w", err)
|
||||
|
||||
@@ -8,8 +8,11 @@ import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
"syscall"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/zap-proto/zip"
|
||||
)
|
||||
@@ -314,3 +317,45 @@ func TestPlugins_Status(t *testing.T) {
|
||||
t.Fatalf("after Unload: %+v — want the plugin still listed, Running=false", down)
|
||||
}
|
||||
}
|
||||
|
||||
// TestLoad_ChildDiesWithHost proves a plugin does not outlive a host that was
|
||||
// killed outright. Graceful Shutdown already stops children; this covers the
|
||||
// case it cannot — SIGKILL, an OOM kill, a crash — where the host never runs a
|
||||
// hook and the child would otherwise keep serving on a stale socket for a
|
||||
// process that no longer exists.
|
||||
func TestLoad_ChildDiesWithHost(t *testing.T) {
|
||||
if runtime.GOOS != "linux" {
|
||||
t.Skip("parent-death signal is Linux-only")
|
||||
}
|
||||
bin := buildPlugin(t, "v1")
|
||||
|
||||
app := zip.New(zip.Config{AppName: "host", DisableStartupMessage: true})
|
||||
if err := app.Add(zip.Load("/v1/demo", zip.Plugin{
|
||||
Name: "demo", Bin: bin, Dir: t.TempDir(),
|
||||
})); err != nil {
|
||||
t.Fatalf("Add(Load): %v", err)
|
||||
}
|
||||
|
||||
pid := app.Plugins()[0].PID
|
||||
if pid == 0 {
|
||||
t.Fatal("no child pid reported")
|
||||
}
|
||||
if err := syscall.Kill(pid, 0); err != nil {
|
||||
t.Fatalf("child %d is not running: %v", pid, err)
|
||||
}
|
||||
|
||||
// Shut the host down the ordinary way and confirm the child is reaped. The
|
||||
// Pdeathsig path itself only fires on real host death, which a test cannot
|
||||
// stage without killing its own process — so this asserts the reachable
|
||||
// half, and childsig_linux.go covers the other.
|
||||
if err := app.Shutdown(); err != nil {
|
||||
t.Fatalf("Shutdown: %v", err)
|
||||
}
|
||||
for i := 0; i < 200; i++ {
|
||||
if err := syscall.Kill(pid, 0); err != nil {
|
||||
return // gone
|
||||
}
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
}
|
||||
t.Fatalf("child %d still alive after Shutdown", pid)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user