Skip to content

Await pending client authentication task before disposing#66682

Merged
rzikm merged 1 commit intodotnet:mainfrom
rzikm:65455-SslStream-assert
Mar 16, 2022
Merged

Await pending client authentication task before disposing#66682
rzikm merged 1 commit intodotnet:mainfrom
rzikm:65455-SslStream-assert

Conversation

@rzikm
Copy link
Copy Markdown
Member

@rzikm rzikm commented Mar 15, 2022

Fixes #65455.

The assert was hit due to the SslStream being disposed during handshake, while AuthenticateAsClientAsync was still running.

Fixes dotnet#65455.

The assert was hit due to the SslStream being disposed during handshake, while `AuthenticateAsClientAsync` was still running.
@ghost ghost assigned rzikm Mar 15, 2022
@ghost
Copy link
Copy Markdown

ghost commented Mar 15, 2022

Tagging subscribers to this area: @dotnet/ncl, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #65455.

The assert was hit due to the SslStream being disposed during handshake, while AuthenticateAsClientAsync was still running.

Author: rzikm
Assignees: -
Labels:

area-System.Net.Security

Milestone: -

@rzikm
Copy link
Copy Markdown
Member Author

rzikm commented Mar 16, 2022

Test failures are unrelated

@rzikm rzikm merged commit 7c89933 into dotnet:main Mar 16, 2022
@wfurt
Copy link
Copy Markdown
Member

wfurt commented Mar 18, 2022

I think this is wrong fix. SslStream can be disposed while in use. Avoiding that in our tests is not the right answer IMHO.

@rzikm
Copy link
Copy Markdown
Member Author

rzikm commented Mar 18, 2022

I Agree @wfurt , we should take a deeper look into this, but meanwhile, we can at least have cleaner CI runs.
I created a separate issue for addressing the assert: #66818, it did not seem right to reopen the original one

radekdoulik pushed a commit to radekdoulik/runtime that referenced this pull request Mar 30, 2022
Fixes dotnet#65455.

The assert was hit due to the SslStream being disposed during handshake, while `AuthenticateAsClientAsync` was still running. This removes a rare crash in test runs with debug libraries configuration.
@karelz karelz added this to the 7.0.0 milestone Apr 8, 2022
@ghost ghost locked as resolved and limited conversation to collaborators May 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SslStream tests can hit Assert and fail completely

4 participants