From 3fef91d3428cf6d80b26dc64478ed1b1809c3275 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Fri, 30 Oct 2026 21:21:59 +0000 Subject: [PATCH] Read the period end of a daily denial under the counter lock --- limit/service.go | 3 ++- limit/service_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/limit/service.go b/limit/service.go index dde68cea..66b46738 100644 --- a/limit/service.go +++ b/limit/service.go @@ -125,11 +125,12 @@ func (s *Service) Allow(key string, rule Rule, n int64) Decision { c := cs.counterNoLock(key, start) total := c.base[i] + c.unflushed[i] if rule.Daily > 0 && total+n > rule.Daily { + end := time.Unix(c.periodStart+daySeconds, 0) // Read under the lock: another call may roll the counter over cs.mu.Unlock() if rule.Burst > 0 { s.refundTokens(key, rule.Kind, n) } - return Decision{RetryAfter: time.Unix(c.periodStart+daySeconds, 0).Sub(dbNow)} + return Decision{RetryAfter: end.Sub(dbNow)} } c.unflushed[i] += n if rule.Threshold == 0 || total+n >= rule.Threshold { diff --git a/limit/service_test.go b/limit/service_test.go index 904e75ec..0300ebae 100644 --- a/limit/service_test.go +++ b/limit/service_test.go @@ -841,3 +841,30 @@ func TestService_Persist_LeftoverAfterRollover(t *testing.T) { }, usageRowOf(t, store, "b")) }) } + +func TestService_Allow_DenyRacesRollover(t *testing.T) { + // A daily denial computes its retry time from the counter it decided on, while other calls + // roll the counter over; run with -race + var ticks atomic.Int64 + start := time.Date(2026, 10, 25, 12, 0, 0, 0, time.UTC) + s := newService(&Config{}, func() time.Time { + return start.Add(time.Duration(ticks.Add(1)) * time.Hour) + }) + rule := Rule{ + Kind: KindMessages, + Daily: 1, + } + var wg sync.WaitGroup + for g := 0; g < 8; g++ { + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 3000; i++ { + if d := s.Allow("k", rule, 1); !d.OK { + require.Greater(t, d.RetryAfter, time.Duration(0)) + } + } + }() + } + wg.Wait() +}