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

Add Socket(SafeSocketHandle) ctor #34727

Merged
merged 1 commit into from
Apr 10, 2020
Merged

Add Socket(SafeSocketHandle) ctor #34727

merged 1 commit into from
Apr 10, 2020

Conversation

stephentoub
Copy link
Member

Fixes #862
cc: @wfurt, @tmds

@stephentoub stephentoub added this to the 5.0 milestone Apr 9, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Apr 9, 2020

Tagging @dotnet/ncl as an area owner

@stephentoub stephentoub changed the title Add Socket(SocketSafeHandle) ctor Add Socket(SafeSocketHandle) ctor Apr 9, 2020
wfurt
wfurt approved these changes Apr 9, 2020
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.

src/libraries/Common/src/Interop/Unix/System.Native/Interop.Fcntl.cs Show resolved Hide resolved
src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Unix.cs Show resolved Hide resolved
@tmds
Copy link
Member

tmds commented Apr 9, 2020

@stephentoub we should also set _isListening to match the underling socket. On Linux you can use SO_ACCEPTCONN to check if a socket is in listen mode. If we don't do this Accept will throw even if the socket is a listen socket.
This allows to wrap listen sockets coming from systemd init manager.

@stephentoub stephentoub force-pushed the socketsafehandlector branch 2 times, most recently from 6e18569 to a490150 Compare April 10, 2020 13:20
@jaredpar jaredpar mentioned this pull request Apr 10, 2020
Closed
@stephentoub stephentoub merged commit ca1a684 into dotnet:master Apr 10, 2020
@stephentoub stephentoub deleted the socketsafehandlector branch April 10, 2020 23:54
This was referenced Apr 14, 2020
Merged
Merged
@davidfowl davidfowl mentioned this pull request Apr 15, 2020
Closed
@hez2010 hez2010 mentioned this pull request Apr 15, 2020
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tmds tmds tmds left review comments

@wfurt wfurt wfurt approved these changes

Assignees
No one assigned
Labels
area-System.Net.Sockets
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

Consider exposing Socket(SafeSocketHandle fd) constructor
4 participants
@stephentoub @Dotnet-GitSync-Bot @tmds @wfurt