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

Simplify managed SNI receive callback use #1186

Merged
merged 2 commits into from
May 26, 2022

Conversation

Wraith2
Copy link
Contributor

@Wraith2 Wraith2 commented Jul 24, 2021

I investigated why there seem to be two ways to set the callbacks which are executed on async reads and writes to packets. It looks like the original method was to pass the callback as a parameter to the function and then later because of the need to support MARS there needed to be a way to store set the callback before that call so it could override the parameter which was made nullable.

This PR removes the ability to pass the callback as part of the call and requires that it always be set by calling SetReceiveCallback. Doing this removes the need to pass the callback as the state parameter to the async call meaning that the packet can be used as state instead which allows the callback to be static because everything needed to finish the call is now part of the packet itself.

@cheenamalhotra cheenamalhotra added the ➕ Code Health Changes related to source code improvements label Aug 20, 2021
@Wraith2
Copy link
Contributor Author

Wraith2 commented Apr 13, 2022

Freshly rebased and the real CI is green, all ready to go.

@DavoudEshtehari DavoudEshtehari added this to the 5.0.0-preview3 milestone May 18, 2022
@DavoudEshtehari DavoudEshtehari requested review from David-Engel and removed request for johnnypham May 18, 2022 23:33
Copy link
Contributor

@David-Engel David-Engel left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@DavoudEshtehari DavoudEshtehari left a comment

Choose a reason for hiding this comment

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

Thank you, It's pretty much cleaner. Just some unnecessary usings in SNITcpHandle, SNIpacket, and SNINpHandle should be removed.

@Wraith2
Copy link
Contributor Author

Wraith2 commented May 20, 2022

Usings cleaned up in the files mentioned.

@DavoudEshtehari DavoudEshtehari merged commit 2c56830 into dotnet:main May 26, 2022
@Wraith2 Wraith2 deleted the sni-simplifycallbacks branch May 26, 2022 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
➕ Code Health Changes related to source code improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants