Skip to content

fix(lxdclient): fix context race condition in ProtocolLXD WithContext - #77

Merged
whywaita merged 1 commit into
feat/metrics-mutex-waitfrom
fix/context-race-condition
Mar 10, 2026
Merged

whywaita merged 1 commit into
feat/metrics-mutex-waitfrom
fix/context-race-condition

Conversation

@whywaita

@whywaita whywaita commented Mar 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • Fix race condition where ProtocolLXD.WithContext() was called before APICallMutex.Lock(), causing concurrent goroutines to overwrite each other's context on the shared client
  • Fix context not being reset on error paths, leaving the client with a canceled context for all subsequent calls
  • Fix setLXDHostResourceCache modifying client context without holding the Mutex

Changes

  • server/pkg/api/server_host.go: Move WithContext after APICallMutex.Lock(), add defer reset on all paths
  • server/pkg/lxdclient/resource.go:
    • GetResourceFromLXD: Move WithContext after Mutex acquisition + defer reset
    • GetResourceFromLXDWithClient: Remove xsemaphore and internal WithContext/reset (callers now own this responsibility)
  • server/pkg/metric/scrape_lxd.go: Add WithContext + defer reset after TryLock
  • server/pkg/resourcecache/resource_cache.go: Add APICallMutex.Lock() + WithContext + defer reset
  • server/go.mod: Remove xsemaphore dependency
  • server/pkg/lxdclient/connect_test.go: Add regression tests for Mutex-based concurrency control

Test Plan

  • go test ./... — all tests pass
  • go vet ./... — no issues
  • go build ./... — build succeeds

Notes

  • Root cause fix for production incident where context canceled errors caused no available instance failures despite available capacity
  • Unified pattern applied to all call sites: Mutex Lock → WithContext → defer reset → API call

WithContext was called before APICallMutex.Lock(), causing concurrent
goroutines to overwrite each other's context on the shared client.
Additionally, context was not reset on error paths, leaving the client
with a canceled context for all subsequent calls.

Changes:
- Move WithContext after Mutex acquisition in all call sites
- Add defer reset to context.Background() on all paths
- Add Mutex to setLXDHostResourceCache (was missing)
- Remove xsemaphore from GetResourceFromLXDWithClient (callers now
  hold the Mutex and set context)
- Add regression tests for Mutex-based concurrency control
@whywaita
whywaita merged commit 5c9b79b into feat/metrics-mutex-wait Mar 10, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant