From 9f2c092366af9ca473bc92bf175dc0f37b87d8af Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 25 Sep 2020 10:06:52 +0200 Subject: [PATCH 01/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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(); From b77d31f4b32a0622b0c54da62064bd56a4759468 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 14 Oct 2020 20:59:56 +0200 Subject: [PATCH 11/26] update tests --- .../tests/System/Net/Sockets/SocketTestExtensions.cs | 4 ++-- .../tests/FunctionalTests/SendReceive.cs | 12 ++++++++++-- .../tests/FunctionalTests/TelemetryTest.cs | 6 ------ 3 files changed, 12 insertions(+), 10 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..fc22110ef22b92 100644 --- a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs +++ b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs @@ -38,9 +38,9 @@ public static void ForceNonBlocking(this Socket socket, bool force) } } - public static (Socket, Socket) CreateConnectedSocketPair() => CreateConnectedSocketPair(IPAddress.Loopback, false); + public static (Socket client, Socket server) CreateConnectedSocketPair() => CreateConnectedSocketPair(IPAddress.Loopback, false); - public static (Socket, Socket) CreateConnectedSocketPair(IPAddress serverAddress, bool dualModeClient) + public static (Socket client, Socket server) CreateConnectedSocketPair(IPAddress serverAddress, bool dualModeClient) { using Socket listener = new Socket(serverAddress.AddressFamily, SocketType.Stream, ProtocolType.Tcp); listener.Bind(new IPEndPoint(serverAddress, 0)); diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs index 2cfb14393a4ee8..0be941a21a7971 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs @@ -7,6 +7,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.DotNet.RemoteExecutor; +using Microsoft.DotNet.XUnitExtensions; using Xunit; using Xunit.Abstractions; @@ -1027,10 +1028,17 @@ public async Task UdpReceiveGetsCanceledByDispose(IPAddress address) { false, IPAddress.IPv6Loopback, false }, }; - [Theory(Timeout = 30000)] + [Theory(Timeout = 40000)] [MemberData(nameof(TcpReceiveSendGetsCanceledByDispose_Data))] public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, IPAddress serverAddress, bool dualModeClient) { + if (UsesSync && PlatformDetection.IsRedHatFamily7 && + receiveOrSend && (serverAddress.AddressFamily == AddressFamily.InterNetworkV6 || dualModeClient)) + { + // TODO: open a new issue for this case, if the PR gets accepted. + throw new SkipTestException("The IPV6 synchronous Receive case times out on RedHat7 systems"); + } + // 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 // see a SocketException either. @@ -1064,7 +1072,7 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, IPAddr Task disposeTask = Task.Run(() => socket1.Dispose()); var cts = new CancellationTokenSource(); - Task timeoutTask = Task.Delay(15000, cts.Token); + Task timeoutTask = Task.Delay(30000, cts.Token); Assert.NotSame(timeoutTask, await Task.WhenAny(disposeTask, socketOperation, timeoutTask)); cts.Cancel(); diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/TelemetryTest.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/TelemetryTest.cs index 8f834a5c112c3d..bb6a8bd2adf622 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/TelemetryTest.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/TelemetryTest.cs @@ -168,12 +168,6 @@ await listener.RunWithCallbackAsync(e => events.Enqueue((e, e.ActivityId)), asyn [MemberData(nameof(SocketMethods_WithBools_MemberData))] public void EventSource_SocketConnectFailure_LogsConnectFailed(string connectMethod, bool useDnsEndPoint) { - if (connectMethod == "Sync" && PlatformDetection.IsRedHatFamily7) - { - // [ActiveIssue("https://github.com/dotnet/runtime/issues/42686")] - throw new SkipTestException("Disposing a Socket performing a sync operation can hang on RedHat7 systems"); - } - RemoteExecutor.Invoke(async (connectMethod, useDnsEndPointString) => { EndPoint endPoint = await GetRemoteEndPointAsync(useDnsEndPointString, port: 12345); From 1da5006d99c0de404cdc2678b20bcf9cf9e62175 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 14 Oct 2020 21:44:56 +0200 Subject: [PATCH 12/26] OuterLoop test using 1.1.1.1 --- .../System.Net.Sockets/tests/FunctionalTests/Connect.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs index 89b5e68d4c53e7..e9c7abb86420db 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs @@ -127,6 +127,7 @@ public async Task Connect_AfterDisconnect_Fails() { IPAddress.Parse("1.1.1.1").MapToIPv6() }, }; + [OuterLoop("Uses external server")] [Theory] [MemberData(nameof(ConnectGetsCanceledByDispose_Data))] [PlatformSpecific(~(TestPlatforms.OSX | TestPlatforms.FreeBSD))] // Not supported on BSD like OSes. From 5b9b879c81b27e18e0e599f947a0bd8e74a55e53 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 14 Oct 2020 23:19:37 +0200 Subject: [PATCH 13/26] change CreateConnectedSocketPair API --- .../Net/Sockets/SocketTestExtensions.cs | 6 +++--- .../tests/FunctionalTests/SendReceive.cs | 20 +++++++++---------- 2 files changed, 13 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 fc22110ef22b92..ddc09357599c77 100644 --- a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs +++ b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs @@ -38,10 +38,10 @@ public static void ForceNonBlocking(this Socket socket, bool force) } } - public static (Socket client, Socket server) CreateConnectedSocketPair() => CreateConnectedSocketPair(IPAddress.Loopback, false); - - public static (Socket client, Socket server) CreateConnectedSocketPair(IPAddress serverAddress, bool dualModeClient) + public static (Socket client, Socket server) CreateConnectedSocketPair(bool ipv6 = false, bool dualModeClient = false) { + IPAddress serverAddress = ipv6 ? IPAddress.IPv6Loopback : IPAddress.Loopback; + using Socket listener = new Socket(serverAddress.AddressFamily, SocketType.Stream, ProtocolType.Tcp); listener.Bind(new IPEndPoint(serverAddress, 0)); listener.Listen(1); diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs index 0be941a21a7971..17ed02deebd0a7 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs @@ -1018,22 +1018,22 @@ public async Task UdpReceiveGetsCanceledByDispose(IPAddress address) } } - public static readonly TheoryData TcpReceiveSendGetsCanceledByDispose_Data = new TheoryData + 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 }, + { true, false, false }, + { true, false, true }, + { true, true, false }, + { false, false, false }, + { false, false, true }, + { false, true, false }, }; [Theory(Timeout = 40000)] [MemberData(nameof(TcpReceiveSendGetsCanceledByDispose_Data))] - public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, IPAddress serverAddress, bool dualModeClient) + public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool ipv6Server, bool dualModeClient) { if (UsesSync && PlatformDetection.IsRedHatFamily7 && - receiveOrSend && (serverAddress.AddressFamily == AddressFamily.InterNetworkV6 || dualModeClient)) + receiveOrSend && (ipv6Server || dualModeClient)) { // TODO: open a new issue for this case, if the PR gets accepted. throw new SkipTestException("The IPV6 synchronous Receive case times out on RedHat7 systems"); @@ -1046,7 +1046,7 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, IPAddr int retries = 10; while (true) { - (Socket socket1, Socket socket2) = SocketTestExtensions.CreateConnectedSocketPair(serverAddress, dualModeClient); + (Socket socket1, Socket socket2) = SocketTestExtensions.CreateConnectedSocketPair(ipv6Server, dualModeClient); using (socket2) { Task socketOperation; From 102561a0fb9a8e97f5793382dd45d43b3c72cc4e Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 14 Oct 2020 23:20:21 +0200 Subject: [PATCH 14/26] do not use SkipTestException --- .../System.Net.Sockets/tests/FunctionalTests/SendReceive.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs index 17ed02deebd0a7..9dd0c74ab4067c 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs @@ -1035,8 +1035,9 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool i if (UsesSync && PlatformDetection.IsRedHatFamily7 && receiveOrSend && (ipv6Server || dualModeClient)) { + // The IPV6 synchronous Receive case times out on RedHat7 systems // TODO: open a new issue for this case, if the PR gets accepted. - throw new SkipTestException("The IPV6 synchronous Receive case times out on RedHat7 systems"); + return; } // We try this a couple of times to deal with a timing race: if the Dispose happens From f591786177e83c30419fb8d9fa8286fcafdcec60 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Fri, 16 Oct 2020 16:53:34 +0200 Subject: [PATCH 15/26] CI tweaks --- .../tests/FunctionalTests/SendReceive.cs | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs index 9dd0c74ab4067c..39975eb0cca60c 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs @@ -1032,13 +1032,13 @@ public async Task UdpReceiveGetsCanceledByDispose(IPAddress address) [MemberData(nameof(TcpReceiveSendGetsCanceledByDispose_Data))] public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool ipv6Server, bool dualModeClient) { - if (UsesSync && PlatformDetection.IsRedHatFamily7 && - receiveOrSend && (ipv6Server || dualModeClient)) - { - // The IPV6 synchronous Receive case times out on RedHat7 systems - // TODO: open a new issue for this case, if the PR gets accepted. - return; - } + //if (UsesSync && PlatformDetection.IsRedHatFamily7 && + // receiveOrSend && (ipv6Server || dualModeClient)) + //{ + // // The IPV6 synchronous Receive case times out on RedHat7 systems + // // TODO: open a new issue for this case, if the PR gets accepted. + // return; + //} // 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 @@ -1077,12 +1077,15 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool i Assert.NotSame(timeoutTask, await Task.WhenAny(disposeTask, socketOperation, timeoutTask)); cts.Cancel(); + _output.WriteLine("awaiting disposeTask..."); await disposeTask; + _output.WriteLine("disposeTask completed."); SocketError? localSocketError = null; bool disposedException = false; try { + _output.WriteLine("awaiting socketOperation..."); await socketOperation; } catch (SocketException se) @@ -1093,6 +1096,7 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool i { disposedException = true; } + _output.WriteLine("socketOperation completed or failed"); try { @@ -1120,7 +1124,9 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool i { try { + _output.WriteLine("awaiting ReceiveAsync(socket2) ..."); int received = await ReceiveAsync(socket2, receiveBuffer); + _output.WriteLine("ReceiveAsync(socket2) done."); if (received == 0) { break; @@ -1129,6 +1135,7 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool i catch (SocketException se) { peerSocketError = se.SocketErrorCode; + _output.WriteLine("ReceiveAsync(socket2) thrown: " + peerSocketError); break; } } From 6909cc82e164d6d214628c76b10d6e2b4b4d6f4c Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 22 Oct 2020 14:33:10 +0200 Subject: [PATCH 16/26] fix Unix build --- .../tests/FunctionalTests/InlineCompletions.Unix.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 d73d831c1f2bfb..a8c9d93d960fca 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, serverAddress: IPAddress.Loopback, dualModeClient: false); - await new SendReceiveEap(null).TcpReceiveSendGetsCanceledByDispose(receiveOrSend: false, serverAddress: IPAddress.Loopback, dualModeClient: false); + await new SendReceiveEap(null).TcpReceiveSendGetsCanceledByDispose(receiveOrSend: true, ipv6Server: false, dualModeClient: false); + await new SendReceiveEap(null).TcpReceiveSendGetsCanceledByDispose(receiveOrSend: false, ipv6Server: false, dualModeClient: false); }, options).Dispose(); } } From 8a3ce60f82b4f361d60102849e3b625f5cb430f3 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 22 Oct 2020 14:55:31 +0200 Subject: [PATCH 17/26] fix TcpReceiveSendGetsCanceledByDispose for RHEL7 --- .../tests/FunctionalTests/SendReceive.cs | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs index 39975eb0cca60c..5892bbc69c7068 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs @@ -1032,13 +1032,10 @@ public async Task UdpReceiveGetsCanceledByDispose(IPAddress address) [MemberData(nameof(TcpReceiveSendGetsCanceledByDispose_Data))] public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool ipv6Server, bool dualModeClient) { - //if (UsesSync && PlatformDetection.IsRedHatFamily7 && - // receiveOrSend && (ipv6Server || dualModeClient)) - //{ - // // The IPV6 synchronous Receive case times out on RedHat7 systems - // // TODO: open a new issue for this case, if the PR gets accepted. - // return; - //} + // RHEL7 kernel has a bug preventing close(AF_UNKNOWN) to succeed with IPv6 sockets. + // In this case Dispose will trigger a graceful shutdown, which means that receive will succeed on socket2. + // TODO: Remove this, once CI machines are updated to a newer kernel. + bool expectGracefulShutdown = UsesSync && PlatformDetection.IsRedHatFamily7 && receiveOrSend && (ipv6Server || 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 @@ -1077,15 +1074,12 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool i Assert.NotSame(timeoutTask, await Task.WhenAny(disposeTask, socketOperation, timeoutTask)); cts.Cancel(); - _output.WriteLine("awaiting disposeTask..."); await disposeTask; - _output.WriteLine("disposeTask completed."); SocketError? localSocketError = null; bool disposedException = false; try { - _output.WriteLine("awaiting socketOperation..."); await socketOperation; } catch (SocketException se) @@ -1096,7 +1090,6 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool i { disposedException = true; } - _output.WriteLine("socketOperation completed or failed"); try { @@ -1124,9 +1117,7 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool i { try { - _output.WriteLine("awaiting ReceiveAsync(socket2) ..."); int received = await ReceiveAsync(socket2, receiveBuffer); - _output.WriteLine("ReceiveAsync(socket2) done."); if (received == 0) { break; @@ -1135,11 +1126,18 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool i catch (SocketException se) { peerSocketError = se.SocketErrorCode; - _output.WriteLine("ReceiveAsync(socket2) thrown: " + peerSocketError); break; } } - Assert.Equal(SocketError.ConnectionReset, peerSocketError); + if (expectGracefulShutdown) + { + Assert.Equal(SocketError.ConnectionReset, peerSocketError); + } + else + { + Assert.Equal(null, peerSocketError); + } + } break; } From 800d910443e4830a059cff7087e469085425dfb7 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 22 Oct 2020 18:20:58 +0200 Subject: [PATCH 18/26] actually fix TcpReceiveSendGetsCanceledByDispose --- .../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 5892bbc69c7068..6848148d4d5b5e 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs @@ -1129,13 +1129,13 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool i break; } } - if (expectGracefulShutdown) + if (!expectGracefulShutdown) { Assert.Equal(SocketError.ConnectionReset, peerSocketError); } else { - Assert.Equal(null, peerSocketError); + Assert.Null(peerSocketError); } } From b371ae70527e711ca85ef91ad8ccdf12a31e460d Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 22 Oct 2020 19:06:11 +0200 Subject: [PATCH 19/26] RetryHelper: add retryWhen argument --- .../tests/TestUtilities/System/RetryHelper.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/libraries/Common/tests/TestUtilities/System/RetryHelper.cs b/src/libraries/Common/tests/TestUtilities/System/RetryHelper.cs index cd5221e2b41ed2..dabd05dd0d2755 100644 --- a/src/libraries/Common/tests/TestUtilities/System/RetryHelper.cs +++ b/src/libraries/Common/tests/TestUtilities/System/RetryHelper.cs @@ -10,12 +10,13 @@ namespace System public static partial class RetryHelper { private static readonly Func s_defaultBackoffFunc = i => Math.Min(i * 100, 60_000); + private static readonly Predicate s_defaultRetryWhenFunc = _ => true; /// Executes the action up to a maximum of times. /// The maximum number of times to invoke . /// The test to invoke. /// After a failure, invoked to determine how many milliseconds to wait before the next attempt. It's passed the number of iterations attempted. - public static void Execute(Action test, int maxAttempts = 5, Func backoffFunc = null) + public static void Execute(Action test, int maxAttempts = 5, Func backoffFunc = null, Predicate retryWhen = null) { // Validate arguments if (maxAttempts < 1) @@ -27,6 +28,8 @@ public static void Execute(Action test, int maxAttempts = 5, Func back throw new ArgumentNullException(nameof(test)); } + retryWhen ??= s_defaultRetryWhenFunc; + // Execute the test until it either passes or we run it maxAttempts times var exceptions = new List(); for (int i = 1; i <= maxAttempts; i++) @@ -36,7 +39,7 @@ public static void Execute(Action test, int maxAttempts = 5, Func back test(); return; } - catch (Exception e) + catch (Exception e) when (retryWhen(e)) { exceptions.Add(e); if (i == maxAttempts) @@ -53,7 +56,8 @@ public static void Execute(Action test, int maxAttempts = 5, Func back /// The maximum number of times to invoke . /// The test to invoke. /// After a failure, invoked to determine how many milliseconds to wait before the next attempt. It's passed the number of iterations attempted. - public static async Task ExecuteAsync(Func test, int maxAttempts = 5, Func backoffFunc = null) + /// Invoked to select the exceptions to retry on. If not set, any exception will trigger a retry. + public static async Task ExecuteAsync(Func test, int maxAttempts = 5, Func backoffFunc = null, Predicate retryWhen = null) { // Validate arguments if (maxAttempts < 1) @@ -65,6 +69,8 @@ public static async Task ExecuteAsync(Func test, int maxAttempts = 5, Func throw new ArgumentNullException(nameof(test)); } + retryWhen ??= s_defaultRetryWhenFunc; + // Execute the test until it either passes or we run it maxAttempts times var exceptions = new List(); for (int i = 1; i <= maxAttempts; i++) @@ -74,7 +80,7 @@ public static async Task ExecuteAsync(Func test, int maxAttempts = 5, Func await test().ConfigureAwait(false); return; } - catch (Exception e) + catch (Exception e) when (retryWhen(e)) { exceptions.Add(e); if (i == maxAttempts) From 720c52204a4dc5f044104c01c9e279c1f026bb9e Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 22 Oct 2020 19:07:46 +0200 Subject: [PATCH 20/26] AcceptGetsCanceledByDispose: use RetryHelper again --- .../tests/FunctionalTests/Accept.cs | 43 ++++++------------- 1 file changed, 13 insertions(+), 30 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs index 15a6de75bc1739..5eb3fc8c617f3d 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs @@ -5,6 +5,7 @@ using System.Threading.Tasks; using Xunit; using Xunit.Abstractions; +using Xunit.Sdk; namespace System.Net.Sockets.Tests { @@ -303,8 +304,7 @@ 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. int msDelay = 100; - int retries = 10; - while (true) + await RetryHelper.ExecuteAsync(async () => { var listener = new Socket(loopback.AddressFamily, SocketType.Stream, ProtocolType.Tcp); if (loopback.IsIPv4MappedToIPv6) listener.DualMode = true; @@ -318,11 +318,7 @@ public async Task AcceptGetsCanceledByDispose(IPAddress loopback) msDelay *= 2; Task disposeTask = Task.Run(() => listener.Dispose()); - var cts = new CancellationTokenSource(); - Task timeoutTask = Task.Delay(30000, cts.Token); - Assert.NotSame(timeoutTask, await Task.WhenAny(disposeTask, acceptTask, timeoutTask)); - cts.Cancel(); - + await Task.WhenAny(disposeTask, acceptTask).TimeoutAfter(30000); await disposeTask; SocketError? localSocketError = null; @@ -340,33 +336,20 @@ public async Task AcceptGetsCanceledByDispose(IPAddress loopback) disposedException = true; } - try + if (UsesApm) { - if (UsesApm) - { - Assert.Null(localSocketError); - Assert.True(disposedException); - } - else if (UsesSync) - { - Assert.Equal(SocketError.Interrupted, localSocketError); - } - else - { - Assert.Equal(SocketError.OperationAborted, localSocketError); - } - break; + Assert.Null(localSocketError); + Assert.True(disposedException); } - catch + else if (UsesSync) { - if (retries-- > 0) - { - continue; - } - - throw; + Assert.Equal(SocketError.Interrupted, localSocketError); } - } + else + { + Assert.Equal(SocketError.OperationAborted, localSocketError); + } + }, maxAttempts: 10, retryWhen: e => e is XunitException); } [Fact] From 412a1df2bd533132f6bb7838db30b7f1ce0a30ec Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 22 Oct 2020 19:25:05 +0200 Subject: [PATCH 21/26] ConnectGetsCanceledByDispose: use RetryHelper again --- .../tests/FunctionalTests/Connect.cs | 52 ++++++------------- 1 file changed, 17 insertions(+), 35 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs index e9c7abb86420db..40a15f7185dfe9 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs @@ -5,6 +5,7 @@ using System.Threading.Tasks; using Xunit; using Xunit.Abstractions; +using Xunit.Sdk; namespace System.Net.Sockets.Tests { @@ -123,11 +124,10 @@ public async Task Connect_AfterDisconnect_Fails() public static readonly TheoryData ConnectGetsCanceledByDispose_Data = new TheoryData { { IPAddress.Parse("1.1.1.1") }, - // TODO: Figure out how to test this with vanilla IPV6 { IPAddress.Parse("1.1.1.1").MapToIPv6() }, }; - [OuterLoop("Uses external server")] + [OuterLoop("Connects to external server")] [Theory] [MemberData(nameof(ConnectGetsCanceledByDispose_Data))] [PlatformSpecific(~(TestPlatforms.OSX | TestPlatforms.FreeBSD))] // Not supported on BSD like OSes. @@ -136,8 +136,7 @@ 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. int msDelay = 100; - int retries = 10; - while (true) + await RetryHelper.ExecuteAsync(async () => { var client = new Socket(address.AddressFamily, SocketType.Stream, ProtocolType.Tcp); if (address.IsIPv4MappedToIPv6) client.DualMode = true; @@ -149,11 +148,7 @@ public async Task ConnectGetsCanceledByDispose(IPAddress address) msDelay *= 2; Task disposeTask = Task.Run(() => client.Dispose()); - var cts = new CancellationTokenSource(); - Task timeoutTask = Task.Delay(30000, cts.Token); - Assert.NotSame(timeoutTask, await Task.WhenAny(disposeTask, connectTask, timeoutTask)); - cts.Cancel(); - + await Task.WhenAny(disposeTask, connectTask).TimeoutAfter(30000); await disposeTask; SocketError? localSocketError = null; @@ -164,6 +159,9 @@ public async Task ConnectGetsCanceledByDispose(IPAddress address) } catch (SocketException se) { + // On connection timeout, retry. + Assert.NotEqual(SocketError.TimedOut, se.SocketErrorCode); + localSocketError = se.SocketErrorCode; } catch (ObjectDisposedException) @@ -171,36 +169,20 @@ public async Task ConnectGetsCanceledByDispose(IPAddress address) disposedException = true; } - try + if (UsesApm) { - // On connection timeout, retry. - Assert.NotEqual(SocketError.TimedOut, localSocketError); - - if (UsesApm) - { - Assert.Null(localSocketError); - Assert.True(disposedException); - } - else if (UsesSync) - { - Assert.Equal(SocketError.NotSocket, localSocketError); - } - else - { - Assert.Equal(SocketError.OperationAborted, localSocketError); - } - break; + Assert.Null(localSocketError); + Assert.True(disposedException); } - catch + else if (UsesSync) { - if (retries-- > 0) - { - continue; - } - - throw; + Assert.Equal(SocketError.NotSocket, localSocketError); } - } + else + { + Assert.Equal(SocketError.OperationAborted, localSocketError); + } + }, maxAttempts: 10, retryWhen: e => e is XunitException); } } From 86dca506c0c809a6c3d73bd10fce63ab649a1343 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 22 Oct 2020 19:31:03 +0200 Subject: [PATCH 22/26] SyncSendFileGetsCanceledByDispose: use RetryHelper again --- .../tests/FunctionalTests/SendFile.cs | 52 +++++++------------ 1 file changed, 19 insertions(+), 33 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs index 31f3ccea9482d4..0b94dbd6282c0d 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using Xunit; +using Xunit.Sdk; namespace System.Net.Sockets.Tests { @@ -288,8 +289,7 @@ 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; - int retries = 10; - while (true) + await RetryHelper.ExecuteAsync(async () => { (Socket socket1, Socket socket2) = SocketTestExtensions.CreateConnectedSocketPair(); using (socket2) @@ -329,48 +329,34 @@ public async Task SyncSendFileGetsCanceledByDispose() } catch (ObjectDisposedException) { } + Assert.Equal(SocketError.ConnectionAborted, localSocketError); - try + // On OSX, we're unable to unblock the on-going socket operations and + // perform an abortive close. + if (!PlatformDetection.IsOSXLike) { - 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) + SocketError? peerSocketError = null; + var receiveBuffer = new byte[4096]; + while (true) { - SocketError? peerSocketError = null; - var receiveBuffer = new byte[4096]; - while (true) + try { - try + int received = socket2.Receive(receiveBuffer); + if (received == 0) { - int received = socket2.Receive(receiveBuffer); - if (received == 0) - { - break; - } - } - catch (SocketException se) - { - peerSocketError = se.SocketErrorCode; break; } } - Assert.Equal(SocketError.ConnectionReset, peerSocketError); - } - break; - } - catch - { - if (retries-- > 0) - { - continue; + catch (SocketException se) + { + peerSocketError = se.SocketErrorCode; + break; + } } - - throw; + Assert.Equal(SocketError.ConnectionReset, peerSocketError); } } - } + }, maxAttempts: 10, retryWhen: e => e is XunitException); } [OuterLoop] From 5b109e33d8690a1ab2e9e93fb7f877976bcf62c5 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 22 Oct 2020 20:30:57 +0200 Subject: [PATCH 23/26] SendReceive: use RetryHelper again --- .../tests/FunctionalTests/SendFile.cs | 6 +- .../tests/FunctionalTests/SendReceive.cs | 134 +++++++----------- 2 files changed, 52 insertions(+), 88 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs index 0b94dbd6282c0d..539ed40e867af5 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs @@ -311,11 +311,7 @@ await RetryHelper.ExecuteAsync(async () => msDelay *= 2; Task disposeTask = Task.Run(() => socket1.Dispose()); - var cts = new CancellationTokenSource(); - Task timeoutTask = Task.Delay(30000, cts.Token); - Assert.NotSame(timeoutTask, await Task.WhenAny(disposeTask, socketOperation, timeoutTask)); - cts.Cancel(); - + await Task.WhenAny(disposeTask, socketOperation).TimeoutAfter(30000); await disposeTask; SocketError? localSocketError = null; diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs index 6848148d4d5b5e..75978ccddfc91b 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs @@ -10,6 +10,7 @@ using Microsoft.DotNet.XUnitExtensions; using Xunit; using Xunit.Abstractions; +using Xunit.Sdk; namespace System.Net.Sockets.Tests { @@ -953,8 +954,10 @@ 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. int msDelay = 100; - int retries = 10; - while (true) + // 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 () => { var socket = new Socket(address.AddressFamily, SocketType.Dgram, ProtocolType.Udp); if (address.IsIPv4MappedToIPv6) socket.DualMode = true; @@ -967,11 +970,7 @@ public async Task UdpReceiveGetsCanceledByDispose(IPAddress address) msDelay *= 2; Task disposeTask = Task.Run(() => socket.Dispose()); - var cts = new CancellationTokenSource(); - Task timeoutTask = Task.Delay(30000, cts.Token); - Assert.NotSame(timeoutTask, await Task.WhenAny(disposeTask, receiveTask, timeoutTask)); - cts.Cancel(); - + await Task.WhenAny(disposeTask, receiveTask).TimeoutAfter(30000); await disposeTask; SocketError? localSocketError = null; @@ -989,33 +988,20 @@ public async Task UdpReceiveGetsCanceledByDispose(IPAddress address) disposedException = true; } - try + if (UsesApm) { - if (UsesApm) - { - Assert.Null(localSocketError); - Assert.True(disposedException); - } - else if (UsesSync) - { - Assert.Equal(SocketError.Interrupted, localSocketError); - } - else - { - Assert.Equal(SocketError.OperationAborted, localSocketError); - } - break; + Assert.Null(localSocketError); + Assert.True(disposedException); } - catch + else if (UsesSync) { - if (retries-- > 0) - { - continue; - } - - throw; + Assert.Equal(SocketError.Interrupted, localSocketError); } - } + else + { + Assert.Equal(SocketError.OperationAborted, localSocketError); + } + }, maxAttempts: 10, retryWhen: e => e is XunitException); } public static readonly TheoryData TcpReceiveSendGetsCanceledByDispose_Data = new TheoryData @@ -1041,8 +1027,7 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool i // before the operation is started, the peer won't see a ConnectionReset SocketException and we won't // see a SocketException either. int msDelay = 100; - int retries = 10; - while (true) + await RetryHelper.ExecuteAsync(async () => { (Socket socket1, Socket socket2) = SocketTestExtensions.CreateConnectedSocketPair(ipv6Server, dualModeClient); using (socket2) @@ -1069,11 +1054,7 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool i msDelay *= 2; Task disposeTask = Task.Run(() => socket1.Dispose()); - var cts = new CancellationTokenSource(); - Task timeoutTask = Task.Delay(30000, cts.Token); - Assert.NotSame(timeoutTask, await Task.WhenAny(disposeTask, socketOperation, timeoutTask)); - cts.Cancel(); - + await Task.WhenAny(disposeTask, socketOperation).TimeoutAfter(30000); await disposeTask; SocketError? localSocketError = null; @@ -1091,67 +1072,54 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool i disposedException = true; } - try + if (UsesApm) { - if (UsesApm) - { - Assert.Null(localSocketError); - Assert.True(disposedException); - } - else if (UsesSync) - { - Assert.Equal(SocketError.ConnectionAborted, localSocketError); - } - else - { - Assert.Equal(SocketError.OperationAborted, localSocketError); - } + 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)) + // 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) { - SocketError? peerSocketError = null; - var receiveBuffer = new ArraySegment(new byte[4096]); - while (true) + try { - try - { - int received = await ReceiveAsync(socket2, receiveBuffer); - if (received == 0) - { - break; - } - } - catch (SocketException se) + int received = await ReceiveAsync(socket2, receiveBuffer); + if (received == 0) { - peerSocketError = se.SocketErrorCode; break; } } - if (!expectGracefulShutdown) + catch (SocketException se) { - Assert.Equal(SocketError.ConnectionReset, peerSocketError); - } - else - { - Assert.Null(peerSocketError); + peerSocketError = se.SocketErrorCode; + break; } - } - break; - } - catch - { - if (retries-- > 0) + + if (!expectGracefulShutdown) { - continue; + Assert.Equal(SocketError.ConnectionReset, peerSocketError); + } + else + { + Assert.Null(peerSocketError); } - - throw; } } - } + }, maxAttempts: 10, retryWhen: e => e is XunitException); } [Fact] From 9842c814dd56b6bf1130c34ad202c0a73260b983 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 22 Oct 2020 20:32:49 +0200 Subject: [PATCH 24/26] remove empty line --- .../Common/tests/System/Net/Sockets/SocketTestExtensions.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs index ddc09357599c77..b316d8e6e271f5 100644 --- a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs +++ b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs @@ -49,7 +49,6 @@ public static (Socket client, Socket server) CreateConnectedSocketPair(bool ipv6 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); From 1769820f92891b699c3a3047e4d1ac50f3982adf Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 22 Oct 2020 20:36:13 +0200 Subject: [PATCH 25/26] fix build --- .../System.Net.Sockets/tests/FunctionalTests/SendReceive.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs index 75978ccddfc91b..b6d5eea3d38108 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs @@ -951,9 +951,6 @@ error is SocketException || [PlatformSpecific(~TestPlatforms.OSX)] // Not supported on OSX. 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. - int msDelay = 100; // 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; From 7475c155be6a37a281732c1da8fa6d8789f473d6 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 22 Oct 2020 20:39:45 +0200 Subject: [PATCH 26/26] add missing docs --- src/libraries/Common/tests/TestUtilities/System/RetryHelper.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/Common/tests/TestUtilities/System/RetryHelper.cs b/src/libraries/Common/tests/TestUtilities/System/RetryHelper.cs index dabd05dd0d2755..8378d59a8b1a30 100644 --- a/src/libraries/Common/tests/TestUtilities/System/RetryHelper.cs +++ b/src/libraries/Common/tests/TestUtilities/System/RetryHelper.cs @@ -16,6 +16,7 @@ public static partial class RetryHelper /// The maximum number of times to invoke . /// The test to invoke. /// After a failure, invoked to determine how many milliseconds to wait before the next attempt. It's passed the number of iterations attempted. + /// Invoked to select the exceptions to retry on. If not set, any exception will trigger a retry. public static void Execute(Action test, int maxAttempts = 5, Func backoffFunc = null, Predicate retryWhen = null) { // Validate arguments