Skip to content

Commit

Permalink
Always set stream when connecting HttpUrlConnection
Browse files Browse the repository at this point in the history
That way we will always close the stream in cleanup(). Prior to this change, when cancel was called, stream was never set to non-null and therefore never closed in cleanup. Because disconnecting the stream doesn’t seem to close the contained ResponseBody object, failing to obtain and then close the stream results in leaked objects.

Progress towards #2352
  • Loading branch information
sjudd committed Sep 7, 2017
1 parent 7f62597 commit 73a8054
Showing 1 changed file with 4 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ private InputStream loadDataWithRedirects(URL url, int redirects, URL lastUrl,

// Connect explicitly to avoid errors in decoders if connection fails.
urlConnection.connect();
// Set the stream so that it's closed in cleanup to avoid resource leaks. See #2352.
stream = urlConnection.getInputStream();
if (isCancelled) {
return null;
}
Expand All @@ -114,9 +116,7 @@ private InputStream loadDataWithRedirects(URL url, int redirects, URL lastUrl,
URL redirectUrl = new URL(url, redirectUrlString);
// Closing the stream specifically is required to avoid leaking ResponseBodys in addition
// to disconnecting the url connection below. See #2352.
urlConnection.getInputStream().close();
urlConnection.disconnect();
urlConnection = null;
cleanup();
return loadDataWithRedirects(redirectUrl, redirects + 1, url, headers);
} else if (statusCode == -1) {
throw new HttpException(statusCode);
Expand Down Expand Up @@ -151,6 +151,7 @@ public void cleanup() {
if (urlConnection != null) {
urlConnection.disconnect();
}
urlConnection = null;
}

@Override
Expand Down

0 comments on commit 73a8054

Please sign in to comment.