thrift git commit: THRIFT-3430 Go THttpClient does not read HTTP response body to completion when closing Client: Go Patch: Justin Larrabee

[je...@apache.org]

Repository: thrift
Updated Branches:
  refs/heads/master 2faac5afa -> bbb8f5c46


THRIFT-3430 Go THttpClient does not read HTTP response body to completion when closing
Client: Go
Patch: Justin Larrabee

This closes #703


Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/bbb8f5c4
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/bbb8f5c4
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/bbb8f5c4

Branch: refs/heads/master
Commit: bbb8f5c46b8e15511b7923c5e363000fed65ed34
Parents: 2faac5a
Author: Justin Larrabee <ju...@backflipstudios.com>
Authored: Wed Nov 18 11:33:31 2015 -0700
Committer: Jens Geyer <je...@apache.org>
Committed: Thu Nov 19 21:34:43 2015 +0100

----------------------------------------------------------------------
 lib/go/thrift/http_client.go | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/thrift/blob/bbb8f5c4/lib/go/thrift/http_client.go
----------------------------------------------------------------------
diff --git a/lib/go/thrift/http_client.go b/lib/go/thrift/http_client.go
index 16f1cdd..45720c9 100644
--- a/lib/go/thrift/http_client.go
+++ b/lib/go/thrift/http_client.go
@@ -22,6 +22,7 @@ package thrift
 import (
 	"bytes"
 	"io"
+	"io/ioutil"
 	"net/http"
 	"net/url"
 	"strconv"
@@ -168,6 +169,13 @@ func (p *THttpClient) IsOpen() bool {
 func (p *THttpClient) closeResponse() error {
 	var err error
 	if p.response != nil && p.response.Body != nil {
+		// The docs specify that if keepalive is enabled and the response body is not
+		// read to completion the connection will never be returned to the pool and
+		// reused. Errors are being ignored here because if the connection is invalid
+		// and this fails for some reason, the Close() method will do any remaining
+		// cleanup.
+		io.Copy(ioutil.Discard, p.response.Body)
+
 		err = p.response.Body.Close()
 	}
 
@@ -226,8 +234,11 @@ func (p *THttpClient) Flush() error {
 		return NewTTransportExceptionFromError(err)
 	}
 	if response.StatusCode != http.StatusOK {
-		// Close the response to avoid leaking file descriptors.
-		response.Body.Close()
+		// Close the response to avoid leaking file descriptors. closeResponse does
+		// more than just call Close(), so temporarily assign it and reuse the logic.
+		p.response = response
+		p.closeResponse()
+
 		// TODO(pomack) log bad response
 		return NewTTransportException(UNKNOWN_TRANSPORT_EXCEPTION, "HTTP Response code: "+strconv.Itoa(response.StatusCode))
 	}