-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Tagging subscribers to this area: @dotnet/area-microsoft-win32 Issue DetailsFixes #71773
|
danmoseley
left a comment
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 . |
We've done things in the past like this: runtime/src/libraries/Common/src/Microsoft/Win32/SafeHandles/SafeX509Handles.Unix.cs Lines 12 to 26 in 60d3d05
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's a lot. |
Fixes #71773