From 5cc05e9119ce4ad86d15f80dd4cc748606fe0883 Mon Sep 17 00:00:00 2001 From: James Pettyjohn Date: Sun, 30 Sep 2018 19:49:39 -0700 Subject: [PATCH] removed use of sync.pool from HandleContext and added test coverage (#1565) As per #1230 there is an issue when using HandleContext where the context of the request is returned to the context sync.Pool before the parent request has finished, causing context to be used in a non-thread safe manner. I've removed the bug by not entering the context back in the pool and leaving that to ServeHTTP. There was no test coverage for this function so I've also added the test to cover it. As the bug only happens when there are concurrent requests, the tests issues hundreds of concurrent requests. Without the bug fixed the tests do consistently recreate the error. --- gin.go | 1 - gin_integration_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/gin.go b/gin.go index 11f6d94a..92c24ba2 100644 --- a/gin.go +++ b/gin.go @@ -336,7 +336,6 @@ func (engine *Engine) ServeHTTP(w http.ResponseWriter, req *http.Request) { func (engine *Engine) HandleContext(c *Context) { c.reset() engine.handleHTTPRequest(c) - engine.pool.Put(c) } func (engine *Engine) handleHTTPRequest(c *Context) { diff --git a/gin_integration_test.go b/gin_integration_test.go index 52f78842..12a943b0 100644 --- a/gin_integration_test.go +++ b/gin_integration_test.go @@ -12,6 +12,7 @@ import ( "net/http" "net/http/httptest" "os" + "sync" "testing" "time" @@ -119,6 +120,29 @@ func TestWithHttptestWithAutoSelectedPort(t *testing.T) { testRequest(t, ts.URL+"/example") } +func TestConcurrentHandleContext(t *testing.T) { + router := New() + router.GET("/", func(c *Context) { + c.Request.URL.Path = "/example" + router.HandleContext(c) + }) + router.GET("/example", func(c *Context) { c.String(http.StatusOK, "it worked") }) + + ts := httptest.NewServer(router) + defer ts.Close() + + var wg sync.WaitGroup + iterations := 200 + wg.Add(iterations) + for i := 0; i < iterations; i++ { + go func() { + testRequest(t, ts.URL+"/") + wg.Done() + }() + } + wg.Wait() +} + // func TestWithHttptestWithSpecifiedPort(t *testing.T) { // router := New() // router.GET("/example", func(c *Context) { c.String(http.StatusOK, "it worked") })