amadan

carlos / platform Public

activator: the control socket outlives the drain

childProc.Stop closed the control listener first and signalled the child
second, so the socket was unbound and unlinked before the child had even
seen SIGTERM. An app that registers or re-asserts its once-timers on the way
down — the story spec §7.1 tells and §7.2's verbs serve — got
connection-refused on the channel the platform had just handed it, during
precisely the window it was told to use.

Signal first; close after the process is gone. The close is a defer, so it
stays unconditional on every path out: already-exited, lost-the-race,
kill-timeout, and even a Signal error handed back to the caller. Nothing
else in this file closes it, and a listener left bound with no process
behind it is the #129 stale-socket wake trap waiting to happen.

Two tests. The first drives the real childProc — a child that traps SIGTERM
and stays up until released, a real edge.ControlListener with a verb
mounted, and a request issued strictly between "SIGTERM delivered" (the
child's own trap file) and "child exited"; it fails with the old ordering
(dial: no such file or directory) and then pins the socket being unlinked
once Stop returns. The second covers the early return an already-dead child
takes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev e26b600e75bc65633912507ece3bc7f2baa82b91 parent cc42202
2 files changed, +147 −3
  • internal/activator/activator_test.go +126 −0
  • internal/activator/backend_exec.go +21 −3
diff --git a/internal/activator/activator_test.go b/internal/activator/activator_test.go
index 8d771b423..96386e86e 100644
--- a/internal/activator/activator_test.go
+++ b/internal/activator/activator_test.go
@@ -20,6 +20,7 @@ import (
"net/http/httptest"
"net/http/httputil"
"os"
+ "os/exec"
"path/filepath"
"slices"
"strings"
@@ -3610,3 +3611,128 @@ func TestExecBackendDoesNotNormalizeSquatterSocket(t *testing.T) {
t.Fatalf("squatter socket mode after Start = %o, want its own 600 — a socket our child did not bind is never ours to widen", fi.Mode().Perm())
}
}
+
+// The control socket must OUTLIVE the drain. Stop used to close it before the
+// child was even signalled, which made the spec's §7.1 story — an app that
+// re-asserts its once-timers as it winds down, over the §7.2 verbs on
+// CARLOS_CONTROL_SOCKET — a connection-refused during exactly the window the
+// app was told to use it.
+//
+// The test drives the real childProc: a child that traps SIGTERM and stays up
+// until the test releases it, a real edge.ControlListener with a verb
+// mounted, and a request issued strictly between "SIGTERM delivered" and
+// "child exited". It also pins the other half — that the listener is still
+// closed, on this path, once the child is gone.
+func TestChildProcStopKeepsTheControlSocketUpUntilTheChildExits(t *testing.T) {
+ requireUnixSockets(t)
+ dir := t.TempDir()
+ ready := filepath.Join(dir, "ready") // the child's SIGTERM trap is installed
+ termed := filepath.Join(dir, "termed") // the child saw SIGTERM
+ release := filepath.Join(dir, "release") // the test lets the child finish
+ script := filepath.Join(dir, "app")
+ // The ready marker is touched AFTER the trap is installed, and the test
+ // waits for it before signalling: a SIGTERM that arrives in the shell's
+ // first few instructions takes the default action and kills the child
+ // outright, and there is no drain window to observe.
+ body := "#!/bin/sh\n" +
+ "trap \"touch '" + termed + "'\" TERM\n" +
+ "touch '" + ready + "'\n" +
+ "while [ ! -f '" + release + "' ]; do sleep 0.02; done\n" +
+ "exit 0\n"
+ if err := os.WriteFile(script, []byte(body), 0o755); err != nil {
+ t.Fatal(err)
+ }
+
+ ctlSock := tmpSock(t)
+ ctl, err := edge.ListenControl(ctlSock, "drain.example.com", nil, func(mux *http.ServeMux) {
+ mux.HandleFunc("GET /schedule", func(w http.ResponseWriter, r *http.Request) {
+ w.WriteHeader(http.StatusOK)
+ })
+ })
+ if err != nil {
+ t.Fatalf("ListenControl: %v", err)
+ }
+
+ cmd := exec.Command(script)
+ if err := cmd.Start(); err != nil {
+ ctl.Close()
+ t.Fatalf("start the fake child: %v", err)
+ }
+ p := &childProc{cmd: cmd, done: make(chan struct{}), ctl: ctl}
+ go func() {
+ p.err = cmd.Wait()
+ close(p.done)
+ }()
+
+ waitForFile(t, ready, 10*time.Second)
+
+ stopped := make(chan error, 1)
+ go func() {
+ ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+ defer cancel()
+ stopped <- p.Stop(ctx)
+ }()
+
+ // Strictly inside the drain: SIGTERM has been delivered (the child's own
+ // trap says so) and the child has not exited.
+ waitForFile(t, termed, 10*time.Second)
+ if p.Exited() {
+ t.Fatal("the child exited before the drain window could be observed — the test proves nothing")
+ }
+ req, err := http.NewRequest("GET", "http://carlos/schedule", nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+ resp, err := socketClient(ctlSock).Do(req)
+ if err != nil {
+ t.Fatalf("control request during the drain: %v — the socket must still answer while the child drains", err)
+ }
+ resp.Body.Close()
+ if resp.StatusCode != http.StatusOK {
+ t.Fatalf("control request during the drain = %d, want 200", resp.StatusCode)
+ }
+
+ if err := os.WriteFile(release, nil, 0o600); err != nil {
+ t.Fatal(err)
+ }
+ select {
+ case err := <-stopped:
+ if err != nil {
+ t.Fatalf("Stop: %v", err)
+ }
+ case <-time.After(30 * time.Second):
+ t.Fatal("Stop never returned after the child was released")
+ }
+ // The close is only deferred, not dropped: a listener left bound to a
+ // socket with no process behind it is the #129 stale-socket wake trap.
+ if _, err := os.Stat(ctlSock); !os.IsNotExist(err) {
+ t.Fatalf("control socket after Stop returned: stat err = %v, want it unlinked", err)
+ }
+}
+
+// The already-exited path takes an early return, and the listener close has
+// to survive it: an instance whose child crashed on its own still leaves the
+// agent holding a bound socket nothing is behind.
+func TestChildProcStopClosesTheControlSocketWhenTheChildIsAlreadyGone(t *testing.T) {
+ requireUnixSockets(t)
+ ctlSock := tmpSock(t)
+ ctl, err := edge.ListenControl(ctlSock, "gone.example.com", nil, nil)
+ if err != nil {
+ t.Fatalf("ListenControl: %v", err)
+ }
+ cmd := exec.Command("/bin/sh", "-c", "exit 0")
+ if err := cmd.Start(); err != nil {
+ ctl.Close()
+ t.Fatal(err)
+ }
+ p := &childProc{cmd: cmd, done: make(chan struct{}), ctl: ctl}
+ p.err = cmd.Wait()
+ close(p.done) // the child is gone before Stop is ever called
+
+ if err := p.Stop(context.Background()); err != nil {
+ t.Fatalf("Stop on an already-exited child: %v", err)
+ }
+ if _, err := os.Stat(ctlSock); !os.IsNotExist(err) {
+ t.Fatalf("control socket after Stop on an exited child: stat err = %v, want it unlinked", err)
+ }
+}
diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go
index beb11f9f4..566022c07 100644
--- a/internal/activator/backend_exec.go
+++ b/internal/activator/backend_exec.go
@@ -922,9 +922,27 @@ func (p *childProc) Exited() bool {
}
func (p *childProc) Stop(ctx context.Context) error {
- if p.ctl != nil {
- p.ctl.Close()
- }
+ // The control listener OUTLIVES the drain, and the ordering is the whole
+ // point. Closing it first — which is what this did — unbound and unlinked
+ // the socket before the child had even seen SIGTERM, so an app that
+ // registers or re-asserts its once-timers on the way down (spec §7.1's
+ // "re-assert at boot, survive restarts" story; §7.2 is the verb) got
+ // connection-refused on the channel the platform had just told it about,
+ // during precisely the window it was told to use. Signal first, close
+ // after the process is gone.
+ //
+ // Deferred, so it is unconditional on every path out of here — the
+ // already-exited return, the lost-race return, the kill-timeout return,
+ // and even a Signal error we hand back to the caller. A listener left
+ // bound to a socket no process is behind is the stale-socket wake trap
+ // (#129) waiting to happen, and this is the only close a childProc ever
+ // gets — Start's own is on the path where the spawn failed and no
+ // childProc was ever built.
+ defer func() {
+ if p.ctl != nil {
+ p.ctl.Close()
+ }
+ }()
if p.Exited() {
return nil
}