Goroutine Leak in RateLimitMiddleware

[Bhup-GitHUB]

🛠️ Rate Limiter Cleanup Goroutine Never Stops During Graceful Shutdown

❗ Problem Summary

The RateLimitMiddleware starts a background cleanup goroutine inside NewRateLimitMiddleware() (line 49 in internal/restapi/rate_limit_middleware.go). However, this goroutine never stops, even during a graceful shutdown, causing a goroutine leak.

---

🔍 Details

1. Goroutine Starts but Never Ends

// internal/restapi/rate_limit_middleware.go:49
go middleware.cleanup() // Runs forever

This launches a cleanup loop that continues running even after the server shuts down.

---

2. A Stop Method Exists (But Is Never Called)

// internal/restapi/rate_limit_middleware.go:174-179
func (rl *RateLimitMiddleware) Stop() {
    if rl.cleanupTick != nil {
        rl.cleanupTick.Stop()
    }
}

The cleanup ticker can be stopped — but nothing ever calls this method.

---

3. Root Cause

The RestAPI struct only stores middleware handlers, not the actual RateLimitMiddleware instance.

As a result, cmd/api/app.go has no reference to the middleware, so it cannot call Stop() during shutdown.

This prevents graceful cleanup of the goroutine.

---

🚨 Impact

• Goroutine leaks
• Cleanup ticker continues running after shutdown
• Incomplete graceful shutdown
• Unnecessary resource usage

---

✅ Proposed Fix

1. Modify RestAPI to store a reference to the RateLimitMiddleware instance.
2. During shutdown, call rateLimiter.Stop() (similar to gtfsManager.Shutdown()).
3. Ensure the cleanup goroutine exits when the ticker stops.

This ensures a clean, leak-free shutdown.

---

🙋 I Would Love to Work on This!

I'm interested in implementing this fix and improving the shutdown behavior of the rate-limiter middleware.

[Bhup-GitHUB]

Hi @Ahmedhossamdev , could you please review this issue? It looks like it may be critical. If you agree, I can work on a solution and open a PR.

[Ahmedhossamdev]

Good catch! @Bhup-GitHUB

I think one of the solution we could just store the RateLimitMiddleware instance in RestAPI so we can call stop fucntion.

[Bhup-GitHUB]

Hi @Ahmedhossamdev , this patch makes CreateServer return both *http.Server and *restapi.RestAPI so we can call api.Shutdown() during graceful shutdown, but that broke the cmd/api/app_test.go cases that only expect a single return. Are you OK with me updating those tests in this PR, or would you prefer another approach?

[Bhup-GitHUB]

Also, even after wiring the Stop() call, the cleanup goroutine still blocks because time.Ticker.Stop() doesn’t close the channel. Would you like me to add a shutdown signal (context/done channel) so cleanup() can exit, or do you have a different plan in mind?

[Ahmedhossamdev]

Also, even after wiring the Stop() call, the cleanup goroutine still blocks because time.Ticker.Stop() doesn’t close the channel. Would you like me to add a shutdown signal (context/done channel) so cleanup() can exit, or do you have a different plan in mind?

Great idea you could try it.