- Notifications
You must be signed in to change notification settings - Fork5.1k
Avoid SafeHandle finalization from failed registry operations#71854
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ghost commentedJul 8, 2022
Tagging subscribers to this area: @dotnet/area-microsoft-win32 Issue DetailsFixes#71773
|
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.
Just curious, do we make any attempt in tests of some sort to look for unintended finalizations? I guess without some kind of diagnostics into the GC there is no reliable way and we just rely on code inspection, and the unlikely situation of it being exposed by what stress tests we do run.
Thinking, perhaps it would look something like: an environment variable to disable the finalizer thread (does that exist?); then manually running a set of unit tests (presumably we have unit tests for invalid reg keys e.g.), forcing GC, and inspecting a dump to see objects pending finalization, perhaps focusing on certain types. Or simply events/tracing of finalizer activity . |
stephentoub commentedJul 11, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
We've done things in the past like this: runtime/src/libraries/Common/src/Microsoft/Win32/SafeHandles/SafeX509Handles.Unix.cs Lines 12 to 26 in60d3d05
I'll play around with what that would look like in the base type. That of course would only handle safe handles, only with a debug/checked runtime, and only if enabled. |
Curious how many hits we'd get if we ran our unit tests now with that pasted into SafeRegistryHandle, e.g.. I'm guessing leaky tests would hide such bugs. Would it be worth creating an issue in case a community member is interested in such an experiment? If it was not too painful they could then try with SafeFileHandle E.g. |
I'm already working on it. It'sa lot. |
Fixes#71773