Skip to content

Simplify winsock initialization#43284

Merged
jkotas merged 7 commits intodotnet:masterfrom
jkotas:sockets-init
Oct 13, 2020
Merged

Simplify winsock initialization#43284
jkotas merged 7 commits intodotnet:masterfrom
jkotas:sockets-init

Conversation

@jkotas
Copy link
Member

@jkotas jkotas commented Oct 11, 2020

Change winsock initialization to be internal detail of Windows PAL. It is unnecessary on non-Windows platforms.

@ghost
Copy link

ghost commented Oct 11, 2020

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

….Windows.cs

Co-authored-by: Stephen Toub <stoub@microsoft.com>
if (Interlocked.CompareExchange(ref s_initialized, 1, 0) != 0)
{
// Keep the winsock initialization count ballanced if other thread beats us to finish the initialization.
WSACleanup();
Copy link
Member

Choose a reason for hiding this comment

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

I assume this is just for good hygiene? Since we're not cleaning up after these in general, presumably it wouldn't matter technically right now if there were a few extra startups in there.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is just for good hygiene. I will add comment about it.

{
WSAData d;
return WSAStartup(0x0202 /* 2.2 */, &d);
if (s_initialized == 0)
Copy link
Member

Choose a reason for hiding this comment

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

Might be worth a brief comment about why volatile isn't necessary here.

…artup.cs

Co-authored-by: Stephen Toub <stoub@microsoft.com>
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Thanks.

@jkotas jkotas merged commit c80e100 into dotnet:master Oct 13, 2020
@jkotas jkotas deleted the sockets-init branch October 13, 2020 04:25
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
@karelz karelz added this to the 6.0.0 milestone Jan 26, 2021
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.

4 participants