Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Socket tests: don't retry CanceledByDispose tests on timeout #42725

Closed
wants to merge 4 commits into from
Closed

Socket tests: don't retry CanceledByDispose tests on timeout #42725

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9f2c092
Socket tests: don't retry CanceledByDispose tests on timeout
tmds Sep 25, 2020
37176da
print g_config_specified_ciphersuites
tmds Sep 28, 2020
b7b8d2c
Move connection timeout retry
tmds Oct 1, 2020
ee3dd19
Undo unrelated changes
tmds Oct 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <assert.h>
#include <string.h>
#include <stdbool.h>
#include <unistd.h>

c_static_assert(PAL_SSL_ERROR_NONE == SSL_ERROR_NONE);
c_static_assert(PAL_SSL_ERROR_SSL == SSL_ERROR_SSL);
Expand Down Expand Up @@ -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);
tmds marked this conversation as resolved.
Show resolved Hide resolved
}

void CryptoNative_EnsureLibSslInitialized()
Expand Down
36 changes: 25 additions & 11 deletions src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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]
Expand Down
42 changes: 28 additions & 14 deletions src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

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

                catch (SocketException se)
                {
                    // On connection timeout, retry.
                    Assert.NotEqual(SocketError.TimedOut, se.SocketErrorCode);

                    localSocketError = se.SocketErrorCode;
                }

The test fails here on Windows. Should do some more research on SocketError.TimedOut cases, but I think we should keep retrying here.

{
var client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);

Expand All @@ -153,30 +154,43 @@ await RetryHelper.ExecuteAsync(async () =>
}
catch (SocketException se)
{
// On connection timeout, retry.
Assert.NotEqual(SocketError.TimedOut, se.SocketErrorCode);

localSocketError = se.SocketErrorCode;
}
catch (ObjectDisposedException)
{
disposedException = true;
}

if (UsesApm)
{
Assert.Null(localSocketError);
Assert.True(disposedException);
}
else if (UsesSync)
try
{
Assert.Equal(SocketError.NotSocket, localSocketError);
// 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;
}
else
catch
{
Assert.Equal(SocketError.OperationAborted, localSocketError);
if (retries-- > 0)
{
continue;
}

throw;
}
}, maxAttempts: 10);
}
}
}

Expand Down
51 changes: 33 additions & 18 deletions src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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]
Expand Down
110 changes: 69 additions & 41 deletions src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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]
Expand All @@ -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)
Expand Down Expand Up @@ -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<byte>(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<byte>(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]
Expand Down