From dda80547b10d698784713eb62a04f6f42eae107b Mon Sep 17 00:00:00 2001 From: Yuxuan 'fishy' Wang Date: Tue, 15 Dec 2020 17:56:15 -0800 Subject: [PATCH] THRIFT-5324: Create new req buffer for every http request Client: go The fix in https://github.com/apache/thrift/pull/2293 doesn't work for go1.10.8 due to the possibility of data races. This exposes a bigger, underlying issue regarding the ownership of the request buffer in THttpClient between THttpClient itself and the http request it creates. Instead of reset and reuse the same buffer, always give up the ownership of it and create a new buffer after each Flush call. --- lib/go/thrift/http_client.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/go/thrift/http_client.go b/lib/go/thrift/http_client.go index 19c63a98512..26e52b387d7 100644 --- a/lib/go/thrift/http_client.go +++ b/lib/go/thrift/http_client.go @@ -197,15 +197,11 @@ func (p *THttpClient) Flush(ctx context.Context) error { // Close any previous response body to avoid leaking connections. p.closeResponse() - // Request might not have been fully read by http client. - // Reset so we don't send the remains on next call. - defer func() { - if p.requestBuffer != nil { - p.requestBuffer.Reset() - } - }() - - req, err := http.NewRequest("POST", p.url.String(), p.requestBuffer) + // Give up the ownership of the current request buffer to http request, + // and create a new buffer for the next request. + buf := p.requestBuffer + p.requestBuffer = new(bytes.Buffer) + req, err := http.NewRequest("POST", p.url.String(), buf) if err != nil { return NewTTransportExceptionFromError(err) }