Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client: fix race between client-side stream cancellation and compressed server data arriving #3054

Merged
merged 3 commits into from Oct 1, 2019

Commits on Sep 27, 2019

  1. client: fix race between client-side stream cancellation and compress…

    …ed server data arriving
    
    `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:
    
    1. RPC starts.
    
    2. Client cancels RPC.
    
    3. `RecvCompress` tells `ClientStream.Recv` that compression used is "" because
       of the context error.  `as.decomp` is left nil, because there is no
       compressor to look up in the registry.
    
    4. Server's header and first message hit client.
    
    5. Client sees the header and message and allows grpc's stream to see them.
       (We only provide context errors if we need to block.)
    
    6. Client performs a successful `Read` on the stream, receiving the gzipped
       payload, then checks `as.decomp`.
    
    7. 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 get
       this from the now-received header.  This leads to the confusing message
       about how "gzip" is not installed even though it is.
    
    This change makes `RecvCompress` return an error instead of empty string (which
    is also a valid response), and makes `ClientStream.Recv` return an error when
    this happens.  This effectively terminates the stream and prevents subsequent
    operations.  This results in 10k/10k passing runs (previously observed failure
    rate of ~1/100).
    dfawley committed Sep 27, 2019
    Copy the full SHA
    d8c668b View commit details
    Browse the repository at this point in the history

Commits on Sep 30, 2019

  1. different approach

    dfawley committed Sep 30, 2019
    Copy the full SHA
    9ffa82e View commit details
    Browse the repository at this point in the history

Commits on Oct 1, 2019

  1. review cleanups

    dfawley committed Oct 1, 2019
    Copy the full SHA
    eef7557 View commit details
    Browse the repository at this point in the history