Skip to content

Commit d51b862

Browse files
committed
Transport: Fix memory leak when retrying 5xx responses
To prevent a memory leak when a request with a retryable status code is executed (502, etc), drain and close the body before the request is retried. Fixes #159 (cherry picked from commit c50a178)
1 parent 26ee26e commit d51b862

File tree

2 files changed

+56
-2
lines changed

2 files changed

+56
-2
lines changed

estransport/estransport.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,9 @@ func (c *Client) Perform(req *http.Request) (*http.Response, error) {
226226

227227
for i := 1; i <= c.maxRetries; i++ {
228228
var (
229-
conn *Connection
230-
shouldRetry bool
229+
conn *Connection
230+
shouldRetry bool
231+
shouldCloseBody bool
231232
)
232233

233234
// Get connection from the pool
@@ -308,6 +309,7 @@ func (c *Client) Perform(req *http.Request) (*http.Response, error) {
308309
for _, code := range c.retryOnStatus {
309310
if res.StatusCode == code {
310311
shouldRetry = true
312+
shouldCloseBody = true
311313
}
312314
}
313315
}
@@ -317,6 +319,14 @@ func (c *Client) Perform(req *http.Request) (*http.Response, error) {
317319
break
318320
}
319321

322+
// Drain and close body when retrying after response
323+
if shouldCloseBody && i < c.maxRetries {
324+
if res.Body != nil {
325+
io.Copy(ioutil.Discard, res.Body)
326+
res.Body.Close()
327+
}
328+
}
329+
320330
// Delay the retry if a backoff function is configured
321331
if c.retryBackoff != nil {
322332
time.Sleep(c.retryBackoff(i))

estransport/estransport_internal_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,50 @@ func TestTransportPerformRetries(t *testing.T) {
501501
}
502502
})
503503

504+
t.Run("Close response body for a 5xx response", func(t *testing.T) {
505+
var (
506+
i int
507+
numReqs = 5
508+
)
509+
510+
u, _ := url.Parse("http://foo.bar")
511+
tp, _ := New(Config{
512+
URLs: []*url.URL{u, u, u},
513+
MaxRetries: numReqs,
514+
Transport: &mockTransp{
515+
RoundTripFunc: func(req *http.Request) (*http.Response, error) {
516+
i++
517+
fmt.Printf("Request #%d", i)
518+
fmt.Print(": 502\n")
519+
body := ioutil.NopCloser(strings.NewReader(`MOCK`))
520+
return &http.Response{StatusCode: 502, Body: body}, nil
521+
},
522+
}})
523+
524+
req, _ := http.NewRequest("GET", "/", nil)
525+
526+
res, err := tp.Perform(req)
527+
528+
if err != nil {
529+
t.Fatalf("Unexpected error: %s", err)
530+
}
531+
532+
if i != numReqs {
533+
t.Errorf("Unexpected number of requests, want=%d, got=%d", numReqs, i)
534+
}
535+
536+
if res.StatusCode != 502 {
537+
t.Errorf("Unexpected response: %+v", res)
538+
}
539+
540+
resBody, _ := ioutil.ReadAll(res.Body)
541+
res.Body.Close()
542+
543+
if string(resBody) != "MOCK" {
544+
t.Errorf("Unexpected body, want=MOCK, got=%s", resBody)
545+
}
546+
})
547+
504548
t.Run("Retry request and return error when max retries exhausted", func(t *testing.T) {
505549
var (
506550
i int

0 commit comments

Comments
 (0)