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.
This commit is contained in:
James Pettyjohn 2018-09-30 19:49:39 -07:00 committed by Bo-Yi Wu
parent 91a4459dd2
commit e9f187f60a
2 changed files with 24 additions and 1 deletions

1
gin.go
View File

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

View File

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