Skip to content

Commit

Permalink
THRIFT-5324: Create new req buffer for every http request
Browse files Browse the repository at this point in the history
Client: go

The fix in #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.
  • Loading branch information
fishy committed Dec 16, 2020
1 parent 1e243a7 commit dda8054
Showing 1 changed file with 5 additions and 9 deletions.
14 changes: 5 additions & 9 deletions lib/go/thrift/http_client.go
Expand Up @@ -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)
}
Expand Down

0 comments on commit dda8054

Please sign in to comment.