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
1.99k stars 84 forks source link

Calling `IMMDevice.Activate()` results in a NullReferenceException #1081

Closed pingzing closed 6 months ago

pingzing commented 8 months ago

Actual behavior

Calling the IMMDevice.Activate() overload with the convenience extension (e.g. the one that takes a Guid rather than a Guid*) method results in a NullReferenceExcception.

It seems the issue is in the generated convenience extension method that looks like this:

internal static unsafe void Activate(this winmdroot.Media.Audio.IMMDevice @this, in global::System.Guid iid, winmdroot.System.Com.CLSCTX dwClsCtx, winmdroot.System.Com.StructuredStorage.PROPVARIANT ? pActivationParams, out object ppInterface)
{
    fixed (global::System.Guid* iidLocal = &iid)
    {
        winmdroot.System.Com.StructuredStorage.PROPVARIANT pActivationParamsLocal = pActivationParams ?? default(winmdroot.System.Com.StructuredStorage.PROPVARIANT );
        @this.Activate(iidLocal, dwClsCtx, pActivationParams.HasValue ? pActivationParamsLocal : Unsafe.NullRef<winmdroot.System.Com.StructuredStorage.PROPVARIANT >(), out ppInterface);
    }
}

The actual culprit is the Unsafe.NullRef<PROPVARIANT>() call--it seems that passing a non-nullable struct type to it results in a NullReferenceException.

Expected behavior

Calling the .Activate() convenience overload works!

Repro steps

  1. NativeMethods.txt content:

    CoCreateInstance
    MMDevice
    MMDeviceEnumerator
    IMMDeviceEnumerator
    IAudioMeterInformation
  2. NativeMethods.json content (if present): N/A

  3. Any of your own code that should be shared?

Here's a little minimal sample, cut down a bit from what I have:

IAudioMeterInformation _audioMeterInfo;
HRESULT devEnumResult = PInvoke.CoCreateInstance(
    typeof(MMDeviceEnumerator).GUID,
    null,
    CLSCTX.CLSCTX_INPROC_SERVER,
    typeof(IMMDeviceEnumerator).GUID,
    out object rawDeviceEnumerator);

if (devEnumResult.Failed)
{
    Debug.WriteLine($"Failed to create a device enumerator: HRESULT {devEnumResult}");
    return;
}

IMMDeviceEnumerator? deviceEnumerator = (IMMDeviceEnumerator)rawDeviceEnumerator;
deviceEnumerator.GetDefaultAudioEndpoint(EDataFlow.eRender, ERole.eConsole, out IMMDevice mmDevice);

// this following block *does* work.
unsafe
{
    Guid infoGuid = typeof(IAudioMeterInformation).GUID;
    Guid* iidLocal = &infoGuid;
    var nullPropvariant = new PROPVARIANT();
    mmDevice.Activate(iidLocal, CLSCTX.CLSCTX_ALL, nullPropvariant, out object rawMeterInfo);
    _audioMeterInfo = (IAudioMeterInformation)rawMeterInfo;
}

//mmDevice.Activate(typeof(IAudioMeterInformation).GUID, CLSCTX.CLSCTX_ALL, null, out object rawMeterInfo); // <-- this crashes!

Context

Note: this is a WPF project.

riverar commented 6 months ago

I hit this as well. The nullable pActivationParams isn't being handled correctly here, ends up trying to access pActivationParams.HasValue.

Poking @AArnott for prioritization 😅

AArnott commented 6 months ago

I don't think there's anything wrong with calling pActivationParams.HasValue in the friendly overload, as that is a nullable struct, so it can't throw NRE.

I suspect the issue is that the .NET interop layer cannot handle a null reference passed into an in struct parameter on a COM interface. That seems unfortunate because the COM side should only need the address, and taking the address of a null reference is allowed in C# and produces a null pointer as intended.

But on the flip side, I don't know why CsWin32 is generating the COM interface with an in PROPVARIANT rather than PROPVARIANT* in the first place. Given it's an [Optional] parameter, CsWin32 should be (I think) be preferring pointers. That's the first thing I'll look into.

AArnott commented 6 months ago

This bug was surprisingly difficult to solve.

riverar commented 6 months ago

Yay, thanks @AArnott. Will try it out tonight.