Is DangerousAddRef/GetHandle usage correct?

[User1785604260]

I needed to use DangerousGetHandle in some of my own code, and wanted to use the generated CsWin32 code as a reference to see if I was doing so correctly. In looking at the generated CsWin32 code, I'm now wondering if its DangerousAddRef/GetHandle usage is correct. I am 1000% not an expert on this, so I may be completely incorrect.

What I noticed is that the success of DangerousAddRef is only used to determine whether to call DangerousRelease. If DangerousAddRef fails, the generated code still proceeds to call DangerousGetHandle and use that handle. From what I can tell this is incorrect, as failure means that the handle is closed if I'm understanding correctly.

If failure shouldn't be ignored, I'm not sure what the correct behavior would be instead. Maybe throwing ObjectDisposedException?

For example, here's an arbitrary method I picked that uses DangerousGetHandle:

public static unsafe int DestroyPhysicalMonitor(SafeHandle hMonitor)
{
    bool hMonitorAddRef = false;
    try
    {
        winmdroot.Foundation.HANDLE hMonitorLocal;
        if (hMonitor is object)
        {
            hMonitor.DangerousAddRef(ref hMonitorAddRef);
            hMonitorLocal = (winmdroot.Foundation.HANDLE)hMonitor.DangerousGetHandle();
        }
        else
            hMonitorLocal = default(winmdroot.Foundation.HANDLE);
        int __result = PInvoke.DestroyPhysicalMonitor(hMonitorLocal);
        return __result;
    }
    finally
    {
        if (hMonitorAddRef)
            hMonitor.DangerousRelease();
    }
}

On a more advanced note, the documentation for DangerousAddRef also indicates that a Constrained Execution Region should be used. I've never touched those and don't know what proper usage of that would look like, and if it's relevant to CsWin32.

[User1785604260]

Now that I'm looking at that code some more, I'm also wondering why default(winmdroot.Foundation.HANDLE) is used when hMonitor is null, and why the call to DestroyPhysicalMonitor is still made. That would result in a 0 HANDLE, which is an actually valid handle you could get from PHYSICAL_MONITOR.

[AArnott]

Another excellent observation. I filed #755 to track.

[AArnott]

Great observation. I filed #754 to track this.

[AaronRobinsonMSFT]

Updating docs at dotnet/dotnet-api-docs#8626