fix(middleware): bound the upstream singleflight call with the backend timeout

[silverbackdan]

Fixes #849.

Problem

Upstream() coalesces the upstream calls with singleflight.Do, and Do keeps the entry in the group until the callback returns. An upstream that never returns therefore has two consequences:

• the cache key is unusable for the lifetime of the process, every later request for it joins a call that cannot complete;
• each of those requests gets its 504 when the backend timeout is exceeded, but its goroutine stays parked in wg.Wait() afterwards.

In the report the process had one owner blocked for 177 minutes:

goroutine 71 [select, 177 minutes]:
github.com/dunglas/frankenphp.handleRequestWithRegularPHPThreads(...)
...
github.com/darkweak/souin/pkg/middleware.(*SouinBaseHandler).Upstream.func2()
golang.org/x/sync/singleflight.(*Group).doCall(...)
golang.org/x/sync/singleflight.(*Group).Do(...)
github.com/darkweak/souin/pkg/middleware.(*SouinBaseHandler).Upstream(...)

and 1127 joiners accumulated behind it, the oldest also 177 minutes old:

goroutine 111 [sync.WaitGroup.Wait, 177 minutes]:
sync.(*WaitGroup).Wait(...)
golang.org/x/sync/singleflight.(*Group).Do(...)
github.com/darkweak/souin/pkg/middleware.(*SouinBaseHandler).Upstream(...)

The pod couldn't pass its readiness probe anymore on a URL it was otherwise serving in ~7ms.

Change

Upstream() now uses DoChan and selects the result against rq.Context().Done(). The request context already carries the backend timeout (context/timeout.go), so the shared call gets the same bound as its callers, no new constant nor configuration key. When it's exceeded the key is forgotten, so the next request runs a new call instead of inheriting the dead one, and the caller returns instead of waiting forever.

Two details:

• the callback runs in its own goroutine with DoChan, so the recover moved inside it. The panic is stored and rethrown in the caller goroutine, where the existing defer handles http.ErrAbortHandler as before. Otherwise singleflight rethrows it with go panic(e), in a goroutine nobody can recover from.
• since Upstream() can now return while the callback is still running, the background goroutine in ServeHTTP doesn't return the buffer to the pool when the request context is done: the abandoned call may still write into it.

Deduplication is unchanged on the normal path: concurrent requests still share the call and still log Reused response from concurrent request with the key.

Revalidate() has the same Do pattern. I left it alone to keep this focused, happy to align it the same way if you'd like.

Tests

TestStalledUpstreamDoesNotHoldTheSingleflightKey covers it: an upstream that never returns, a second call for the same key that must return when the backend timeout is exceeded rather than wait for it, and a third one that must reach the upstream and get its body once the key has been forgotten. On master it fails with BUG: the request that joined the stalled upstream call waits for it indefinitely.

go build ./... and go test -race ./... pass, and the new test passes with -race -count=4. Plugins weren't built, only the root module.

[netlify]

✅ Deploy Preview for teal-sprinkles-4c7f14 ready!

Name	Link
🔨 Latest commit	67ee7b1
🔍 Latest deploy log	https://app.netlify.com/projects/teal-sprinkles-4c7f14/deploys/6a7ec590895c7f0008dc3bde
😎 Deploy Preview	https://deploy-preview-850--teal-sprinkles-4c7f14.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.