Remove Debug.Assert in SetSslCertificate that aborts process during dispose-during-handshake race#124631
Remove Debug.Assert in SetSslCertificate that aborts process during dispose-during-handshake race#124631
Conversation
…ng-handshake race Co-authored-by: rzikm <32671551+rzikm@users.noreply.github.com>
|
Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones |
There was a problem hiding this comment.
Pull request overview
This PR fixes a test crash caused by Debug.Assert statements that abort the process during a race condition between Dispose() and AuthenticateAsServerAsync. The assertions were checking SafeHandle validity before passing certificate and key handles to OpenSSL p/invoke methods. When the handles become invalid due to disposal during the handshake, the asserts fire and terminate the process with SIGABRT instead of allowing the p/invoke marshaller to throw ObjectDisposedException as expected.
Changes:
- Remove two
Debug.Assertstatements inSetSslCertificatethat check certificate and key handle validity - Allow the .NET p/invoke marshaller to naturally throw
ObjectDisposedExceptionwhen closed SafeHandles are passed across the native boundary
|
|
||
| private static void SetSslCertificate(SafeSslContextHandle contextPtr, SafeX509Handle certPtr, SafeEvpPKeyHandle keyPtr) | ||
| { | ||
| Debug.Assert(certPtr != null && !certPtr.IsInvalid); |
There was a problem hiding this comment.
What will happen if we pass NULL to the native call? Should we check and simply return on such condition?
During the race between
AuthenticateAsServerAsyncandDispose(), certificate/keySafeHandles can become invalid before reachingSetSslCertificate. TheDebug.Asserton handle validity fires and terminates the process with SIGABRT (exit code 134) instead of letting the p/invoke marshaller throwObjectDisposedExceptionas expected.Changes
Interop.OpenSsl.cs: Remove the twoDebug.Assertvalidity checks inSetSslCertificate. The p/invoke marshaller already throwsObjectDisposedExceptionwhen closedSafeHandles are passed across the native boundary, making these asserts both redundant and harmful.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.