client: fix race between client-side stream cancellation and compressed server data arriving #3054
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 #3053
transport/Stream.RecvCompress
returns what the header contains, if present,or empty string if a context error occurs. However, it "prefers" the header
data even if there is a context error, to prevent a related race. What happens
here is:
RPC starts.
Client cancels RPC.
RecvCompress
tellsClientStream.Recv
that compression used is "" becauseof the context error.
as.decomp
is left nil, because there is nocompressor to look up in the registry.
Server's header and first message hit client.
Client sees the header and message and allows grpc's stream to see them.
(We only provide context errors if we need to block.)
Client performs a successful
Read
on the stream, receiving the gzippedpayload, then checks
as.decomp
.We have no decompressor but the payload has a bit set indicating the message
is compressed, so this is an error. However, when forming the error string,
RecvCompress
now returns "gzip" because it doesn't need to block to getthis from the now-received header. This leads to the confusing message
about how "gzip" is not installed even though it is.
This change makes
waitOnHeader
close the stream when context cancellation happens.Then
RecvCompress
uses whatever value is present in the stream at that time, whichcan no longer change because the stream is closed. Also, this will be in sync with
the messages on the stream - if there are any messages present, the headers must
have been processed first, and
RecvCompress
will contain the proper value.