diff options
author | Yuxuan 'fishy' Wang <yuxuan.wang@reddit.com> | 2020-12-15 17:56:15 -0800 |
---|---|---|
committer | Yuxuan 'fishy' Wang <fishywang@gmail.com> | 2020-12-16 09:27:25 -0800 |
commit | dda80547b10d698784713eb62a04f6f42eae107b (patch) | |
tree | 26d8684bbd15944fbf41e3c89c67436611952477 | |
parent | 1e243a76b186142f551a3cb8419131a468ff54ff (diff) | |
download | thrift-dda80547b10d698784713eb62a04f6f42eae107b.tar.gz |
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.
-rw-r--r-- | lib/go/thrift/http_client.go | 14 |
1 files changed, 5 insertions, 9 deletions
diff --git a/lib/go/thrift/http_client.go b/lib/go/thrift/http_client.go index 19c63a985..26e52b387 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) } |