Make toolsrv socket polling cancellable
This commit is contained in:
parent
a9f4899ed3
commit
9bbb6b9152
|
|
@ -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)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue