Search code examples
c#winapipinvokeroslyn-code-analysissafehandle

Which `IntPtr` flavors are candidates for a `SafeHandle`


I am getting a CA2006 ("Review usage of ... (a 'IntPtr' instance) to determine whether it should be replaced with a SafeHandle or CriticalHandle") for an IntPtr returned from ConvertStringSecurityDescriptorToSecurityDescriptor which I know is a security descriptor that can be freed with a FreeLocal. I am using it only briefly and probably not leaking the memory involved. No kernel handle is associated as far as I can tell.

There are lots of SafeHandle subclasses and none of them appear to be calling FreeLocal at the end of life of the wrapped object. However, I can't find any definitive information concerning which IntPtr instances (say, returned by the Win32 API) are usefully managed by a SafeHandle and which ones are not.

Should I just suppress the CA warning?

More importantly, how to determine the same for the next IntPtr usage I will run into?

(A table that would map SafeHandle subclasses to the functions that will eventually free the handle would be great.)


Solution

  • If your question is "is there already a SafeHandle subclass that calls LocalFree to free its handle", then yes, there is -- it's SafeLocalAllocHandle. The class is internal, so it's not available to the general public, but the code is obvious if you wanted to recreate it. If your question is "should I use such a class", well, that's a bit of a judgment call. CA2006 and the SafeHandle documentation explain the design rationale: SafeHandle avoids issues with handle recycling in multithreaded scenarios and has special support for P/Invokes to ensure handles aren't accidentally freed before unmanaged code is done with them.

    When is an IntPtr a handle that could be wrapped by SafeHandle? You won't be able to tell from the IntPtr, you have to know what function is returning the IntPtr. It will document whether or not you're dealing with a handle and if so, how you should close the handle when you're done with it (very often, but by no means always, CloseHandle). If you only have a P/Invoke signature, then technically, you have nothing. In particular, it will not be possible to determine what function should be called to free the handle, if it is a handle. An IntPtr can also be used to marshal PVOID or SIZE_T or P<some_struct> (even though out or ref parameters are more natural for that) or any other type that's supposed to be pointer-sized.

    Any handle-as-IntPtr that escapes beyond a single function (especially one stored in a field) is a really good candidate for a SafeHandle wrapper, as is any use of multiple IntPtrs at the same time (to prevent accidentally mixing up incompatible handles). For a simple one-off that involves no multithreading and doesn't let the handle escape beyond its scope, a try .. finally block is probably enough. If you want, everything can be wrapped in a SafeHandle for consistency's sake (or to explicitly document what object type you're dealing with), but that doesn't necessarily lead to better results. Handle recycling is a serious issue, but not a concern here since the handle is to local memory, and reliability isn't an issue because any problem serious enough to bypass a finally block is also serious enough to make a small memory leak a non-issue.

    Your particular scenario (parsing SDDL into a security descriptor) is already implemented in the framework in the form of RawSecurityDescriptor, which you should consider using in favor of reinventing the wheel (and/or implementing your own SecurityDescriptor subclass if it's not already covered by the existing classes in System.Security.AccessControl. For what it's worth, RawSecurityDescriptor also P/Invokes to ConvertStringSecurityDescriptorToSecurityDescriptorW and doesn't bother with a SafeHandle. Framework code should not necessarily be treated as a good example of what to do (plenty of code violates the official guidelines), but it's something.