Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| go func() { | ||
| ticker := time.NewTicker(5 * time.Minute) | ||
| defer ticker.Stop() | ||
| for range ticker.C { | ||
| mu.Lock() | ||
| for ip, e := range limiters { | ||
| if time.Since(e.lastSeen) > 10*time.Minute { | ||
| delete(limiters, ip) | ||
| } | ||
| } | ||
| mu.Unlock() | ||
| } | ||
| }() |
There was a problem hiding this comment.
🟡 Goroutine leak: rate limiter cleanup goroutine cannot be stopped if Start() fails or is never called
The rateLimit function at api/rpc/middleware.go:42 spawns a background goroutine during NewServer (via newHandlerStack at api/rpc/server.go:122). The only way to stop this goroutine is by canceling the context, which happens in Stop() at api/rpc/server.go:214. However, Stop() guards cancelFunc() behind s.started.CompareAndSwap(true, false) — if Start() was never called or failed (e.g., port already in use at api/rpc/server.go:183-186), the started flag remains false, the CompareAndSwap fails, and the method returns early at line 212 without ever calling cancelFunc(). This leaks the background goroutine (and its ticker). In the fx lifecycle (nodebuilder/rpc/module.go:21-26), if Start fails, fx does not call Stop for that component, so the goroutine is never cleaned up.
Was this helpful? React with 👍 or 👎 to provide feedback.
- Set WithMaxRequestSize(5 MiB) on go-jsonrpc server (down from 100 MiB default) - Add http.Server timeouts: ReadTimeout, WriteTimeout, IdleTimeout, MaxHeaderBytes - Add per-IP rate limiting middleware (100 req/s sustained, 200 burst) - Add concurrent connection limiting middleware (500 max) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wondertan
force-pushed
the
feat/rpc-server-hardening
branch
from
e3f7e12 to
2d78b9b
Compare
1 participant
Those are not configurable atm and we could add them to configs if requested.