Skip to content

AcceptAsync with existing Socket support on Unix, Review Changes - #74645

Merged
liveans merged 2 commits into
dotnet:mainfrom
liveans:aa-socket-fix-unix-review-changes-1483
Sep 20, 2022
Merged

liveans merged 2 commits into
dotnet:mainfrom
liveans:aa-socket-fix-unix-review-changes-1483

Conversation

@liveans

@liveans liveans commented Aug 26, 2022

Copy link
Copy Markdown
Contributor

Review changes PR for #73499.

@ghost ghost added area-System.Net.Sockets community-contribution Indicates that the PR has been added by a community member labels Aug 26, 2022
@ghost

ghost commented Aug 26, 2022

Copy link
Copy Markdown

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

Issue Details

Review changes PR for #73499.

Author: liveans
Assignees: -
Labels:

area-System.Net.Sockets

Milestone: -

@liveans

liveans commented Aug 26, 2022 •

Copy link
Copy Markdown
Contributor Author

Do we have sufficient testing for disposing of a Socket that's had its handle nulled out?

@stephentoub Looks like we don't have it. Dispose method has a condition like this:

if (handle is not null) 

I can add a test for this behavior if you want to.

@liveans

liveans commented Sep 6, 2022

Copy link
Copy Markdown
Contributor Author

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@liveans liveans removed the community-contribution Indicates that the PR has been added by a community member label Sep 6, 2022
@karelz karelz added this to the 8.0.0 milestone Sep 8, 2022
@liveans

liveans commented Sep 20, 2022

Copy link
Copy Markdown
Contributor Author

CI failures unrelated

@liveans
liveans merged commit 97d4e84 into dotnet:main Sep 20, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Oct 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants