-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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.
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
Assert while running System.Net.Security tests #44689
Comments
Tagging subscribers to this area: @dotnet/ncl Issue Details
|
Triage: Add code to dump the content of the error when we hit this -- then see if we can get more useful info |
The error message is:
Seems to be an issue with some TLSv1 case. |
Configuration: |
I think it can/should be deleted. The SslRead call calls ERR_clear_error() to ensure it has a pristine state. runtime/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c Lines 399 to 403 in 4e75015
|
It was there for a reason. But it seems like we changed strategy how we go about errors. We should make sure we do it consistent as for example we may call handshake after Read() in case of renegotiation. |
Yeah, that's what the ERR_clear_error() is for in CryptoNative_SslRead. Given that we've always been supportive of other libraries calling into OpenSSL (otherwise we wouldn't have had the SafeHandle-accepting RSAOpenSsl/etc ctors) it has always been possible that this assert tripped because of something a third-party library did. The new approach of "make it clean, do work, keep it tidy if you know there was an error you ignored" leads to the error always being relevant (or throwing a very generic exception because something failed with no populated error code). |
Fixes dotnet#44689 As of dotnet#65148, libraries use different approach to handling OpenSSL errors. The original assert which would be hit if a previous operation left errors in the queue is no longer necessary as all OpenSSL calls are prepended by `ERR_clear_error()` to make sure the latest (and most relevant) error is reported.
Fixes #44689 As of #65148, libraries use different approach to handling OpenSSL errors. The original assert which would be hit if a previous operation left errors in the queue is no longer necessary as all OpenSSL calls are prepended by `ERR_clear_error()` to make sure the latest (and most relevant) error is reported.
I did MANY runs (Ubuntu 18.04) and this is first occurrence so it may be hard to reproduce. The assert is guarded by
#if DEBUG
so it should impact released product but it suggests that there was prior error we did not handle or there is some race condition with other thread.The text was updated successfully, but these errors were encountered: