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.

Sign up for GitHub

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

Jump to bottom

[Unix]: AcceptAsync with existing AcceptSocket support on Unix #73499

Merged
merged 7 commits into from
Aug 26, 2022
Merged

[Unix]: AcceptAsync with existing AcceptSocket support on Unix #73499

merged 7 commits into from
Aug 26, 2022
+101 −9

Conversation

liveans
Copy link
Member

@liveans liveans commented Aug 5, 2022

This pr aims to support AcceptAsync operation with existing AcceptSocket on Unix-like systems.
Closes #1483

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Aug 5, 2022
@dnfadmin
Copy link

dnfadmin commented Aug 5, 2022 •
edited
Loading

CLA assistant check
All CLA requirements met.

@ghost
Copy link

ghost commented Aug 5, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

This pr aims to support AcceptAsync operation with existing AcceptSocket on Unix-like systems.
Closes #1483

Author: liveans
Assignees: -
Labels:

area-System.Net.Sockets
Milestone: -

@liveans
Copy link
Member Author

liveans commented Aug 6, 2022

@stephentoub I tried to make GetOrCreateAcceptSocket compatible with the Windows definition of this function, but in this way, we're creating an unnecessary socket when acceptSocket? is null. We'll copy the state from the accepted socket to this socket at FinishOperationAccept anyway, but is this the correct behavior here, I feel something is wrong. Maybe I can change the behavior to check if it's null or not.

@liveans liveans marked this pull request as ready for review August 6, 2022 09:50
@liveans
Copy link
Member Author

liveans commented Aug 6, 2022

cc @karelz

@wfurt
Copy link
Member

wfurt commented Aug 16, 2022

Thanks for contributing @liveans. Generally looks ok to me, I Ieft few comments.

@liveans liveans requested a review from wfurt August 20, 2022 14:53
@danmoseley
Copy link
Member

Welcome to the repo @liveans

@liveans
Copy link
Member Author

liveans commented Aug 21, 2022

Welcome to the repo @liveans

Thank you very much!

wfurt
wfurt approved these changes Aug 26, 2022
View reviewed changes
Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM
thanks.

@wfurt wfurt merged commit c112de1 into dotnet:main Aug 26, 2022
@karelz karelz added this to the 8.0.0 milestone Aug 26, 2022
@liveans liveans mentioned this pull request Aug 26, 2022
Merged
@ghost ghost locked as resolved and limited conversation to collaborators Sep 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@danmoseley danmoseley danmoseley left review comments

@stephentoub stephentoub stephentoub left review comments

@jack5341 jack5341 jack5341 approved these changes

@wfurt wfurt wfurt approved these changes

Assignees

@wfurt wfurt

Labels
area-System.Net.Sockets community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.

AcceptAsync with existing Socket not implemented on Unix
7 participants
@liveans @dnfadmin @wfurt @danmoseley @stephentoub @jack5341 @karelz