fix: concurrent-safe route registration with mutex synchronization

- Add sync.RWMutex to Engine struct for thread-safe access
- Protect addRoute() with Lock() for exclusive write access
- Protect Routes() with RLock() for concurrent read access
- Add concurrent test to verify no data race
- Add tests for serveError function coverage

Fixes #4457
This commit is contained in:
mehrdadbn9 2026-02-13 12:15:54 +03:30
parent f5c267d2f8
commit 42008a7ca6
2 changed files with 62 additions and 0 deletions

8
gin.go
View File

@ -96,6 +96,9 @@ type Engine struct {
// (used for routing HTTP requests) happens only once, even if called multiple times concurrently.
routeTreesUpdated sync.Once
// mu protects the route trees during concurrent access.
mu sync.RWMutex
// RedirectTrailingSlash enables automatic redirection if the current route can't be matched but a
// handler for the path with (without) the trailing slash exists.
// For example if /foo/ is requested but a route only exists for /foo, the
@ -368,6 +371,9 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) {
debugPrintRoute(method, path, handlers)
engine.mu.Lock()
defer engine.mu.Unlock()
root := engine.trees.get(method)
if root == nil {
root = new(node)
@ -388,6 +394,8 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) {
// Routes returns a slice of registered routes, including some useful information, such as:
// the http method, path, and the handler name.
func (engine *Engine) Routes() (routes RoutesInfo) {
engine.mu.RLock()
defer engine.mu.RUnlock()
for _, tree := range engine.trees {
routes = iterate("", tree.method, routes, tree.root)
}

View File

@ -15,6 +15,7 @@ import (
"reflect"
"strconv"
"strings"
"sync"
"sync/atomic"
"testing"
"time"
@ -1084,3 +1085,56 @@ func TestUpdateRouteTreesCalledOnce(t *testing.T) {
assert.Equal(t, "ok", w.Body.String())
}
}
func TestConcurrentAddRouteAndRoutes(t *testing.T) {
router := New()
var wg sync.WaitGroup
for i := 0; i < 100; i++ {
wg.Add(2)
go func(i int) {
defer wg.Done()
router.GET(fmt.Sprintf("/route%d", i), func(c *Context) {
c.String(http.StatusOK, "ok")
})
}(i)
go func(i int) {
defer wg.Done()
_ = router.Routes()
}(i)
}
wg.Wait()
assert.Len(t, router.Routes(), 100)
}
func TestServeErrorWritten(t *testing.T) {
router := New()
router.GET("/", func(c *Context) {
c.Status(http.StatusInternalServerError)
_, _ = c.Writer.Write([]byte("custom error"))
})
router.NoRoute(func(c *Context) {
c.Status(http.StatusNotFound)
})
w := httptest.NewRecorder()
req, _ := http.NewRequest(http.MethodGet, "/notfound", nil)
router.ServeHTTP(w, req)
assert.Equal(t, http.StatusNotFound, w.Code)
}
func TestServeErrorStatusMismatch(t *testing.T) {
router := New()
router.HandleMethodNotAllowed = true
router.NoMethod(func(c *Context) {
c.Status(http.StatusForbidden)
})
router.GET("/exists", func(c *Context) {
c.String(http.StatusOK, "ok")
})
w := httptest.NewRecorder()
req, _ := http.NewRequest(http.MethodPost, "/exists", nil)
router.ServeHTTP(w, req)
assert.Equal(t, http.StatusForbidden, w.Code)
}