-
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
One-shot hash feedback #39372
One-shot hash feedback #39372
Conversation
@@ -24,12 +29,86 @@ public static HashProvider CreateMacProvider(string hashAlgorithmId, ReadOnlySpa | |||
|
|||
public static class OneShotHashProvider | |||
{ | |||
private static volatile bool s_UseCompatOneShot; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private static volatile bool s_UseCompatOneShot; | |
private static volatile bool s_useCompatOneShot; |
....Security.Cryptography.Algorithms/src/Internal/Cryptography/HashProviderDispenser.Windows.cs
Show resolved
Hide resolved
....Security.Cryptography.Algorithms/src/Internal/Cryptography/HashProviderDispenser.Windows.cs
Show resolved
Hide resolved
....Security.Cryptography.Algorithms/src/Internal/Cryptography/HashProviderDispenser.Windows.cs
Show resolved
Hide resolved
You marked this as fixing the issue, but it isn't doing the EVP algorithm handle caching. (The easiest fix on that is to just change the |
@bartonjs Was this change perhaps overlooked, or am I holding it wrong? https://github.com/dotnet/runtime/pull/39372/files#diff-283eaf9255778de038e266b90f9bea05R32 |
Ah, looks like the notification I opened was for the second commit being pushed, instead of the root of the PR. I feel better now as to why it wasn't on my screen, but am nonetheless embarrassed and apologetic. Since I don't see any other calls to those interop methods the place you decided to cache them seems fine. |
All good! As always I appreciate the quick and thorough reviews. |
This addresses the feedback from #17590.
I wasn't able to find any good examples of "Use this Win32 API if it's available, otherwise emulate it". If there is a example to model that off of other than what I did, happy to re-do the approach.
Fixes #17590.