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

Suspend request stream writes before the RPC is ready #1411

Merged
merged 2 commits into from
May 25, 2022

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented May 24, 2022

Motivation:

Writes on the request stream should suspend before the RPC is ready.
That's not the case right now.

Modifications:

  • Allow the writability of a request stream writer to be specified when
    initialized and have clients default to not writable.
  • Plumb through an onStart callback which is triggered on the
    channelActive of the HTTP/2 stream which toggles the writability.

Result:

Attempting to write on a request stream before the underlying http/2
stream is ready will suspend.

Motivation:

Writes on the request stream should suspend before the RPC is ready.
That's not the case right now.

Modifications:

- Allow the writability of a request stream writer to be specified when
  initialized and have clients default to not writable.
- Plumb through an `onStart` callback which is triggered on the
  `channelActive` of the HTTP/2 stream which toggles the writability.

Result:

Attempting to write on a request stream before the underlying http/2
stream is ready will suspend.
@glbrntt glbrntt requested a review from fabianfett May 24, 2022 15:22
@glbrntt glbrntt merged commit 8c5a8af into grpc:1.7.1-async-await May 25, 2022
@glbrntt glbrntt deleted the gb-no-writes-before-start branch May 25, 2022 12:23
try self.pool.close().wait()
self.pool = nil
if self.pool != nil {
try self.pool.close().wait()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there an async equivalent for this?

try self.server.close().wait()
self.server = nil
if self.server != nil {
try self.server.close().wait()
Copy link
Collaborator

Choose a reason for hiding this comment

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

async equivalent?

@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Jun 13, 2022
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.

3 participants