SafeHandle overloads about vanara HOT 14 CLOSED

I've tried to do that where required. Looks like I missed this one. Thanks. My logic has been to use IntPtr in situations where it is a handle that does not have to be closed or where the closure is handled automatically. If you find situations, like the one above, where I've missed this, please let me know.

from vanara.

Can you show such function ?
Whenever you open a handle you want to close it, and you use SafeHandle wrapper.

In such rare cases you can pass false in ownsHandle parameter:
https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.safehandle.-ctor?view=netframework-4.7.2

from vanara.

Menu handles, Window handles, Resource handles. All of which are closed by the OS during other operations. Often these handles are returned by functions so there is no means to set the ownership to false. If there are areas where you'd like changes, please start submitting PRs.

from vanara.

I see, you can introduce a SafeHandle wrapper which doesn't close.

class SafeMenuHandle : SafeHandle { void ReleaseHandle() {/*nothing*/} }

It is better than just IntPtr since it doesn't allow you to pass incorrect handle type to the function.

from vanara.

I didn't use SafeTokenHandle in the CreateProcessAsUser method (and I'm sure some others) because it would require a project dependency on Vanara.PInvoke.Security which would introduce a circular reference. In general, I agree with your assertion that SafeHandles should be used whenever possible.

from vanara.

It doesn't sound like a good decision.
If you need to break circular refererence you can do it.
Technically you can make manually circular reference but it is not good idea :)

from vanara.

I've been thinking a lot about this post and how I could improve the library and make it more type safe. Will you review the following code? I'm considering making this "breaking" change across the library. I think it captures the need for type specificity and pointer disposal. Effectively there will be a type to match the Window's handle type definition (HWND, HICON, HDC, etc.). In those cases where the creation of the handle requires a subsequent call to close it, there will be a derived type titled SafeHxxx (SafeHWND, SafeHDC, etc.) that will perform that closure call on disposal.

public class HANDLE : SafeHandleZeroOrMinusOneIsInvalid
{
   private Func<IntPtr, bool> closeMeth;
   public HANDLE() : base(true) { }
   protected HANDLE(IntPtr ptr, Func<IntPtr, bool> closeMethod = null, bool ownsHandle = true) : base(ownsHandle) { SetHandle(ptr); }
   public bool IsNull { get { return handle == IntPtr.Zero; } }
   public static readonly HANDLE NULL = new HANDLE();
   protected virtual Func<IntPtr, bool> CloseMethod { get { return closeMeth; } }
   protected override bool ReleaseHandle() { return (IsInvalid || CloseMethod == null) ? true : CloseMethod.Invoke(handle); }
}

public class HICON : HANDLE
{
   public HICON() { }
   protected HICON(IntPtr ptr, Func<IntPtr, bool> closeMethod, bool ownsHandle = true) : base(ptr, closeMethod, ownsHandle) { }
}

public class SafeHICON : HICON
{
   public SafeHICON() : this(IntPtr.Zero) { }
   public SafeHICON(IntPtr hIcon, bool ownsHandle = true) : base(hIcon, DestroyIcon, ownsHandle) { }
}

[DllImport("User32", SetLastError = true, ExactSpelling = true)]
public static extern SafeHICON CreateIcon(HINSTANCE hInstance, int nWidth, int nHeight, byte cPlanes, byte cBitsPixel, [In] byte[] lpbANDbits, [In] byte[] lpbXORbits);

[DllImport("User32", SetLastError = true, ExactSpelling = true)]
[return: MarshalAs(UnmanagedType.Bool)]
public static extern bool DestroyIcon(HICON hIcon);

[DllImport("User32", SetLastError = true, ExactSpelling = true)]
[return: MarshalAs(UnmanagedType.Bool)]
public static extern bool DrawIcon(HDC hDC, int X, int Y, HICON hIcon);

from vanara.

Seems like little bit overcomplicated, especially Func in the constructor.
I guess you don't really need SafeHandle where you don't need it.

If you want C# HICON to be synonym for HANDLE but more type safe you can use simple struct.

[StructLayout(LayoutKind.Explicit, Pack = 0)]
struct HICON 
{
  [FieldOffset(0)]
  private IntPtr handle;
}

This way marshaller will marshal the structure as simple IntPtr but gives you type safety, e.g. you cannot pass HINSTANCE into DestroyIcon.

WDYT ?

from vanara.

I thought about that, but then I can't derive the SafeXX version from it and have it marshaled correctly.

The Func in the constructor just makes it easier to build 30+ derived classes by just supplying a method rather than overloading ReleaseHandle every time.

from vanara.

I think deriving both the raw handle and disposing handle from SafeHandle is important so you get the reference safety ensured by SafeHandle (reference counting like in COM). It also automatically marshals correctly and adds an initial reference. This is particularly important with HWND and HDC that could potentially be disposed while in use. Deriving SafeHXX from raw HXX allows for the method coverage that is always type-safe regardless of using either. For example, take the DrawIcon function. You can create an icon, which has to be disposed, or you can get one of the system icons which cannot be disposed. I can either do this using derived types or have 2 instances of the DrawIcon method. Derived types seems cleaner.

from vanara.

Your use case is duality of HMENU etc.
Like GetMenu doesn't need to be destroyed but CreateMenu does. Correct?

So for first you want SafeHandle that doesn't destroy and for second you do want.
E.g.:

public static extern HMENU GetMenu();

public static extern SafeMenuHandle CreateMenu();

public static extern void DestroyMenu(HMENU menu);

Next is have two types of overloads.

If you want life simpler than you can do what you proposed just give better names.
HMENU name implies that it is not safehandle and you don't need to dispose it.

As for Func parameter it involves additional member instead of overloading function.
In the end it is the same amount of code but without Func you have smaller objects which better for GC.
If you have too much copy paste code just use T4, it is not so bad as you might think.

from vanara.

I just checked in a ton of changes implemented the idea above. It will seriously screw anyone who has used the library in the past, but I think it is a major improvement. Thanks for pushing the idea. Give the posts a look and see what you think. I think I will have to change the version to 2.0 and popup a web page explaining the changes.

from vanara.

I'll take a look ASAP.
IMO SafeIconHandle is better name than SafeHICON.
You have several examples for this convention even in .NET itself.
https://github.com/dotnet/corefx/tree/master/src/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles

from vanara.

I thought about that naming for the exact reason you suggest, but think that parity with the Windows API, especially reading their documentation, makes keeping with the API names a better option. I do that with functions, enums and structures, so I can't justify deviating for handles.

from vanara.