Skip to content

Commit

Permalink
Fix test after TcpClient change (dotnet#35721)
Browse files Browse the repository at this point in the history
My change to TcpClient.ConnectAsync resulted in a sporadic test failure.  ConnectAsync had been implemented in terms of Begin/EndConnect.  It would call BeginConnect, and then the callback invoked when the asynchronous operation completed would call EndConnect.  The very first thing EndConnect would do is check whether the TcpClient was disposed, and throw an ObjectDisposedException if it is, ignoring the actual result or failure in the callback.  By changing the ConnectAsync implementation to bypass Begin/EndConnect, it's also skipping that check, and so if you dispose of the TcpClient while the operation is in flight, you'll end up with a SocketError.OperationAborted SocketException (exactly what you get from Socket) rather than an ObjectDisposedException.

Since SocketException was already possible from TcpClient.ConnectAsync (and is documented as such), since we want an actual error to propagate rather than hiding it with a different one, since this is about behavior after dispose, and since this is effectively an implementation detail that was leaking out, I've opted to fix the test rather than change the implementation.
  • Loading branch information
stephentoub authored May 2, 2020
1 parent 44a3313 commit 847a054
Showing 1 changed file with 2 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -429,12 +429,12 @@ public async Task Dispose_CancelsConnectAsync(bool connectByName)

// There is a race condition here. If the connection succeeds before the
// disposal, then the task will complete successfully. Otherwise, it should
// fail with an ObjectDisposedException.
// fail with an exception.
try
{
await connectTask;
}
catch (ObjectDisposedException) { }
catch (SocketException e) when (e.SocketErrorCode == SocketError.OperationAborted) { }
sw.Stop();

Assert.Null(client.Client); // should be nulled out after Dispose
Expand Down

0 comments on commit 847a054

Please sign in to comment.