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

Move idle handler state to closed on channelInactive #953

Merged
merged 1 commit into from
Aug 25, 2020

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Aug 25, 2020

Motivation:

Related to #949.

It's possible for the client's connection to be dropped, and for the
connection state to become idle, prior to the scheduled close from the
keepalive handler firing. As the idle handler never updates its state to
'closed' in 'channelInactive', when the scheduled close event is picked
up in the idle handler it effectively ask the connection manager to
double-idle, hitting a precondition failure.

Modifications:

  • Move the 'GRPCIdleHandler' to '.closed' in 'channelInactive'
  • Cancel scheduled pings and timeouts on 'channelInactive' in the
    'GRPCKeepaliveHandlers'

Result:

  • We avoid another path to a precondition failure

@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Aug 25, 2020
@glbrntt glbrntt requested a review from Lukasa August 25, 2020 09:08
Motivation:

Related to grpc#949.

It's possible for the client's connection to be dropped, and for the
connection state to become idle, prior to the scheduled close from the
keepalive handler firing. As the idle handler never updates its state to
'closed' in 'channelInactive', when the scheduled close event is picked
up in the idle handler it effectively ask the connection manager to
double-idle, hitting a precondition failure.

Modifications:

- Move the 'GRPCIdleHandler' to '.closed' in 'channelInactive'
- Cancel scheduled pings and timeouts on 'channelInactive' in the
  'GRPCKeepaliveHandlers'

Result:

- We avoid another path to a precondition failure
@glbrntt glbrntt merged commit cc1f91f into grpc:main Aug 25, 2020
@glbrntt glbrntt deleted the gb-keepalive branch August 25, 2020 10:39
@glbrntt glbrntt mentioned this pull request Aug 25, 2020
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.

2 participants