Cannot pass SafeHandle instance in ReleaseHandle to native method

501 views Asked by At

I just recently learned about SafeHandle and, for a test, I implemented it for the SDL2 library, creating and destroying a window:

[DllImport(_libName, CallingConvention = CallingConvention.Cdecl)]
internal static extern IntPtr SDL_CreateWindow(
    [MarshalAs(UnmanagedType.LPStr)] string title, int x, int y, int w, int h, uint flags);

[DllImport(_libName, CallingConvention = CallingConvention.Cdecl)]
internal static extern void SDL_DestroyWindow(IntPtr window);

public class Window : SafeHandleZeroOrMinusOneIsInvalid
{
    public Window() : base(true)
    {
        SetHandle(SDL_CreateWindow("Hello", 400, 400, 800, 600, 0));
    }

    protected override bool ReleaseHandle()
    {
        SDL_DestroyWindow(handle);
        return true;
    }
}

This works fine, then I learned of another advantage of using SafeHandle: The possibility to use the class in the p/invoke signature directly, like so:

[DllImport(_libName, CallingConvention = CallingConvention.Cdecl)]
internal static extern Window SDL_CreateWindow(
    [MarshalAs(UnmanagedType.LPStr)] string title, int x, int y, int w, int h, uint flags);

[DllImport(_libName, CallingConvention = CallingConvention.Cdecl)]
internal static extern void SDL_DestroyWindow(Window window);

This is of course much better than generic IntPtr parameters / returns, because I have type safety passing / retrieving actual Window (handles) to / from those methods.

While this works for SDL_CreateWindow, which correctly returns a Window instance now, it doesn't work for SDL_DestroyWindow, which is called by me inside of Window.ReleaseHandle like this:

public Window() : base(true)
{
    SetHandle(SDL_CreateWindow("Hello", 400, 400, 800, 600, 0).handle);
}

protected override bool ReleaseHandle()
{
    SDL_DestroyWindow(this);
    return true;
}

When trying to pass this to SDL_DestroyWindow, I get an ObjectDisposedException: Safe handle has been closed. Indeed the IsClosed property is true, which I didn't expect to be at this time. Apparently it internally tries to increase a reference count, but notices IsClosed is true. According to the documentation, it has been set to true because "The Dispose method or Close method was called and there are no references to the SafeHandle object on other threads.", so I guess Dispose was implicitly called before in the call stack to invoke my ReleaseHandle.

ReleaseHandle apparently isn't the correct place to clean up if I want to use a class parameter in the p/invoke signature, so I wonder if there is any method where I could clean up without breaking SafeHandle internals?

1

There are 1 answers

1
Ray On BEST ANSWER

My question above is slightly misguided by wrong information I learned about SafeHandle (via some blog posts I won't mention). While I was told that replacing IntPtr parameters in P/Invoke methods with class instances is "the main advantage provided by SafeHandle" and definitely nice, it turns out to be only partly useful:

Careful with automatic SafeHandle creation by the marshaller

For one, I say this because my code above has a big issue I didn't see at first. I have written this code:

void DoStuff()
{
    Window window = new Window();
}

public class Window : SafeHandleZeroOrMinusOneIsInvalid
{
    public Window() : base(true)
    {
        // SDL_CreateWindow will create another `Window` instance internally!!
        SetHandle(SDL_CreateWindow("Hello", 400, 400, 800, 600, 0).handle);
    }

    protected override bool ReleaseHandle()
    {
        SDL_DestroyWindow(handle); // Since "this" won't work here (s. below)
        return true;
    }

    // Returns Window instance rather than IntPtr via the automatic SafeHandle creation
    [DllImport(_libName, CallingConvention = CallingConvention.Cdecl)]
    private static extern Window SDL_CreateWindow(
        [MarshalAs(UnmanagedType.LPStr)] string title, int x, int y, int w, int h, uint flags);

    // Accept Window instance rather than IntPtr (won't work out, s. below)
    [DllImport(_libName, CallingConvention = CallingConvention.Cdecl)]
    private static extern void SDL_DestroyWindow(Window window);
}

When the marshaller calls the P/Invoke method of SDL_CreateWindow in the Window constructor, it internally creates another instance of the Window class for the return value (calling the required parameterless constructor and then setting the handle member internally). This means I now have two SafeHandle instances:

  • one returned by the SDL_CreateWindow method - which I don't use anywhere (only stripping out the handle property)
  • one created by my user code calling new Window(), the SafeHandle class itself

The only correct way to implement the SafeHandle here is to let SDL_CreateWindow return an IntPtr again, so no internal marshalling SafeHandle instances are created anymore.

Can't pass SafeHandle around in ReleaseHandle

As Simon Mourier explained / cited in the comments, the SafeHandle itself cannot be used at all anymore when cleaning up in ReleaseHandle, since the object is garbage collected and trying to do "fancy" things like passing it to a P/Invoke method is not safe anymore / doomed to fail. (Given I were told that the replacement of IntPtr parameters in the P/Invoke is one of "the main features" of SafeHandle, it surprised me first that this is not supported and deemed "fancy"). That is also why the ObjectDisposedException I receive is very justified.

I can still access the handle property here, but then, again, my P/Invoke method no longer accepts a Window instance, but the "classic" IntPtr.

Am I better off with IntPtr for P/invoke parameters again?

I'd say so, my final implementation looks like this and solves the above two problems while still using the advantages of SafeHandle, just without the fancy P/Invoke argument replacements. And as an extra feature, I can still connote the IntPtr parameters to "accept" an SDL_Window (the native type pointed to) with a using alias.

using SDL_Window = System.IntPtr;

public class Window : SafeHandleZeroOrMinusOneIsInvalid
{
    private Window(IntPtr handle) : base(true)
    {
        SetHandle(handle);
    }

    public Window() : this(SDL_CreateWindow("Hello", 400, 400, 800, 600, 0)) { }

    protected override bool ReleaseHandle()
    {
        SDL_DestroyWindow(handle);
        return true;
    }

    [DllImport(_libName, CallingConvention = CallingConvention.Cdecl)]
    private static extern SDL_Window SDL_CreateWindow(
        [MarshalAs(UnmanagedType.LPStr)] string title, int x, int y, int w, int h, uint flags);

    [DllImport(_libName, CallingConvention = CallingConvention.Cdecl)]
    private static extern void SDL_DestroyWindow(SDL_Window window);
}