fix: singleflight shares the winning caller's context with all waiters #456

New issue

Open

opened 2026-07-25 23:33:33 +02:00 by dries · 0 comments

dries commented

2026-07-25 23:33:33 +02:00

Owner

Code-quality audit (P2). internal/hostsvc/local/host.go:290-292 (EnsureProjectOpencode) and :310-312 (RestartProjectOpencode).

v, err, _ := h.sf.Do(repoRoot, func() (any, error) {
	return h.ensureLocked(ctx, repoRoot)   // ctx belongs to whichever caller won the race
})

Why it matters

singleflight collapses concurrent ensure calls for one repo root, but the shared body captures the winning caller's context. If that caller's HTTP request is cancelled mid-launch, ensureLocked -> waitForProbe returns ctx.Err(), and every coalesced waiter — whose own contexts are still live — receives a spurious failure for a launch that may well have succeeded. The user-visible symptom is a "failed to launch opencode" error on a project that is in fact running, and it only reproduces under concurrency, so it will be reported as flaky rather than diagnosed.

The same applies to RestartProjectOpencode, which shares the singleflight key.

Suggested fix

Decouple the shared body from any single caller's lifetime:

shared := context.WithoutCancel(ctx)   // plus an explicit launch deadline
done := make(chan struct{})
var v any; var err error
go func() { defer close(done); v, err, _ = h.sf.Do(repoRoot, func() (any, error) {
	return h.ensureLocked(shared, repoRoot)
}) }()
select {
case <-done:
case <-ctx.Done():
	return nil, ctx.Err()   // this caller gives up; the launch continues for the others
}

Give the shared context its own bounded deadline so an abandoned launch cannot run forever.

Not applicable

The audit also flagged internal/platforms/opencode/httpcache.go:161-181 as the same pattern. It is not — getOrFetch uses context.Background() and its fetcher closures (client.go:70,149, http.go:94) call rawGet, which takes no context. No caller context is captured there.

Acceptance criteria

The singleflight body runs under a context not tied to any one caller.
Regression test: two concurrent EnsureProjectOpencode calls, cancel the first — the second still gets a successful result. Must fail on current main.
The shared launch has an explicit deadline.

Effort: S.

Code-quality audit (P2). `internal/hostsvc/local/host.go:290-292` (`EnsureProjectOpencode`) and `:310-312` (`RestartProjectOpencode`). ```go v, err, _ := h.sf.Do(repoRoot, func() (any, error) { return h.ensureLocked(ctx, repoRoot) // ctx belongs to whichever caller won the race }) ``` ## Why it matters `singleflight` collapses concurrent ensure calls for one repo root, but the shared body captures **the winning caller's** context. If that caller's HTTP request is cancelled mid-launch, `ensureLocked` -> `waitForProbe` returns `ctx.Err()`, and every coalesced waiter — whose own contexts are still live — receives a spurious failure for a launch that may well have succeeded. The user-visible symptom is a "failed to launch opencode" error on a project that is in fact running, and it only reproduces under concurrency, so it will be reported as flaky rather than diagnosed. The same applies to `RestartProjectOpencode`, which shares the singleflight key. ## Suggested fix Decouple the shared body from any single caller's lifetime: ```go shared := context.WithoutCancel(ctx) // plus an explicit launch deadline done := make(chan struct{}) var v any; var err error go func() { defer close(done); v, err, _ = h.sf.Do(repoRoot, func() (any, error) { return h.ensureLocked(shared, repoRoot) }) }() select { case <-done: case <-ctx.Done(): return nil, ctx.Err() // this caller gives up; the launch continues for the others } ``` Give the shared context its own bounded deadline so an abandoned launch cannot run forever. ## Not applicable The audit also flagged `internal/platforms/opencode/httpcache.go:161-181` as the same pattern. It is not — `getOrFetch` uses `context.Background()` and its `fetcher` closures (`client.go:70,149`, `http.go:94`) call `rawGet`, which takes no context. No caller context is captured there. ## Acceptance criteria - [ ] The singleflight body runs under a context not tied to any one caller. - [ ] Regression test: two concurrent `EnsureProjectOpencode` calls, cancel the first — the second still gets a successful result. Must fail on current `main`. - [ ] The shared launch has an explicit deadline. Effort: S.