Socket: don't assign right endpoint until the connect is successful.

[tmds]

'Right endpoint' must match the address family of the Socket or
we can't serialize the LocalEndPoint and RemoteEndPoint.

When multiple connect attempts are made against a DualMode Socket with
both IPv4 and IPv6 addresses, a failed attempt must not set 'right
endpoint'.

Fixes #53447.

cc @wfurt @geoffkizer @antonfirsov @stephentoub @pepone

[ghost]

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

Issue Details

---

'Right endpoint' must match the address family of the Socket or
we can't serialize the LocalEndPoint and RemoteEndPoint.

When multiple connect attempts are made against a DualMode Socket with
both IPv4 and IPv6 addresses, a failed attempt must not set 'right
endpoint'.

Fixes #53447.

cc @wfurt @geoffkizer @antonfirsov @stephentoub @pepone

Author:	tmds
Assignees:	-
Labels:	area-System.Net.Sockets
Milestone:	-

[tmds]

There is a test failure here:

runtime/src/libraries/System.Net.Sockets/tests/FunctionalTests/SocketTaskExtensionsTest.cs

Line 38 in f4e01f5

await Assert.ThrowsAsync<SocketException>(async () => await SocketTaskExtensions.ReceiveFromAsync(s, new ArraySegment<byte>(buffer), SocketFlags.None, badEndPoint));

Assert.Throws() Failure
Expected: typeof(System.Net.Sockets.SocketException)
Actual:   typeof(System.InvalidOperationException): You must call the Bind method before performing this operation.
---- System.InvalidOperationException : You must call the Bind method before performing this operation.

The InvalidOperationException is valid. Before the PR, the ConnectAsync operations that are performed earlier in the test caused the 'right endpoint' to be set. I need to update the test expectation.

[tmds]

@geoffkizer thanks for the review! I've addressed your feedback.

[tmds]

Windows behaves different from Linux. Something still assigns 'right endpoint' on Windows. @antonfirsov can you help me figure out what it is? If you step through the EnsureMethodsAreCallable test, at some point something assigns _rightEndPoint though it shouldn't because the endpoint is not a valid one.

Assert.Throws() Failure
Expected: typeof(System.InvalidOperationException)
Actual:   typeof(System.Net.Sockets.SocketException): A request to send or receive data was disallowed because the socket is not connected and (when sending on a datagram socket using a sendto call) no address was supplied.
---- System.Net.Sockets.SocketException : A request to send or receive data was disallowed because the socket is not connected and (when sending on a datagram socket using a sendto call) no address was supplied.


Stack trace
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.CreateException(SocketError error, Boolean forAsyncThrow) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs:line 1379
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.ReceiveFromAsync(Socket socket, CancellationToken cancellationToken) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs:line 989
   at System.Net.Sockets.Socket.ReceiveFromAsync(Memory`1 buffer, SocketFlags socketFlags, EndPoint remoteEndPoint, CancellationToken cancellationToken) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs:line 379
   at System.Net.Sockets.Socket.ReceiveFromAsync(ArraySegment`1 buffer, SocketFlags socketFlags, EndPoint remoteEndPoint) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs:line 350
   at System.Net.Sockets.SocketTaskExtensions.ReceiveFromAsync(Socket socket, ArraySegment`1 buffer, SocketFlags socketFlags, EndPoint remoteEndPoint) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketTaskExtensions.cs:line 56
   at System.Net.Sockets.Tests.SocketTaskExtensionsTest.<>c__DisplayClass0_0.<<EnsureMethodsAreCallable>b__13>d.MoveNext() in /_/src/libraries/System.Net.Sockets/tests/FunctionalTests/SocketTaskExtensionsTest.cs:line 38
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at System.Net.Sockets.Tests.SocketTaskExtensionsTest.<>c__DisplayClass0_0.<EnsureMethodsAreCallable>b__13() in System.Net.Sockets.Tests.dll:token 0x6000edc+0x1f
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at System.Net.Sockets.Tests.SocketTaskExtensionsTest.EnsureMethodsAreCallable() in /_/src/libraries/System.Net.Sockets/tests/FunctionalTests/SocketTaskExtensionsTest.cs:line 38
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/ExecutionContext.cs:line 183
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.MoveNext(Thread threadPoolThread) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncTaskMethodBuilderT.cs:line 324
   at System.Threading.Tasks.AwaitTaskContinuation.<>c.<.cctor>b__17_0(Object state) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TaskContinuation.cs:line 670
   at System.Threading.Tasks.AwaitTaskContinuation.RunCallback(ContextCallback callback, Object state, Task& currentTask) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TaskContinuation.cs:line 697
   at System.Threading.Tasks.Task.RunContinuations(Object continuationObject) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:line 3358
   at System.Threading.Tasks.Task`1.TrySetResult(TResult result) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Future.cs:line 400
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.SetExistingTaskResult(Task`1 task, TResult result) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncTaskMethodBuilderT.cs:line 441
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.ExecutionContextCallback(Object s) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncTaskMethodBuilderT.cs:line 286

[antonfirsov]

@tmds this is happenning because of WildcardBindForConnectIfNecessary will bind the socket, and save _rightEndPoint on Windows by calling DoBind:

runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Windows.cs

Lines 210 to 238 in 0fcd769

	partial void WildcardBindForConnectIfNecessary(AddressFamily addressFamily)
	{
	if (_rightEndPoint != null)
	{
	return;
	}
	// The socket must be bound before using ConnectEx.
	CachedSerializedEndPoint csep;
	switch (addressFamily)
	{
	case AddressFamily.InterNetwork:
	csep = IsDualMode ?
	s_cachedMappedAnyV6EndPoint ??= new CachedSerializedEndPoint(s_IPAddressAnyMapToIPv6) :
	s_cachedAnyEndPoint ??= new CachedSerializedEndPoint(IPAddress.Any);
	break;
	case AddressFamily.InterNetworkV6:
	csep = s_cachedAnyV6EndPoint ??= new CachedSerializedEndPoint(IPAddress.IPv6Any);
	break;
	default:
	return;
	}
	if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, csep.IPEndPoint);
	DoBind(csep.IPEndPoint, csep.SocketAddress);

Since we are actually binding the socket in this case, I think this is a platform difference we need to accept. We may open an issue for harmonization if that makes sense.

The test can be "fixed" by simply moving the UDP calls before the connect calls.

[geoffkizer]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[geoffkizer]

Changes look good to me. Let's run outerloop before merging.

[geoffkizer]

Failures look unrelated.