This is an automated email from the ASF dual-hosted git repository.
yuxuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git
The following commit(s) were added to refs/heads/master by this push:
new dda8054 THRIFT-5324: Create new req buffer for every http request
dda8054 is described below
commit dda80547b10d698784713eb62a04f6f42eae107b
Author: Yuxuan 'fishy' Wang <[email protected]>
AuthorDate: Tue Dec 15 17:56:15 2020 -0800
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 19c63a9..26e52b3 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)
}
