Fix SslStreamDisposeTest failures during parallel handshake#113834
Conversation
There was a problem hiding this comment.
Pull Request Overview
The PR aims to address SslStreamDisposeTest failures by using real TCP streams and by improving exception handling during parallel handshakes.
- Replaced ConnectedSslStreams with real Tcp streams to avoid issues with concurrent disposal.
- Updated the using blocks to reflect the new stream instantiation.
- Expanded the exception catch logic to include AuthenticationException for more robust test validation.
Comments suppressed due to low confidence (2)
src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamDisposeTest.cs:119
- [nitpick] Consider renaming the 'server' variable to 'serverSslStream' for clarity and consistency with the client SslStream variable.
using SslStream server = new SslStream(serverStream);
src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamDisposeTest.cs:151
- Consider adding a dedicated test that simulates an AuthenticationException scenario to verify that this expected case is handled correctly.
catch (Exception ex) when (ex
is ObjectDisposedException // disposed locally
or IOException // disposed remotely (received unexpected EOF)
or AuthenticationException) // disposed wrapped in AuthenticationException or error from platform library
rzikm
left a comment
There was a problem hiding this comment.
/azp run runtime-libraries-coreclr outerloop
wfurt
left a comment
There was a problem hiding this comment.
while the platform noise is unpleased IMHO I feel it is OK since we may be breaking operations in-flight. We would probably need to check disposed flag on failure to see that the failure is result of Dispose and throw ODE instead.
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
all System.Net.Security.Tests pass, good to merge. |
Fixes #113834.
3 distinct types of test failures