Skip to content

Delay closing until the next loop tick (#1326) #1335

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

Merged
merged 1 commit into from
Dec 20, 2021

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Dec 17, 2021

Motivation:

When the idle handler determines that the channel needs to be closed
(becuase the connection is no longer required) it does so on the current
event loop tick. Closing immediately means that some events which are
already scheduled to run on the current tick may be dropped unexpectedly.

Modifications:

  • Execute the channel close on the next event-loop tick.
  • Fixup a test which now requires an extra loop run().

Result:

Shutdown is a little more graceful.

(cherry picked from commit c43be58)

Motivation:

When the idle handler determines that the channel needs to be closed
(becuase the connection is no longer required) it does so on the current
event loop tick. Closing immediately means that some events which are
already scheduled to run on the current tick may be dropped unexpectedly.

Modifications:

- Execute the channel close on the next event-loop tick.
- Fixup a test which now requires an extra loop `run()`.

Result:

Shutdown is a little more graceful.

(cherry picked from commit c43be58)
@glbrntt glbrntt requested a review from fabianfett December 17, 2021 12:38
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Dec 17, 2021
Copy link
Collaborator

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

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

Great fix!

@glbrntt glbrntt merged commit 61f63a7 into grpc:1.6.0-async-await Dec 20, 2021
@glbrntt glbrntt deleted the gb-cp branch December 20, 2021 13:07
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