From 9f2c092366af9ca473bc92bf175dc0f37b87d8af Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 25 Sep 2020 10:06:52 +0200 Subject: [PATCH 01/10] Socket tests: don't retry CanceledByDispose tests on timeout These retries cause masking cases where the operation and dispose Task are never completing. --- .../tests/FunctionalTests/Accept.cs | 36 ++++-- .../tests/FunctionalTests/Connect.cs | 36 ++++-- .../tests/FunctionalTests/SendFile.cs | 51 +++++--- .../tests/FunctionalTests/SendReceive.cs | 110 +++++++++++------- 4 files changed, 152 insertions(+), 81 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs index 7f0dc3adc14a41..78b172a16bf016 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs @@ -295,7 +295,8 @@ public async Task AcceptGetsCanceledByDispose() // We try this a couple of times to deal with a timing race: if the Dispose happens // before the operation is started, we won't see a SocketException. int msDelay = 100; - await RetryHelper.ExecuteAsync(async () => + int retries = 10; + while (true) { var listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); listener.Bind(new IPEndPoint(IPAddress.Loopback, 0)); @@ -330,20 +331,33 @@ await RetryHelper.ExecuteAsync(async () => disposedException = true; } - if (UsesApm) - { - Assert.Null(localSocketError); - Assert.True(disposedException); - } - else if (UsesSync) + try { - Assert.Equal(SocketError.Interrupted, localSocketError); + if (UsesApm) + { + Assert.Null(localSocketError); + Assert.True(disposedException); + } + else if (UsesSync) + { + Assert.Equal(SocketError.Interrupted, localSocketError); + } + else + { + Assert.Equal(SocketError.OperationAborted, localSocketError); + } + break; } - else + catch { - Assert.Equal(SocketError.OperationAborted, localSocketError); + if (retries-- > 0) + { + continue; + } + + throw; } - }, maxAttempts: 10); + } } [Fact] diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs index 08ecf15f3b925b..df26fa35f2f14a 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs @@ -127,7 +127,8 @@ public async Task ConnectGetsCanceledByDispose() // We try this a couple of times to deal with a timing race: if the Dispose happens // before the operation is started, we won't see a SocketException. int msDelay = 100; - await RetryHelper.ExecuteAsync(async () => + int retries = 10; + while (true) { var client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); @@ -163,20 +164,33 @@ await RetryHelper.ExecuteAsync(async () => disposedException = true; } - if (UsesApm) - { - Assert.Null(localSocketError); - Assert.True(disposedException); - } - else if (UsesSync) + try { - Assert.Equal(SocketError.NotSocket, localSocketError); + if (UsesApm) + { + Assert.Null(localSocketError); + Assert.True(disposedException); + } + else if (UsesSync) + { + Assert.Equal(SocketError.NotSocket, localSocketError); + } + else + { + Assert.Equal(SocketError.OperationAborted, localSocketError); + } + break; } - else + catch { - Assert.Equal(SocketError.OperationAborted, localSocketError); + if (retries-- > 0) + { + continue; + } + + throw; } - }, maxAttempts: 10); + } } } diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs index 3a8db0f161ee6e..31f3ccea9482d4 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs @@ -288,7 +288,8 @@ public async Task SyncSendFileGetsCanceledByDispose() // before the operation is started, the peer won't see a ConnectionReset SocketException and we won't // see a SocketException either. int msDelay = 100; - await RetryHelper.ExecuteAsync(async () => + int retries = 10; + while (true) { (Socket socket1, Socket socket2) = SocketTestExtensions.CreateConnectedSocketPair(); using (socket2) @@ -328,34 +329,48 @@ await RetryHelper.ExecuteAsync(async () => } catch (ObjectDisposedException) { } - Assert.Equal(SocketError.ConnectionAborted, localSocketError); - // On OSX, we're unable to unblock the on-going socket operations and - // perform an abortive close. - if (!PlatformDetection.IsOSXLike) + try { - SocketError? peerSocketError = null; - var receiveBuffer = new byte[4096]; - while (true) + Assert.Equal(SocketError.ConnectionAborted, localSocketError); + + // On OSX, we're unable to unblock the on-going socket operations and + // perform an abortive close. + if (!PlatformDetection.IsOSXLike) { - try + SocketError? peerSocketError = null; + var receiveBuffer = new byte[4096]; + while (true) { - int received = socket2.Receive(receiveBuffer); - if (received == 0) + try { + int received = socket2.Receive(receiveBuffer); + if (received == 0) + { + break; + } + } + catch (SocketException se) + { + peerSocketError = se.SocketErrorCode; break; } } - catch (SocketException se) - { - peerSocketError = se.SocketErrorCode; - break; - } + Assert.Equal(SocketError.ConnectionReset, peerSocketError); } - Assert.Equal(SocketError.ConnectionReset, peerSocketError); + break; + } + catch + { + if (retries-- > 0) + { + continue; + } + + throw; } } - }, maxAttempts: 10); + } } [OuterLoop] diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs index 50a329e044c2b0..9176b99d24a1bc 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs @@ -944,7 +944,8 @@ public async Task UdpReceiveGetsCanceledByDispose() // We try this a couple of times to deal with a timing race: if the Dispose happens // before the operation is started, we won't see a SocketException. int msDelay = 100; - await RetryHelper.ExecuteAsync(async () => + int retries = 10; + while (true) { var socket = new Socket(AddressFamily.InterNetwork, SocketType.Dgram, ProtocolType.Udp); socket.BindToAnonymousPort(IPAddress.Loopback); @@ -978,20 +979,33 @@ await RetryHelper.ExecuteAsync(async () => disposedException = true; } - if (UsesApm) - { - Assert.Null(localSocketError); - Assert.True(disposedException); - } - else if (UsesSync) + try { - Assert.Equal(SocketError.Interrupted, localSocketError); + if (UsesApm) + { + Assert.Null(localSocketError); + Assert.True(disposedException); + } + else if (UsesSync) + { + Assert.Equal(SocketError.Interrupted, localSocketError); + } + else + { + Assert.Equal(SocketError.OperationAborted, localSocketError); + } + break; } - else + catch { - Assert.Equal(SocketError.OperationAborted, localSocketError); + if (retries-- > 0) + { + continue; + } + + throw; } - }, maxAttempts: 10); + } } [Theory] @@ -1003,7 +1017,8 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend) // before the operation is started, the peer won't see a ConnectionReset SocketException and we won't // see a SocketException either. int msDelay = 100; - await RetryHelper.ExecuteAsync(async () => + int retries = 10; + while (true) { (Socket socket1, Socket socket2) = SocketTestExtensions.CreateConnectedSocketPair(); using (socket2) @@ -1052,46 +1067,59 @@ await RetryHelper.ExecuteAsync(async () => disposedException = true; } - if (UsesApm) - { - Assert.Null(localSocketError); - Assert.True(disposedException); - } - else if (UsesSync) - { - Assert.Equal(SocketError.ConnectionAborted, localSocketError); - } - else + try { - Assert.Equal(SocketError.OperationAborted, localSocketError); - } + if (UsesApm) + { + Assert.Null(localSocketError); + Assert.True(disposedException); + } + else if (UsesSync) + { + Assert.Equal(SocketError.ConnectionAborted, localSocketError); + } + else + { + Assert.Equal(SocketError.OperationAborted, localSocketError); + } - // On OSX, we're unable to unblock the on-going socket operations and - // perform an abortive close. - if (!(UsesSync && PlatformDetection.IsOSXLike)) - { - SocketError? peerSocketError = null; - var receiveBuffer = new ArraySegment(new byte[4096]); - while (true) + // On OSX, we're unable to unblock the on-going socket operations and + // perform an abortive close. + if (!(UsesSync && PlatformDetection.IsOSXLike)) { - try + SocketError? peerSocketError = null; + var receiveBuffer = new ArraySegment(new byte[4096]); + while (true) { - int received = await ReceiveAsync(socket2, receiveBuffer); - if (received == 0) + try + { + int received = await ReceiveAsync(socket2, receiveBuffer); + if (received == 0) + { + break; + } + } + catch (SocketException se) { + peerSocketError = se.SocketErrorCode; break; } } - catch (SocketException se) - { - peerSocketError = se.SocketErrorCode; - break; - } + Assert.Equal(SocketError.ConnectionReset, peerSocketError); + } + break; + } + catch + { + if (retries-- > 0) + { + continue; } - Assert.Equal(SocketError.ConnectionReset, peerSocketError); + + throw; } } - }, maxAttempts: 10); + } } [Fact] From 37176da850ea52041b528dc95db5e022352258fc Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 28 Sep 2020 15:23:06 +0200 Subject: [PATCH 02/10] print g_config_specified_ciphersuites --- .../Native/Unix/System.Security.Cryptography.Native/pal_ssl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c index b53abd73f235cc..44fb1f317328f8 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -11,6 +11,7 @@ #include #include #include +#include c_static_assert(PAL_SSL_ERROR_NONE == SSL_ERROR_NONE); c_static_assert(PAL_SSL_ERROR_SSL == SSL_ERROR_SSL); @@ -118,6 +119,8 @@ static void DetectCiphersuiteConfiguration() g_config_specified_ciphersuites = 1; #endif + write(0, "config\n", 7); + write(0, g_config_specified_ciphersuites == 1 ? "1\n" : "0\n", 2); } void CryptoNative_EnsureLibSslInitialized() From b7b8d2cd9c9bc7c05b44b0f606a29b7ce6611a41 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 1 Oct 2020 17:01:06 +0200 Subject: [PATCH 03/10] Move connection timeout retry --- .../System.Net.Sockets/tests/FunctionalTests/Connect.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs index df26fa35f2f14a..fd84bcfb664dde 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs @@ -154,9 +154,6 @@ public async Task ConnectGetsCanceledByDispose() } catch (SocketException se) { - // On connection timeout, retry. - Assert.NotEqual(SocketError.TimedOut, se.SocketErrorCode); - localSocketError = se.SocketErrorCode; } catch (ObjectDisposedException) @@ -166,6 +163,9 @@ public async Task ConnectGetsCanceledByDispose() try { + // On connection timeout, retry. + Assert.NotEqual(SocketError.TimedOut, localSocketError); + if (UsesApm) { Assert.Null(localSocketError); From ee3dd1944f3d237108b7d29f1d79de753b5352e8 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 6 Oct 2020 19:21:34 +0200 Subject: [PATCH 04/10] Undo unrelated changes --- .../Native/Unix/System.Security.Cryptography.Native/pal_ssl.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c index 44fb1f317328f8..b53abd73f235cc 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -11,7 +11,6 @@ #include #include #include -#include c_static_assert(PAL_SSL_ERROR_NONE == SSL_ERROR_NONE); c_static_assert(PAL_SSL_ERROR_SSL == SSL_ERROR_SSL); @@ -119,8 +118,6 @@ static void DetectCiphersuiteConfiguration() g_config_specified_ciphersuites = 1; #endif - write(0, "config\n", 7); - write(0, g_config_specified_ciphersuites == 1 ? "1\n" : "0\n", 2); } void CryptoNative_EnsureLibSslInitialized() From 512edf5c70bc4969451b45a6a5ad5f66ba40c0c6 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 12 Oct 2020 16:20:31 +0200 Subject: [PATCH 05/10] test cancellation also with IPV6 and Dual Mode --- .../Net/Sockets/SocketTestExtensions.cs | 17 +++++++--- .../tests/FunctionalTests/Accept.cs | 17 +++++++--- .../tests/FunctionalTests/Connect.cs | 17 +++++++--- .../FunctionalTests/InlineCompletions.Unix.cs | 4 +-- .../tests/FunctionalTests/SendReceive.cs | 34 ++++++++++++++----- 5 files changed, 66 insertions(+), 23 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs index 5d2ef3ee1f0980..374aa5a21e9392 100644 --- a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs +++ b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs @@ -38,14 +38,21 @@ public static void ForceNonBlocking(this Socket socket, bool force) } } - public static (Socket, Socket) CreateConnectedSocketPair() + public static (Socket, Socket) CreateConnectedSocketPair() => CreateConnectedSocketPair(IPAddress.Loopback, false); + + public static (Socket, Socket) CreateConnectedSocketPair(IPAddress serverAddress, bool dualModeClient) { - using Socket listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - listener.Bind(new IPEndPoint(IPAddress.Loopback, 0)); + using Socket listener = new Socket(serverAddress.AddressFamily, SocketType.Stream, ProtocolType.Tcp); + listener.Bind(new IPEndPoint(serverAddress, 0)); listener.Listen(1); - Socket client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - client.Connect(listener.LocalEndPoint); + IPEndPoint connectTo = (IPEndPoint)listener.LocalEndPoint; + if (dualModeClient) connectTo = new IPEndPoint(connectTo.Address.MapToIPv6(), connectTo.Port); + + + Socket client = new Socket(connectTo.AddressFamily, SocketType.Stream, ProtocolType.Tcp); + if (dualModeClient) client.DualMode = true; + client.Connect(connectTo); Socket server = listener.Accept(); return (client, server); diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs index 78b172a16bf016..15a6de75bc1739 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs @@ -289,8 +289,16 @@ public async Task AcceptAsync_MultipleAcceptsThenDispose_AcceptsThrowAfterDispos } } - [Fact] - public async Task AcceptGetsCanceledByDispose() + public static readonly TheoryData AcceptGetsCanceledByDispose_Data = new TheoryData + { + { IPAddress.Loopback }, + { IPAddress.IPv6Loopback }, + { IPAddress.Loopback.MapToIPv6() } + }; + + [Theory] + [MemberData(nameof(AcceptGetsCanceledByDispose_Data))] + public async Task AcceptGetsCanceledByDispose(IPAddress loopback) { // We try this a couple of times to deal with a timing race: if the Dispose happens // before the operation is started, we won't see a SocketException. @@ -298,8 +306,9 @@ public async Task AcceptGetsCanceledByDispose() int retries = 10; while (true) { - var listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - listener.Bind(new IPEndPoint(IPAddress.Loopback, 0)); + var listener = new Socket(loopback.AddressFamily, SocketType.Stream, ProtocolType.Tcp); + if (loopback.IsIPv4MappedToIPv6) listener.DualMode = true; + listener.Bind(new IPEndPoint(loopback, 0)); listener.Listen(1); Task acceptTask = AcceptAsync(listener); diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs index fd84bcfb664dde..7bb57fccfa1359 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs @@ -120,9 +120,17 @@ public async Task Connect_AfterDisconnect_Fails() } } - [Fact] + public static readonly TheoryData ConnectGetsCanceledByDispose_Data = new TheoryData + { + { IPAddress.Parse("1.1.1.1") }, + { IPAddress.Parse("fd11:1:1:1:1:1:1:1") }, + { IPAddress.Parse("1.1.1.1").MapToIPv6() }, + }; + + [Theory] + [MemberData(nameof(ConnectGetsCanceledByDispose_Data))] [PlatformSpecific(~(TestPlatforms.OSX | TestPlatforms.FreeBSD))] // Not supported on BSD like OSes. - public async Task ConnectGetsCanceledByDispose() + public async Task ConnectGetsCanceledByDispose(IPAddress address) { // We try this a couple of times to deal with a timing race: if the Dispose happens // before the operation is started, we won't see a SocketException. @@ -130,9 +138,10 @@ public async Task ConnectGetsCanceledByDispose() int retries = 10; while (true) { - var client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); + var client = new Socket(address.AddressFamily, SocketType.Stream, ProtocolType.Tcp); + if (address.IsIPv4MappedToIPv6) client.DualMode = true; - Task connectTask = ConnectAsync(client, new IPEndPoint(IPAddress.Parse("1.1.1.1"), 23)); + Task connectTask = ConnectAsync(client, new IPEndPoint(address, 23)); // Wait a little so the operation is started. await Task.Delay(msDelay); diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/InlineCompletions.Unix.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/InlineCompletions.Unix.cs index 3823e511ba59b4..d73d831c1f2bfb 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/InlineCompletions.Unix.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/InlineCompletions.Unix.cs @@ -31,8 +31,8 @@ public void InlineSocketContinuations() await new SendReceiveEap(null).SendRecv_Stream_TCP(IPAddress.Loopback, useMultipleBuffers: false); await new SendReceiveEap(null).SendRecv_Stream_TCP_MultipleConcurrentReceives(IPAddress.Loopback, useMultipleBuffers: false); await new SendReceiveEap(null).SendRecv_Stream_TCP_MultipleConcurrentSends(IPAddress.Loopback, useMultipleBuffers: false); - await new SendReceiveEap(null).TcpReceiveSendGetsCanceledByDispose(receiveOrSend: true); - await new SendReceiveEap(null).TcpReceiveSendGetsCanceledByDispose(receiveOrSend: false); + await new SendReceiveEap(null).TcpReceiveSendGetsCanceledByDispose(receiveOrSend: true, serverAddress: IPAddress.Loopback, dualModeClient: false); + await new SendReceiveEap(null).TcpReceiveSendGetsCanceledByDispose(receiveOrSend: false, serverAddress: IPAddress.Loopback, dualModeClient: false); }, options).Dispose(); } } diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs index 9176b99d24a1bc..240db1dc78912b 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs @@ -937,9 +937,17 @@ error is SocketException || } } - [Fact] + public static readonly TheoryData UdpReceiveGetsCanceledByDispose_Data = new TheoryData + { + { IPAddress.Loopback }, + { IPAddress.IPv6Loopback }, + { IPAddress.Loopback.MapToIPv6() } + }; + + [Theory] + [MemberData(nameof(UdpReceiveGetsCanceledByDispose_Data))] [PlatformSpecific(~TestPlatforms.OSX)] // Not supported on OSX. - public async Task UdpReceiveGetsCanceledByDispose() + public async Task UdpReceiveGetsCanceledByDispose(IPAddress address) { // We try this a couple of times to deal with a timing race: if the Dispose happens // before the operation is started, we won't see a SocketException. @@ -947,8 +955,9 @@ public async Task UdpReceiveGetsCanceledByDispose() int retries = 10; while (true) { - var socket = new Socket(AddressFamily.InterNetwork, SocketType.Dgram, ProtocolType.Udp); - socket.BindToAnonymousPort(IPAddress.Loopback); + var socket = new Socket(address.AddressFamily, SocketType.Dgram, ProtocolType.Udp); + if (address.IsIPv4MappedToIPv6) socket.DualMode = true; + socket.BindToAnonymousPort(address); Task receiveTask = ReceiveAsync(socket, new ArraySegment(new byte[1])); @@ -1008,10 +1017,19 @@ public async Task UdpReceiveGetsCanceledByDispose() } } + public static readonly TheoryData TcpReceiveSendGetsCanceledByDispose_Data = new TheoryData + { + { true, IPAddress.Loopback, false }, + { true, IPAddress.Loopback, true }, + { true, IPAddress.IPv6Loopback, false }, + { false, IPAddress.Loopback, false }, + { false, IPAddress.Loopback, true }, + { false, IPAddress.IPv6Loopback, false }, + }; + [Theory] - [InlineData(true)] - [InlineData(false)] - public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend) + [MemberData(nameof(TcpReceiveSendGetsCanceledByDispose_Data))] + public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, IPAddress serverAddress, bool dualModeClient) { // We try this a couple of times to deal with a timing race: if the Dispose happens // before the operation is started, the peer won't see a ConnectionReset SocketException and we won't @@ -1020,7 +1038,7 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend) int retries = 10; while (true) { - (Socket socket1, Socket socket2) = SocketTestExtensions.CreateConnectedSocketPair(); + (Socket socket1, Socket socket2) = SocketTestExtensions.CreateConnectedSocketPair(serverAddress, dualModeClient); using (socket2) { Task socketOperation; From 80501f9127ad13e8a57f5240e7036e0285c8e58a Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 12 Oct 2020 17:27:05 +0200 Subject: [PATCH 06/10] do not use 1.1.1.1 and similar --- .../tests/System/Net/Sockets/SocketTestExtensions.cs | 7 +++++++ .../tests/FunctionalTests/Connect.cs | 12 ++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs index 374aa5a21e9392..1c08dda0448ac6 100644 --- a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs +++ b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs @@ -58,6 +58,13 @@ public static (Socket, Socket) CreateConnectedSocketPair(IPAddress serverAddress return (client, server); } + public static int GetUnusedPort(AddressFamily addressFamily, ProtocolType protocolType) + { + using Socket temp = new Socket(addressFamily, protocolType == ProtocolType.Tcp ? SocketType.Stream : SocketType.Dgram, protocolType); + IPAddress address = addressFamily == AddressFamily.InterNetwork ? IPAddress.Loopback : IPAddress.IPv6Loopback; + return temp.BindToAnonymousPort(address); + } + // Tries to connect within the provided timeout interval // Useful to speed up "can not connect" assertions on Windows public static bool TryConnect(this Socket socket, EndPoint remoteEndpoint, int millisecondsTimeout) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs index 7bb57fccfa1359..6a484f0a85639b 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs @@ -122,12 +122,12 @@ public async Task Connect_AfterDisconnect_Fails() public static readonly TheoryData ConnectGetsCanceledByDispose_Data = new TheoryData { - { IPAddress.Parse("1.1.1.1") }, - { IPAddress.Parse("fd11:1:1:1:1:1:1:1") }, - { IPAddress.Parse("1.1.1.1").MapToIPv6() }, + { IPAddress.Loopback }, + { IPAddress.IPv6Loopback }, + { IPAddress.Loopback.MapToIPv6() }, }; - [Theory] + [Theory(Timeout = 30000)] [MemberData(nameof(ConnectGetsCanceledByDispose_Data))] [PlatformSpecific(~(TestPlatforms.OSX | TestPlatforms.FreeBSD))] // Not supported on BSD like OSes. public async Task ConnectGetsCanceledByDispose(IPAddress address) @@ -140,8 +140,8 @@ public async Task ConnectGetsCanceledByDispose(IPAddress address) { var client = new Socket(address.AddressFamily, SocketType.Stream, ProtocolType.Tcp); if (address.IsIPv4MappedToIPv6) client.DualMode = true; - - Task connectTask = ConnectAsync(client, new IPEndPoint(address, 23)); + int port = SocketTestExtensions.GetUnusedPort(address.AddressFamily, ProtocolType.Tcp); + Task connectTask = ConnectAsync(client, new IPEndPoint(address, port)); // Wait a little so the operation is started. await Task.Delay(msDelay); From 4d83e73a38e9b8186b4b7a3ca74307a5634b3b1a Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 12 Oct 2020 17:32:38 +0200 Subject: [PATCH 07/10] Revert "do not use 1.1.1.1 and similar" This reverts commit 80501f9127ad13e8a57f5240e7036e0285c8e58a. --- .../tests/System/Net/Sockets/SocketTestExtensions.cs | 7 ------- .../tests/FunctionalTests/Connect.cs | 12 ++++++------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs index 1c08dda0448ac6..374aa5a21e9392 100644 --- a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs +++ b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs @@ -58,13 +58,6 @@ public static (Socket, Socket) CreateConnectedSocketPair(IPAddress serverAddress return (client, server); } - public static int GetUnusedPort(AddressFamily addressFamily, ProtocolType protocolType) - { - using Socket temp = new Socket(addressFamily, protocolType == ProtocolType.Tcp ? SocketType.Stream : SocketType.Dgram, protocolType); - IPAddress address = addressFamily == AddressFamily.InterNetwork ? IPAddress.Loopback : IPAddress.IPv6Loopback; - return temp.BindToAnonymousPort(address); - } - // Tries to connect within the provided timeout interval // Useful to speed up "can not connect" assertions on Windows public static bool TryConnect(this Socket socket, EndPoint remoteEndpoint, int millisecondsTimeout) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs index 6a484f0a85639b..7bb57fccfa1359 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs @@ -122,12 +122,12 @@ public async Task Connect_AfterDisconnect_Fails() public static readonly TheoryData ConnectGetsCanceledByDispose_Data = new TheoryData { - { IPAddress.Loopback }, - { IPAddress.IPv6Loopback }, - { IPAddress.Loopback.MapToIPv6() }, + { IPAddress.Parse("1.1.1.1") }, + { IPAddress.Parse("fd11:1:1:1:1:1:1:1") }, + { IPAddress.Parse("1.1.1.1").MapToIPv6() }, }; - [Theory(Timeout = 30000)] + [Theory] [MemberData(nameof(ConnectGetsCanceledByDispose_Data))] [PlatformSpecific(~(TestPlatforms.OSX | TestPlatforms.FreeBSD))] // Not supported on BSD like OSes. public async Task ConnectGetsCanceledByDispose(IPAddress address) @@ -140,8 +140,8 @@ public async Task ConnectGetsCanceledByDispose(IPAddress address) { var client = new Socket(address.AddressFamily, SocketType.Stream, ProtocolType.Tcp); if (address.IsIPv4MappedToIPv6) client.DualMode = true; - int port = SocketTestExtensions.GetUnusedPort(address.AddressFamily, ProtocolType.Tcp); - Task connectTask = ConnectAsync(client, new IPEndPoint(address, port)); + + Task connectTask = ConnectAsync(client, new IPEndPoint(address, 23)); // Wait a little so the operation is started. await Task.Delay(msDelay); From ba5df5eb94d3c65564d9100bd86a8ae32b2e7b47 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 12 Oct 2020 17:33:35 +0200 Subject: [PATCH 08/10] do not test IPV6 for now --- .../System.Net.Sockets/tests/FunctionalTests/Connect.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs index 7bb57fccfa1359..89b5e68d4c53e7 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs @@ -123,7 +123,7 @@ public async Task Connect_AfterDisconnect_Fails() public static readonly TheoryData ConnectGetsCanceledByDispose_Data = new TheoryData { { IPAddress.Parse("1.1.1.1") }, - { IPAddress.Parse("fd11:1:1:1:1:1:1:1") }, + // TODO: Figure out how to test this with vanilla IPV6 { IPAddress.Parse("1.1.1.1").MapToIPv6() }, }; From b301bb09434891e80ff270de9ef7867dc5dafebf Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 12 Oct 2020 19:03:37 +0200 Subject: [PATCH 09/10] fall back to shutdown, if connect(AF_UNSPEC) fails --- src/libraries/Native/Unix/System.Native/pal_networking.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libraries/Native/Unix/System.Native/pal_networking.c b/src/libraries/Native/Unix/System.Native/pal_networking.c index 09ec260149f497..cf080d439ac478 100644 --- a/src/libraries/Native/Unix/System.Native/pal_networking.c +++ b/src/libraries/Native/Unix/System.Native/pal_networking.c @@ -3085,6 +3085,11 @@ int32_t SystemNative_Disconnect(intptr_t socket) addr.sa_family = AF_UNSPEC; err = connect(fd, &addr, sizeof(addr)); + if (err != 0) + { + // On some older kernels connect(AF_UNSPEC) may fail. Fall back to shutdown in these cases: + err = shutdown(fd, SHUT_RDWR); + } #elif HAVE_DISCONNECTX // disconnectx causes a FIN close on OSX. It's the best we can do. err = disconnectx(fd, SAE_ASSOCID_ANY, SAE_CONNID_ANY); From 615846033812803cc1a5b1a5f5cf44c81e7353ae Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 12 Oct 2020 20:14:20 +0200 Subject: [PATCH 10/10] TcpReceiveSendGetsCanceledByDispose timeout handling --- .../System.Net.Sockets/tests/FunctionalTests/SendReceive.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs index 240db1dc78912b..2cfb14393a4ee8 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs @@ -1027,7 +1027,7 @@ public async Task UdpReceiveGetsCanceledByDispose(IPAddress address) { false, IPAddress.IPv6Loopback, false }, }; - [Theory] + [Theory(Timeout = 30000)] [MemberData(nameof(TcpReceiveSendGetsCanceledByDispose_Data))] public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, IPAddress serverAddress, bool dualModeClient) { @@ -1064,7 +1064,7 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, IPAddr Task disposeTask = Task.Run(() => socket1.Dispose()); var cts = new CancellationTokenSource(); - Task timeoutTask = Task.Delay(30000, cts.Token); + Task timeoutTask = Task.Delay(15000, cts.Token); Assert.NotSame(timeoutTask, await Task.WhenAny(disposeTask, socketOperation, timeoutTask)); cts.Cancel();