mirror of
https://github.com/lukaszraczylo/graphql-monitoring-proxy.git
synced 2026-10-14 05:48:25 +00:00
fix(coalesce): release waiters when primary fn panics
In RequestCoalescer.Do, the primary path calls fn() before storing the response, deleting the inflight key and calling wg.Done(). If fn() panics, all three steps are skipped: the sync.Map entry leaks and every coalesced waiter remains blocked on wg.Wait() forever (permanent deadlock for that key). Route release through a defer so waiters are always released and the entry cleared, even on panic. Test: TestRequestCoalescer_PanicSafety (a panicking primary must not poison the key for subsequent calls).
This commit is contained in:
@@ -138,6 +138,15 @@ func (rc *RequestCoalescer) Do(key string, fn func() (*CoalescedResponse, error)
|
||||
rc.inflightCount.Add(1)
|
||||
defer rc.inflightCount.Add(-1)
|
||||
|
||||
// Always release waiters and clear the map entry, even if fn() panics.
|
||||
// Without this, a panicking fn() leaves the key in the inflight map and
|
||||
// the WaitGroup counter positive, so every coalesced waiter for that key
|
||||
// blocks on wg.Wait() forever (permanent deadlock + leak).
|
||||
defer func() {
|
||||
rc.inflight.Delete(key)
|
||||
inflight.wg.Done()
|
||||
}()
|
||||
|
||||
// Execute the request
|
||||
response, err := fn()
|
||||
|
||||
@@ -152,10 +161,6 @@ func (rc *RequestCoalescer) Do(key string, fn func() (*CoalescedResponse, error)
|
||||
}
|
||||
inflight.mu.Unlock()
|
||||
|
||||
// Clean up and notify waiters
|
||||
rc.inflight.Delete(key)
|
||||
inflight.wg.Done()
|
||||
|
||||
// Log statistics
|
||||
waiters := inflight.waiters.Load()
|
||||
duration := time.Since(inflight.createdAt)
|
||||
|
||||
@@ -405,3 +405,40 @@ func TestMin(t *testing.T) {
|
||||
assert.Equal(t, tt.expected, result)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRequestCoalescer_PanicSafety verifies that when the primary fn() panics,
|
||||
// the coalescer still releases waiters and clears the inflight key instead of
|
||||
// permanently deadlocking that key (waiters hang on wg.Wait and the entry
|
||||
// leaks).
|
||||
func TestRequestCoalescer_PanicSafety(t *testing.T) {
|
||||
rc := NewRequestCoalescer(true, nil, nil)
|
||||
|
||||
// First call's fn() panics. Recover in a goroutine to emulate the
|
||||
// caller catching/handling the panic.
|
||||
panicDone := make(chan struct{})
|
||||
go func() {
|
||||
defer close(panicDone)
|
||||
defer func() { _ = recover() }()
|
||||
_, _ = rc.Do("key", func() (*CoalescedResponse, error) {
|
||||
panic("boom")
|
||||
})
|
||||
}()
|
||||
<-panicDone
|
||||
|
||||
// A subsequent call for the same key must complete (not hang), proving the
|
||||
// key was released and the entry dropped.
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
defer close(done)
|
||||
resp, err := rc.Do("key", func() (*CoalescedResponse, error) {
|
||||
return &CoalescedResponse{StatusCode: 200}, nil
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, 200, resp.StatusCode)
|
||||
}()
|
||||
select {
|
||||
case <-done:
|
||||
case <-time.After(3 * time.Second):
|
||||
t.Fatal("coalescer key still blocked after primary fn() panic")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user