Skip to content

Fix accept_stream being called post-channel deletion#5560

Merged
vjpai merged 5 commits intogrpc:masterfrom
ctiller:monkey_man
Mar 4, 2016
Merged

Fix accept_stream being called post-channel deletion#5560
vjpai merged 5 commits intogrpc:masterfrom
ctiller:monkey_man

Conversation

@ctiller
Copy link
Copy Markdown
Member

@ctiller ctiller commented Mar 3, 2016

  • Have the server clear the accept_stream callback prior to destroying
    the channel (required a small transport op protocol change)
  • Have the transport not enact transport ops until parsing is completed
    (prevents accept_stream from disappearing mid-parse)

- Have the server clear the accept_stream callback prior to destroying
  the channel (required a small transport op protocol change)
- Have the transport not enact transport ops until parsing is completed
  (prevents accept_stream from disappearing mid-parse)
@@ -139,8 +139,10 @@ typedef struct grpc_transport_op {
gpr_slice *goaway_message;
/** set the callback for accepting new streams;
this is a permanent callback, unlike the other one-shot closures */
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the docstring seems misplaced with the introduction of the boolean guard (which isn't mentioned).

@ctiller
Copy link
Copy Markdown
Member Author

ctiller commented Mar 4, 2016

Updated documentation. Refined post-parse conditional to something that I know is safe.

@vjpai
Copy link
Copy Markdown
Contributor

vjpai commented Mar 4, 2016

LGTM after the changes. Will merge.

vjpai added a commit that referenced this pull request Mar 4, 2016
Fix accept_stream being called post-channel deletion
@vjpai vjpai merged commit da0d46e into grpc:master Mar 4, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants