From 9bbb6b91525d21f58c0a15f33b8e31efa4d559f6 Mon Sep 17 00:00:00 2001
From: Ollie Agent <ollie@example.com>
Date: Tue, 18 Aug 2026 07:27:42 +0200
Subject: [PATCH] Make toolsrv socket polling cancellable

---
 cmd/olliesrv/internal/toolclient/spawn.go | 22 ++++++++++++++-------
 doc/concurrency-resource-audit.md         | 24 ++++++-----------------
 2 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/cmd/olliesrv/internal/toolclient/spawn.go b/cmd/olliesrv/internal/toolclient/spawn.go
index cbc2e10..d66c716 100644
--- a/cmd/olliesrv/internal/toolclient/spawn.go
+++ b/cmd/olliesrv/internal/toolclient/spawn.go
@@ -95,7 +95,7 @@ func Spawn(ctx context.Context, cwd string, opts ...Option) (*Process, error) {
 	}
 
 	// Wait for socket to be ready
-	if err := waitForSocket(socketPath, 5*time.Second); err != nil {
+	if err := waitForSocket(ctx, socketPath, 5*time.Second); err != nil {
 		cancel()
 		_ = cmd.Process.Kill()
 		_ = cmd.Wait()
@@ -289,7 +289,7 @@ exec "$CACHE_DIR/bin/toolsrv" %s
 	}
 
 	// Wait for local socket forwarding to be ready
-	if err := waitForSocket(localSock, 5*time.Second); err != nil {
+	if err := waitForSocket(ctx, localSock, 5*time.Second); err != nil {
 		cancel()
 		_ = cmd.Process.Kill()
 		_ = cmd.Wait()
@@ -470,15 +470,23 @@ func findToolsrv() (string, error) {
 	return "", fmt.Errorf("toolsrv binary not found in PATH or common locations")
 }
 
-func waitForSocket(path string, timeout time.Duration) error {
-	deadline := time.Now().Add(timeout)
-	for time.Now().Before(deadline) {
+func waitForSocket(ctx context.Context, path string, timeout time.Duration) error {
+	deadline := time.NewTimer(timeout)
+	defer deadline.Stop()
+	ticker := time.NewTicker(50 * time.Millisecond)
+	defer ticker.Stop()
+	for {
 		conn, err := net.DialTimeout("unix", path, 100*time.Millisecond)
 		if err == nil {
 			conn.Close()
 			return nil
 		}
-		time.Sleep(50 * time.Millisecond)
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		case <-deadline.C:
+			return fmt.Errorf("timeout waiting for socket %s", path)
+		case <-ticker.C:
+		}
 	}
-	return fmt.Errorf("timeout waiting for socket %s", path)
 }
diff --git a/doc/concurrency-resource-audit.md b/doc/concurrency-resource-audit.md
index 9194c61..7247416 100644
--- a/doc/concurrency-resource-audit.md
+++ b/doc/concurrency-resource-audit.md
@@ -308,33 +308,21 @@ Startup failure and forced termination bypass graceful toolsrv socket cleanup.
 
 ### M17. Process-limit reservation is racy
 
-**Evidence:** `cmd/toolsrv/internal/server/proc.go:226-292`.
+**Status:** Fixed in the working tree.
 
-The count check and process insertion are separated by setup work. Concurrent requests can all pass the limit check.
-
-**Impact:** configured process limits are exceeded during bursts.
-
-**Remediation:** reserve a slot before setup and release it on all failure and completion paths.
+`NewProc` reserves a process slot under `procMu` before registry lookup, setup, and goroutine launch. The deferred failure path releases reservations, while successful insertion transfers the reservation into `st.procs` atomically. Concurrent callers therefore cannot exceed `procLimit` during setup.
 
 ### M18. `virtfs.Rdwr` contexts are retained until clunk
 
-**Evidence:** `virtfs/builder.go:273-286`.
+**Status:** Resolved by the existing `Rdwr` lifecycle.
 
-Successful synchronous writes retain their cancel function until `Close()`. Error paths also do not consistently cancel immediately.
-
-**Impact:** request state lives longer than the operation and complicates concurrent close behavior.
-
-**Remediation:** cancel completed operations promptly and retain only the cancellation needed for genuinely active work.
+Each write creates a request context, publishes its cancellation function only while the handler is active, clears it under `stateMu` on completion, and calls `cancel` immediately after the handler returns. Close cancels only the currently active request.
 
 ### M19. `waitForSocket` delays cancellation
 
-**Evidence:** `cmd/olliesrv/internal/toolclient/spawn.go:396-407`.
+**Status:** Fixed in the working tree.
 
-Polling uses `time.Sleep` and has no context. Cancellation waits until the fixed polling timeout.
-
-**Impact:** bounded but avoidable startup delay and delayed failure cleanup.
-
-**Remediation:** use a context-aware timer or ticker.
+Socket polling now accepts a context, uses a reusable ticker and deadline timer, and exits immediately when startup is canceled. Local and remote startup both pass their process context into the poller.
 
 ### M20. Random-generation errors are ignored