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

credentials: close tls.Conn on failure #3300

Merged
merged 1 commit into from
Jan 10, 2020
Merged

Conversation

ZhenLian
Copy link
Contributor

@ZhenLian ZhenLian commented Jan 7, 2020

No description provided.

Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

These are probably not strictly necessary, as it doesn't look like a tls.Client has any state that requires cleanup, and gRPC already closes the net.Conn if the handshaker fails. But, this is probably best practice (maybe? - the tls package doesn't provide guidance on when a tls.Conn should/must be closed) and double-closing the net.Conn isn't illegal so: LGTM. Thanks!

@ZhenLian
Copy link
Contributor Author

@dfawley
You are welcome, Doug! Feel free to merge this PR or close it if you find it inappropriate later on :-)

@dfawley dfawley changed the title Close Conn When Failure or Not Needed credentials: close tls.Conn on failure Jan 10, 2020
@dfawley dfawley merged commit 69baa3f into grpc:master Jan 10, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2021
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.

2 participants