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

Send go away frame on client transport shutdown #4389

Closed
wants to merge 2 commits into from

Conversation

zasweq
Copy link
Contributor

@zasweq zasweq commented May 5, 2021

Fixes issue #460.

@zasweq zasweq requested a review from dfawley May 5, 2021 22:16
@zasweq zasweq added fixit Type: Behavior Change Behavior changes not categorized as bugs labels May 5, 2021
@zasweq zasweq added this to the 1.38 Release milestone May 5, 2021
@@ -861,6 +861,10 @@ func (t *http2Client) Close() error {
}
t.mu.Unlock()
t.controlBuf.finish()
Copy link
Member

Choose a reason for hiding this comment

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

It's possible this exits while loopywriter is about to write to the connection, I believe.

You may want to add something to loopyWriter.run() that closes a channel / fires an event when it exits via a defer, and then block on that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is what I figured. Will get to this after finishing up Easwar's comments on #4387.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add an event.

@zasweq zasweq force-pushed the send-go-away-frame branch from 0743cb5 to 5129a7b Compare May 6, 2021 13:20
@dfawley dfawley assigned zasweq and unassigned dfawley May 6, 2021
@menghanl menghanl modified the milestones: 1.38 Release, 1.39 Release May 11, 2021
@zasweq zasweq closed this May 12, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
fixit Type: Behavior Change Behavior changes not categorized as bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants