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

@@ -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) {

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") })