Bound toolsrv process completion notifications

cmd/toolsrv/main.go

@@ -22,6 +22,7 @@ import (
 	"os/user"
 	"path/filepath"
 	"strings"
+	"sync"
 	"sync/atomic"
 	"syscall"
 	"time"
@@ -54,6 +55,9 @@ var activeConns atomic.Int64
 // hadConnection is set to true once the first client connects.
 var hadConnection atomic.Bool
 
+// procInterruptWG tracks completion notifications during shutdown.
+var procInterruptWG sync.WaitGroup
+
 func main() {
 	if len(os.Args) < 2 || os.Args[1] != "serve" {
 		fmt.Fprintln(os.Stderr, "usage: toolsrv serve --cwd <path> --listen <socket> [--yolo]")
@@ -91,32 +95,37 @@ func main() {
 		srv.SetRegistry(toolReg)
 	}
 
+	// Set up signal handling
+	runCtx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	sigCh := make(chan os.Signal, 1)
+	signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM)
+	defer signal.Stop(sigCh)
+	go func() {
+		<-sigCh
+		cancel()
+	}()
+
 	// Set up callback to push completed proc output to agent prompt
 	srv.Fs.OnProcExit = func(proc *server.Proc) {
 		if proc.SessionID == "" || proc.AgentID == "" {
 			return
 		}
-		go pushProcInterrupt(proc)
+		procInterruptWG.Add(1)
+		go func() {
+			defer procInterruptWG.Done()
+			pushProcInterrupt(runCtx, proc)
+		}()
 	}
 
-	// Build the filesystem tree
 	tree := srv.BuildTree()
 	if tree == nil {
 		logger.Error("failed to build filesystem tree")
 		os.Exit(1)
 	}
 
-	// Set up signal handling
-	runCtx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	sigCh := make(chan os.Signal, 1)
-	signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM)
-	go func() {
-		<-sigCh
-		cancel()
-	}()
-
 	// Kill all procs on shutdown
+	defer procInterruptWG.Wait()
 	defer srv.Fs.KillAll()
 
 	// Start idle timeout monitor
@@ -218,7 +227,7 @@ monitor:
 }
 
 // pushProcInterrupt pushes the completed proc output to the agent's prompt.
-func pushProcInterrupt(proc *server.Proc) {
+func pushProcInterrupt(ctx context.Context, proc *server.Proc) {
 	// Build the interrupt message
 	proc.Lock()
 	output := proc.OutputString()
@@ -244,12 +253,21 @@ func pushProcInterrupt(proc *server.Proc) {
 	}
 	sockPath := fmt.Sprintf("/tmp/ns.%s.%s/ollie", u.Username, display)
 
-	conn, err := p9client.Dial("unix", sockPath)
+	ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
+	defer cancel()
+	conn, err := dialUnixContext(ctx, sockPath)
 	if err != nil {
 		logger.Warn("pushProcInterrupt: failed to connect to olliesrv: %v", err)
 		return
 	}
-	fsys, err := conn.Attach(nil, u.Username, "")
+	p9conn, err := p9client.NewConn(conn)
+	if err != nil {
+		conn.Close()
+		logger.Warn("pushProcInterrupt: failed to initialize 9P connection: %v", err)
+		return
+	}
+	defer p9conn.Close()
+	fsys, err := p9conn.Attach(nil, u.Username, "")
 	if err != nil {
 		conn.Close()
 		logger.Warn("pushProcInterrupt: failed to attach to olliesrv: %v", err)
@@ -277,7 +295,11 @@ func pushProcInterrupt(proc *server.Proc) {
 	logger.Info("pushed proc %d interrupt to %s", pid, promptPath)
 }
 
-// escapeAttr escapes a string for use in an XML-like attribute.
+func dialUnixContext(ctx context.Context, path string) (net.Conn, error) {
+	var d net.Dialer
+	return d.DialContext(ctx, "unix", path)
+}
+
 func escapeAttr(s string) string {
 	s = strings.ReplaceAll(s, "&", "&amp;")
 	s = strings.ReplaceAll(s, "\"", "&quot;")

doc/concurrency-resource-audit.md

@@ -274,13 +274,9 @@ Timeout and send-error paths delete the request channel from `pending`. Late res
 
 ### M12. `pushProcInterrupt` has weak connection ownership
 
-**Evidence:** `cmd/toolsrv/main.go:247-258`.
+**Status:** Fixed in the working tree.
 
-The function explicitly closes the filesystem but not the underlying connection. It is launched once per completed background process without a bound or timeout.
-
-**Impact:** potential socket retention and completion-goroutine pressure.
-
-**Remediation:** close both filesystem and connection, use a timeout, and track completion work under toolsrv shutdown.
+Completion notifications now use the toolsrv run context with a ten-second timeout, close both the 9P filesystem and connection, and are tracked by a wait group during shutdown.
 
 ### M13. Repeated process-GC startup creates duplicate goroutines