From 1a88017f0d893c109570988708c2bda525338e29 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 5 Feb 2020 16:17:20 +0100 Subject: [PATCH 1/6] reduce execution time of handle inheritance tests --- .../tests/FunctionalTests/CreateSocketTests.cs | 15 +++++++++++---- .../FunctionalTests/SocketDuplicationTests.cs | 14 +++++++++----- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/CreateSocketTests.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/CreateSocketTests.cs index bcb9be6e71574a..87ebf0736d8c09 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/CreateSocketTests.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/CreateSocketTests.cs @@ -122,6 +122,9 @@ public void Ctor_Raw_NotSupported_ExpectedError(AddressFamily addressFamily, Pro [InlineData(false, 2)] public void CtorAndAccept_SocketNotKeptAliveViaInheritance(bool validateClientOuter, int acceptApiOuter) { + // 300 ms should be long enough to connect if the socket is actually present & listening. + const int ConnectionTimeoutMs = 300; + // Run the test in another process so as to not have trouble with other tests // launching child processes that might impact inheritance. RemoteExecutor.Invoke((validateClientString, acceptApiString) => @@ -186,11 +189,15 @@ public void CtorAndAccept_SocketNotKeptAliveViaInheritance(bool validateClientOu listener.Dispose(); using (var tmpClient = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp)) { - Assert.ThrowsAny(() => tmpClient.Connect(ep)); - } + IAsyncResult connectResult = tmpClient.BeginConnect(ep, null, null); + bool connected = connectResult.AsyncWaitHandle.WaitOne(ConnectionTimeoutMs); + connectResult.AsyncWaitHandle.Close(); - // Let the child process terminate. - serverPipe.WriteByte(42); + // Let the child process terminate. + serverPipe.WriteByte(42); + + Assert.False(connected); + } } } } diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SocketDuplicationTests.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SocketDuplicationTests.cs index 7aababd00fd90d..cb1484aaf50106 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SocketDuplicationTests.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SocketDuplicationTests.cs @@ -167,11 +167,13 @@ public async Task DuplicateAndClose_TcpListener() Assert.Equal(TestMessage, receivedMessage); } - [OuterLoop] // long-running [PlatformSpecific(TestPlatforms.Windows)] [Fact] public void DuplicateSocket_IsNotInheritable() { + // 300 ms should be long enough to connect if the socket is actually present & listening. + const int ConnectionTimeoutMs = 300; + // The test is based on CreateSocket.CtorAndAccept_SocketNotKeptAliveViaInheritance, // but contains simpler validation logic, sufficient to test the behavior on Windows static void RunTest() @@ -201,11 +203,13 @@ static void ChildProcessBody(string clientPipeHandle) // Close the listening socket: listenerDuplicate.Dispose(); - // Validate that we after closing the listening socket, we're not able to connect: - SocketException ex = Assert.ThrowsAny(() => client.Connect(ep)); - Debug.Print(ex.Message); - + IAsyncResult connectResult = client.BeginConnect(ep, null, null); + bool connected = connectResult.AsyncWaitHandle.WaitOne(ConnectionTimeoutMs); + connectResult.AsyncWaitHandle.Close(); serverPipe.WriteByte(42); + + // Validate that we after closing the listening socket, we're not able to connect: + Assert.False(connected); } } From 35848f8f08d366a5e7c1e675b66a5a27fa35c0e5 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 5 Feb 2020 19:18:00 +0100 Subject: [PATCH 2/6] expect error when connection did not time out --- .../Net/Sockets/SocketTestExtensions.cs | 26 +++++++++++++++++++ .../FunctionalTests/CreateSocketTests.cs | 4 +-- .../FunctionalTests/SocketDuplicationTests.cs | 7 ++--- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs index 76043a3d3653e9..443afdf5d43d51 100644 --- a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs +++ b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs @@ -49,5 +49,31 @@ public static (Socket, Socket) CreateConnectedSocketPair() return (client, server); } + + // 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) + { + IAsyncResult connectResult = socket.BeginConnect(remoteEndpoint, null, null); + if (connectResult.AsyncWaitHandle.WaitOne(millisecondsTimeout)) + { + try + { + socket.EndConnect(connectResult); + return true; + } + catch (SocketException) + { + return false; + } + finally + { + connectResult.AsyncWaitHandle.Close(); + } + } + connectResult.AsyncWaitHandle.Close(); + + return false; + } } } diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/CreateSocketTests.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/CreateSocketTests.cs index 87ebf0736d8c09..9fc617fd20bc9e 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/CreateSocketTests.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/CreateSocketTests.cs @@ -189,9 +189,7 @@ public void CtorAndAccept_SocketNotKeptAliveViaInheritance(bool validateClientOu listener.Dispose(); using (var tmpClient = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp)) { - IAsyncResult connectResult = tmpClient.BeginConnect(ep, null, null); - bool connected = connectResult.AsyncWaitHandle.WaitOne(ConnectionTimeoutMs); - connectResult.AsyncWaitHandle.Close(); + bool connected = tmpClient.TryConnect(ep, ConnectionTimeoutMs); // Let the child process terminate. serverPipe.WriteByte(42); diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SocketDuplicationTests.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SocketDuplicationTests.cs index cb1484aaf50106..ae370c770188ea 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SocketDuplicationTests.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SocketDuplicationTests.cs @@ -203,12 +203,9 @@ static void ChildProcessBody(string clientPipeHandle) // Close the listening socket: listenerDuplicate.Dispose(); - IAsyncResult connectResult = client.BeginConnect(ep, null, null); - bool connected = connectResult.AsyncWaitHandle.WaitOne(ConnectionTimeoutMs); - connectResult.AsyncWaitHandle.Close(); - serverPipe.WriteByte(42); - // Validate that we after closing the listening socket, we're not able to connect: + bool connected = client.TryConnect(ep, ConnectionTimeoutMs); + serverPipe.WriteByte(42); Assert.False(connected); } } From 5a3897018e3cb354e008dfa034bfa1ca4fdd48d2 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 6 Feb 2020 00:56:28 +0100 Subject: [PATCH 3/6] use SocketAsyncEventArgs --- .../Net/Sockets/SocketTestExtensions.cs | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs index 443afdf5d43d51..7a380a71cf0573 100644 --- a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs +++ b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs @@ -2,6 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Diagnostics; +using System.Threading; + namespace System.Net.Sockets.Tests { internal static class SocketTestExtensions @@ -54,25 +57,26 @@ public static (Socket, Socket) CreateConnectedSocketPair() // Useful to speed up "can not connect" assertions on Windows public static bool TryConnect(this Socket socket, EndPoint remoteEndpoint, int millisecondsTimeout) { - IAsyncResult connectResult = socket.BeginConnect(remoteEndpoint, null, null); - if (connectResult.AsyncWaitHandle.WaitOne(millisecondsTimeout)) + using var mre = new ManualResetEvent(false); + using var sea = new SocketAsyncEventArgs() + { + RemoteEndPoint = remoteEndpoint, + UserToken = mre + }; + + sea.Completed += (s, e) => + { + if (e.UserToken is ManualResetEvent mreInner) mreInner.Set(); + }; + + bool pending = socket.ConnectAsync(sea); + if (!pending || mre.WaitOne(millisecondsTimeout)) { - try - { - socket.EndConnect(connectResult); - return true; - } - catch (SocketException) - { - return false; - } - finally - { - connectResult.AsyncWaitHandle.Close(); - } + return sea.SocketError == SocketError.Success; } - connectResult.AsyncWaitHandle.Close(); + sea.UserToken = null; // Letting the event handler know that we timed out + Socket.CancelConnectAsync(sea); // this will close the socket! return false; } } From 938ebe17c415839e8c7b94a6f9c0d83572078e77 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 6 Feb 2020 01:06:58 +0100 Subject: [PATCH 4/6] remove unnecessary using --- .../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 7a380a71cf0573..f648f1e19b3059 100644 --- a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs +++ b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Diagnostics; using System.Threading; namespace System.Net.Sockets.Tests From 7bede7f8a598f85bcd9c2020216618dc453e3602 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 6 Feb 2020 18:57:51 +0100 Subject: [PATCH 5/6] use ManualResetEventSlim, and do not dispose it --- .../tests/System/Net/Sockets/SocketTestExtensions.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs index f648f1e19b3059..f90b92753efeef 100644 --- a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs +++ b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs @@ -56,25 +56,23 @@ public static (Socket, Socket) CreateConnectedSocketPair() // Useful to speed up "can not connect" assertions on Windows public static bool TryConnect(this Socket socket, EndPoint remoteEndpoint, int millisecondsTimeout) { - using var mre = new ManualResetEvent(false); + // To avoid race conditions, ManualResetEventSlim is left undisposed, + // letting SafeHandle's finalizer do the cleanup work, if needed + var mre = new ManualResetEventSlim(false); using var sea = new SocketAsyncEventArgs() { RemoteEndPoint = remoteEndpoint, UserToken = mre }; - sea.Completed += (s, e) => - { - if (e.UserToken is ManualResetEvent mreInner) mreInner.Set(); - }; + sea.Completed += (s, e) => ((ManualResetEventSlim)e.UserToken).Set(); bool pending = socket.ConnectAsync(sea); - if (!pending || mre.WaitOne(millisecondsTimeout)) + if (!pending || mre.Wait(millisecondsTimeout)) { return sea.SocketError == SocketError.Success; } - sea.UserToken = null; // Letting the event handler know that we timed out Socket.CancelConnectAsync(sea); // this will close the socket! return false; } From ad2cdf977a584b82142e8c30d832f7c9c0db8a3a Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Fri, 14 Feb 2020 15:21:34 +0100 Subject: [PATCH 6/6] Dispose MRE when no timeout occurs --- .../Common/tests/System/Net/Sockets/SocketTestExtensions.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs index f90b92753efeef..f386473f06f81f 100644 --- a/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs +++ b/src/libraries/Common/tests/System/Net/Sockets/SocketTestExtensions.cs @@ -56,8 +56,6 @@ public static (Socket, Socket) CreateConnectedSocketPair() // Useful to speed up "can not connect" assertions on Windows public static bool TryConnect(this Socket socket, EndPoint remoteEndpoint, int millisecondsTimeout) { - // To avoid race conditions, ManualResetEventSlim is left undisposed, - // letting SafeHandle's finalizer do the cleanup work, if needed var mre = new ManualResetEventSlim(false); using var sea = new SocketAsyncEventArgs() { @@ -70,10 +68,14 @@ public static bool TryConnect(this Socket socket, EndPoint remoteEndpoint, int m bool pending = socket.ConnectAsync(sea); if (!pending || mre.Wait(millisecondsTimeout)) { + mre.Dispose(); return sea.SocketError == SocketError.Success; } Socket.CancelConnectAsync(sea); // this will close the socket! + + // In case of time-out, ManualResetEventSlim is left undisposed to avoid race conditions, + // letting SafeHandle's finalizer to do the cleanup. return false; } }