Fix: abort connection before draining in HttpResponse.disconnect()#2163
Open
Animeshz wants to merge 1 commit into
Open
Fix: abort connection before draining in HttpResponse.disconnect()#2163Animeshz wants to merge 1 commit into
Animeshz wants to merge 1 commit into
Conversation
HttpResponse.disconnect() previously called ignore() (which drains the remaining response body via ContentLengthInputStream.close()) before calling LowLevelHttpResponse.disconnect() (which aborts the socket). This caused callers that close a stream after reading only a prefix of a large response to block for minutes draining bytes they do not need. Fix: call response.disconnect() first to abort the socket, then call ignore() for cleanup. Any IOException from ignore() on an already-dead socket is expected and swallowed. This is the corrected version of googleapis#1315 (reverted in googleapis#1427 because the original lacked the try-catch around ignore(), causing TruncatedChunkException to propagate from disconnect()). Fixes googleapis#1303
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1303
After the revert #1427 was merged, this issue was never looked upon. This issue is causing
fadvice: SEQUENTIAL(default) file reads in GoogleCloudDataproc/hadoop-connectors#1729 to read the file full from the point it was seeked to. Hence for reading 2gb out of 40gb file, after reading the file for 2gb and calling disconnect()/close() we're still draining the rest of 38gb stream due to this.HttpResponse.disconnect() calls ignore() (which drains the remaining response body via ContentLengthInputStream.close()) before calling LowLevelHttpResponse.disconnect() (which aborts the socket). This caused callers that close a stream after reading only a prefix of a large response to block for minutes draining bytes they do not need.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: