-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[release/6.0] Remove OpenSSL error queue checking in SslInitializer #65501
Conversation
Tagging subscribers to this area: @dotnet/ncl Issue DetailsBackport of #65435 to release/6.0 /cc @rzikm Customer ImpactTestingRiskIMPORTANT: If this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
|
/cc @bartonjs |
Tagging subscribers to this area: @dotnet/ncl, @vcsjones Issue DetailsBackport of #65435 to release/6.0 Fixes #64492 /cc @rzikm Customer ImpactRace condition between crypto stack and HttpClient/SslStream stack can cause initialization error, making HttpClient/SslStream unusable in the process going forward. When crypto stack runs first, it can leave lingering error in OpenSSL error queue (associated with the thread it ran on). If HttpClient/SslStream run on the same thread (that's the race condition), it checks (unnecessarily) the OpenSSL error queue during initialization and fails due to the lingering error left there by crypto stack. 2 customers hit the problem on regular basis when they start their services (couple of times per day). Regression in 6.0, introduced by #46640 (which made OpenSSL initialization on-demand). There are no good workarounds except process restart, or using private reflection. TestingValidated on targeted repro. RiskLow - removal of unnecessary checking of errors on current thread. Not needed as there was nothing in the intialization that ran and could generate such errors. IMPORTANT: If this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
|
@rzikm @bartonjs @karelz @wfurt @dotnet/ncl Does this change affect any OOB packages that we need to rebuild? The reason I ask is because, per the servicing instructions, we need to make sure backported changes also turn on the Also ask because the change affects a file inside a |
System.Net.Http and System.Net.Security are inbox. I don't know what the status for System.Net.Quic is in release/6.0. @wfurt? |
yes. Quic is part of runtime. For 6.0, it is hidden behind "preview" flag but that does not impact packaging. |
Backport of #65435 to release/6.0
Fixes #64492
/cc @rzikm
Customer Impact
Race condition between crypto stack and HttpClient/SslStream stack can cause initialization error, making HttpClient/SslStream unusable in the process going forward. When crypto stack runs first, it can leave lingering error in OpenSSL error queue (associated with the thread it ran on). If HttpClient/SslStream run on the same thread (that's the race condition), it checks (unnecessarily) the OpenSSL error queue during initialization and fails due to the lingering error left there by crypto stack.
2 customers hit the problem on regular basis when they start their services (couple of times per day).
Regression in 6.0, introduced by #46640 (which made OpenSSL initialization on-demand).
There are no good workarounds except process restart, or using private reflection.
Testing
Validated on targeted repro.
Risk
Low - removal of unnecessary checking of errors on current thread. Not needed as there was nothing in the intialization that ran and could generate such errors.
IMPORTANT: If this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.