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

crypto/tls: avoid creating handshake context on each read/write #52274

Closed
wants to merge 1 commit into from
Closed

crypto/tls: avoid creating handshake context on each read/write #52274

wants to merge 1 commit into from

Conversation

moredure
Copy link
Contributor

@moredure moredure commented Apr 11, 2022

handshakeCtx and cancel function always allocated on read and write operation not depending on whether the handshake is completed

@gopherbot
Copy link
Contributor

This PR (HEAD: c6bf1a2) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/399498 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@moredure moredure changed the title crypto/tls: avoid creating handshake context on each write crypto/tls: avoid creating handshake context on each read/write Apr 11, 2022
@dt
Copy link
Contributor

dt commented Apr 13, 2022

I believe there is a similar CL here: https://golang.org/cl/379034

@moredure
Copy link
Contributor Author

moredure commented Apr 13, 2022

@dt, I guess so, but I just moved the part of handshake completion/error check to the top, instead of checking it's completion in two places in a single Handshake call.
Also my PR kind of preventing simultaneous contexts/goroutines to be allocated in case of Two concurrent handshakes, so it's kind of more safe I guess.
Anyway I want to get rid of allocations ASAP) without breaking anything, that's why I made this pull request, maybe to show that's a problem and not only for a single person. Will be cool if it will be backported.
I also have a solution to get rid of context allocation itself in case of it won't be used in Handshake process by callbacks functions like GetClientCertificate, etc, but first things first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants