From 2ecd7bac8b4c07c8e3d963353f98e11965f8b0f1 Mon Sep 17 00:00:00 2001 From: AdamMagued Date: Sat, 10 Oct 2026 03:41:50 +0300 Subject: [PATCH] fix(runner): prevent data race during rate limiter initialization Avoid copying ratelimit.Limiter value when assigning runner.ratelimiter. Store a pointer (*ratelimit.Limiter) to prevent race conditions on atomic counters and check for nil on Close and Take invocations. Signed-off-by: AdamMagued --- runner/runner.go | 28 ++++++++++++------- runner/runner_test.go | 63 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 9 deletions(-) diff --git a/runner/runner.go b/runner/runner.go index ea0cac3f..9172310b 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -90,7 +90,7 @@ type Runner struct { hm *hybrid.HybridMap excludeCdn bool stats clistats.StatisticsClient - ratelimiter ratelimit.Limiter + ratelimiter *ratelimit.Limiter HostErrorsCache gcache.Cache[string, int] browser *Browser ditClassifier *dit.Classifier @@ -416,11 +416,11 @@ func New(options *Options) (*Runner, error) { runner.hm = hm if options.RateLimitMinute > 0 { - runner.ratelimiter = *ratelimit.New(context.Background(), uint(options.RateLimitMinute), time.Minute) + runner.ratelimiter = ratelimit.New(context.Background(), uint(options.RateLimitMinute), time.Minute) } else if options.RateLimit > 0 { - runner.ratelimiter = *ratelimit.New(context.Background(), uint(options.RateLimit), time.Second) + runner.ratelimiter = ratelimit.New(context.Background(), uint(options.RateLimit), time.Second) } else { - runner.ratelimiter = *ratelimit.NewUnlimited(context.Background()) + runner.ratelimiter = ratelimit.NewUnlimited(context.Background()) } if options.HostMaxErrors >= 0 { @@ -917,7 +917,9 @@ func (r *Runner) Close() { // nolint:errcheck // ignore r.hm.Close() r.hp.Dialer.Close() - r.ratelimiter.Stop() + if r.ratelimiter != nil { + r.ratelimiter.Stop() + } if r.options.HostMaxErrors >= 0 { r.HostErrorsCache.Purge() @@ -1927,7 +1929,9 @@ retry: req.Body = nil } - r.ratelimiter.Take() + if r.ratelimiter != nil { + r.ratelimiter.Take() + } // with rawhttp we should say to the server to close the connection, otherwise it will remain open if scanopts.Unsafe { @@ -2217,7 +2221,9 @@ retry: // check for virtual host isvhost := false if scanopts.VHost { - r.ratelimiter.Take() + if r.ratelimiter != nil { + r.ratelimiter.Take() + } isvhost, _ = hp.IsVirtualHost(req, httpx.UnsafeOptions{}) if isvhost { builder.WriteString(" [vhost]") @@ -2238,7 +2244,9 @@ retry: port = p } } - r.ratelimiter.Take() + if r.ratelimiter != nil { + r.ratelimiter.Take() + } pipeline = hp.SupportPipeline(protocol, method, URL.Host, port) if pipeline { builder.WriteString(" [pipeline]") @@ -2251,7 +2259,9 @@ retry: var http2 bool // if requested probes for http2 if scanopts.HTTP2Probe { - r.ratelimiter.Take() + if r.ratelimiter != nil { + r.ratelimiter.Take() + } http2 = hp.SupportHTTP2(protocol, method, URL.String()) if http2 { builder.WriteString(" [http2]") diff --git a/runner/runner_test.go b/runner/runner_test.go index 3f9d5e55..6a27746a 100644 --- a/runner/runner_test.go +++ b/runner/runner_test.go @@ -1166,3 +1166,66 @@ func TestHandshakeThenCloseKeepsPlainResult(t *testing.T) { require.EqualValues(t, 1, atomic.LoadInt64(&tlsHandshakes), "the failed HTTPS request must complete its TLS handshake first") } + +func TestRunner_RateLimiter_Initialization(t *testing.T) { + t.Run("unlimited rate limiter initialization and concurrency", func(t *testing.T) { + const numGoroutines = 10 + var wg sync.WaitGroup + wg.Add(numGoroutines) + for i := 0; i < numGoroutines; i++ { + go func() { + defer wg.Done() + r, err := New(&Options{}) + require.NoError(t, err) + require.NotNil(t, r.ratelimiter) + r.ratelimiter.Take() + r.Close() + }() + } + wg.Wait() + }) + + t.Run("per second rate limiter initialization and concurrency", func(t *testing.T) { + const numGoroutines = 10 + var wg sync.WaitGroup + wg.Add(numGoroutines) + for i := 0; i < numGoroutines; i++ { + go func() { + defer wg.Done() + r, err := New(&Options{RateLimit: 100}) + require.NoError(t, err) + require.NotNil(t, r.ratelimiter) + r.ratelimiter.Take() + r.Close() + }() + } + wg.Wait() + }) + + t.Run("per minute rate limiter initialization and concurrency", func(t *testing.T) { + const numGoroutines = 10 + var wg sync.WaitGroup + wg.Add(numGoroutines) + for i := 0; i < numGoroutines; i++ { + go func() { + defer wg.Done() + r, err := New(&Options{RateLimitMinute: 600}) + require.NoError(t, err) + require.NotNil(t, r.ratelimiter) + r.ratelimiter.Take() + r.Close() + }() + } + wg.Wait() + }) + + t.Run("close on nil ratelimiter does not panic", func(t *testing.T) { + r, err := New(&Options{}) + require.NoError(t, err) + r.ratelimiter = nil + require.NotPanics(t, func() { + r.Close() + }) + }) +} +