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

Extend lifetime of client interceptor pipeline #1265

Merged
merged 6 commits into from
Sep 16, 2021

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Sep 15, 2021

Motivation:

A client call (i.e. the object the user holds) may live longer than the
transport associated with it (roughly speaking, the http/2 stream channel). An
example of this is when interceptors are use to retry and RPC and
redirect responses back to the original call.

However, the interceptor pipeline is held by the transport and is
currently set to nil when the transport is removed from the channel.
This means events invoked from the call object (such as cancellation)
which go via the transport (holding the interceptor pipeline) are
incorrectly failed.

Modifications:

  • Have the client interceptor pipeline break the ref cycle between the
    transport and itself when the interceptor pipeline closes rather than
    when the transport is closed
  • Emit a cancellation status rater than error on cancellation
  • Update the ordering of when close is called in the interceptor
    pipeline.
  • Add and update tests

Result:

"sub"-RPCs may be cancelled.

Motivation:

A client call (i.e. the object the user holds) may live longer than the
transport associated with it (roughly speaking, the http/2 stream channel). An
example of this is when interceptors are use to retry and RPC and
redirect responses back to the original call.

However, the interceptor pipeline is held by the transport and is
currently set to nil when the transport is removed from the channel.
This means events invoked from the call object (such as cancellation)
which go via the transport (holding the interceptor pipeline) are
incorrectly failed.

Modifications:

- Have the client interceptor pipeline break the ref cycle between the
  transport and itself when the interceptor pipeline closes rather than
  when the transport is closed
- Emit a cancellation status rater than error on cancellation
- Update the ordering of when close is called in the interceptor
  pipeline.
- Add and update tests

Result:

"sub"-RPCs may be cancelled.
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Sep 15, 2021
@glbrntt glbrntt requested a review from fabianfett September 15, 2021 12:52
Copy link
Collaborator

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice change.

@glbrntt glbrntt merged commit 03010c7 into grpc:main Sep 16, 2021
@glbrntt glbrntt deleted the gb-interceptor-pipeline-life branch September 16, 2021 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants