microsoft / CsWin32

A source generator to add a user-defined set of Win32 P/Invoke methods and supporting types to a C# project.
MIT License
2.13k stars 94 forks source link

Implement IHandle and add associated overloads to methods accepting IntPtr #602

Open elachlan opened 2 years ago

elachlan commented 2 years ago

Is your feature request related to a problem? Please describe. In winforms, an IHandle is used to provide "abstract access to classes that contain a potentially owned handle." https://github.com/dotnet/winforms/blob/91f3f2395f156265bdd8f2d8e78361f786015cf8/src/System.Windows.Forms.Primitives/src/Interop/IHandle.cs#L1-L24

I am looking at converting Winforms to use CsWin32, and stumbled into usages that make use of IHandle. They are used to wrap the pinvoke call and then call GC.KeepAlive on the IHandle.

https://github.com/dotnet/winforms/blob/91f3f2395f156265bdd8f2d8e78361f786015cf8/src/System.Windows.Forms.Primitives/src/Interop/User32/Interop.ValidateRect.cs#L1-L21

Should an overload be provided by CsWin32 that provides this functionality? Does it already do so? Or should I keep the wrapper and remove the PInvoke?

jnm2 commented 2 years ago

The IHandle interface you mention appears to be internal to Windows Forms and this is the first I've ever heard of it.

The standard pattern is to use the public HandleRef struct. The interop stub calls GC.KeepAlive on HandleRef.Object and passes HandleRef.Handle to the native call.

JeremyKuhne commented 2 years ago

The IHandle interface you mention appears to be internal to Windows Forms and this is the first I've ever heard of it.

It is a pattern we use to "bind" a handle to its owner. A common mistake made in the WinForms runtime was associating the wrong owner with a given handle with GCHandle. Code would either not get the right finalizable object associated or randomly pick a class for handles that came straight from Windows.

It also allows for slightly more efficient handling as you're not creating and filling a temporary struct and making the generated code more complicated. It also allows avoiding boxing of struct handle wrappers when you write wrappers as generic and constrain to the interface. .NET is smart enough to not introduce a box when you pass a struct to GC.KeepAlive (at least in current versions).

Note that I plan to iterate on it and make it an IHandle<T> (IHandle<HWND> for example). I've prototyped, but haven't found the time to implement it yet.

You can also use this pattern to associate extension methods with an "owned" handle. Something like:

    public static unsafe Rectangle GetClientRectangle<T>(this T window)
        where T : IHandle<HWND>
    {
        Unsafe.SkipInit(out RECT rect);
        Error.ThrowLastErrorIfFalse(PInvoke.GetClientRect(window.Handle, &rect));
        GC.KeepAlive(window);
        return rect.ToRectangle();
    }

Providing the same pattern here might not be practical, but it is useful to understand and consider.

elachlan commented 2 years ago

Probably a good idea to implement it along side https://github.com/microsoft/CsWin32/issues/596.

elachlan commented 2 years ago

@JeremyKuhne Did you want this all handled in winforms? or did you want CsWin32 to eventually emit it?

JeremyKuhne commented 2 years ago

Did you want this all handled in winforms? or did you want CsWin32 to eventually emit it?

It would be nice to have CsWin32 have the type at least and possibly the ability to generate "friendly" overloads that use it. I'm currently working on a PR in WinForms that more fully fleshes this out and will follow up on this issue with details about the patterns.