diff --git a/cmd/toolsrv/internal/bypass/bypass.go b/cmd/toolsrv/internal/bypass/bypass.go index da70897..8519949 100644 --- a/cmd/toolsrv/internal/bypass/bypass.go +++ b/cmd/toolsrv/internal/bypass/bypass.go @@ -36,8 +36,10 @@ var ( // If context is cancelled, returns (false, ctx.Err()). func markCancelled(req *Request) { mu.Lock() - delete(requests, req.ID) - req.cancelled = true + if current, ok := requests[req.ID]; ok && current == req { + delete(requests, req.ID) + req.cancelled = true + } mu.Unlock() } @@ -98,13 +100,13 @@ func NextPending(ctx context.Context) *Request { // Returns false if the request ID is not found (already resolved or invalid). func Resolve(id string, approved bool, errMsg string) bool { mu.Lock() - req, ok := requests[id] - mu.Unlock() + defer mu.Unlock() + req, ok := requests[id] if !ok { return false } - + delete(requests, id) req.approved = approved if errMsg != "" { req.err = errors.New(errMsg) diff --git a/doc/concurrency-resource-audit.md b/doc/concurrency-resource-audit.md index 916310d..1f4b3b1 100644 --- a/doc/concurrency-resource-audit.md +++ b/doc/concurrency-resource-audit.md @@ -256,13 +256,9 @@ Cancellation removes the request from the active set. `NextPending` discards sta ### M9. Concurrent bypass resolution can panic -**Evidence:** `cmd/toolsrv/internal/bypass/bypass.go:88-104`. +**Status:** Fixed in the working tree. -Resolution checks under a mutex, unlocks, then closes the request channel. Two concurrent resolvers can both close the same channel. - -**Impact:** duplicate approval messages can panic toolsrv. - -**Remediation:** remove and mark the request resolved atomically under the mutex. Only the winning resolver closes the completion channel. +`Resolve` now removes the request, updates its result, and closes the completion channel while holding the mutex. Only the first resolver can find the request and close the channel. Cancellation also verifies request ownership before removing it. ### M10. LSP shutdown is not idempotent