Undefined behavior should not be invoked when a ComObject marshaled by UniqueComInterfaceMarshaller<T> is used after calling FinalRelease

[Tan90909090]

Description

In an API review about ComObject.FinalRelease, there is a comment that the second call should be a no-op and InvalidOperationException should be thrown the COM object is used after calling FinalRelease. In practice, however, undefined behaviors are invoked at the second call of FinalRelease for a ComObject marshaled by UniqueComInterfaceMarshaller<T>, at the use of the ComObject after the FinalRelease call, and at the invocation of GC for the ComObject after FinalRelease. For example, in the above situation, AccessViolationException may be thrown or the process may terminate abnormally.

Reproduction Steps

1. Write a following .csproj file:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <LangVersion>preview</LangVersion>
    <ImplicitUsings>disable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
  </PropertyGroup>

</Project>

2. Write a following code:

using System;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices.Marshalling;
using System.Runtime.Versioning;

[SupportedOSPlatform("windows")]

internal partial class C
{
    static void Main()
    {
        Bad1_UseAfterRelease();
        Bad2_DoubleReleaseExplicitly();
        Bad3_DoubleReleaseImplicitlyWhenFinalize();
        Ok_ButVeryUrgy();
    }

    static void Bad1_UseAfterRelease()
    {
        int hr = CoCreateInstance(
            CLSID_GraphBuilder,
            IntPtr.Zero,
            CLSCTX_INPROC_SERVER,
            typeof(IFilterGraph).GUID,
            out object? obj);
        Marshal.ThrowExceptionForHR(hr);

        ((ComObject)obj).FinalRelease();
        ((IFilterGraph)obj).EnumFilters(out _); // In my environment, the process will crash with code -1073740791, which means STATUS_STACK_BUFFER_OVERRUN
    }

    static void Bad2_DoubleReleaseExplicitly()
    {
        int hr = CoCreateInstance(
            CLSID_GraphBuilder,
            IntPtr.Zero,
            CLSCTX_INPROC_SERVER,
            typeof(IFilterGraph).GUID,
            out object? obj);
        Marshal.ThrowExceptionForHR(hr);

        ((ComObject)obj).FinalRelease();
        ((ComObject)obj).FinalRelease(); // In my environment, the process will crash with code -1073740940, which means "A heap has been corrupted"
    }

    static void Bad3_DoubleReleaseImplicitlyWhenFinalize()
    {
        int hr = CoCreateInstance(
            CLSID_GraphBuilder,
            IntPtr.Zero,
            CLSCTX_INPROC_SERVER,
            typeof(IFilterGraph).GUID,
            out object? obj);
        Marshal.ThrowExceptionForHR(hr);

        ((ComObject)obj).FinalRelease();
        obj = null;

        GC.Collect();
        GC.WaitForPendingFinalizers(); // In my environment, the process will crash with code -1073740940, which means "A heap has been corrupted"
    }

    static void Ok_ButVeryUrgy()
    {
        int hr = CoCreateInstance(
            CLSID_GraphBuilder,
            IntPtr.Zero,
            CLSCTX_INPROC_SERVER,
            typeof(IFilterGraph).GUID,
            out object? obj);
        Marshal.ThrowExceptionForHR(hr);

        ((ComObject)obj).FinalRelease();
        GC.SuppressFinalize(obj); // Very important suppression even if "CA1816: Call GC.SuppressFinalize correctly" occur
        obj = null;

        GC.Collect();
        GC.WaitForPendingFinalizers(); // no-op
    }

    [LibraryImport("ole32.dll")]
    internal static partial int CoCreateInstance(
        in Guid rclsid,
        IntPtr pUnkOuter, // Actual type is IUnknown*, but change it for simplify
        uint dwClsContext,
        in Guid riid,
        [MarshalUsing(typeof(UniqueComInterfaceMarshaller<object>))] out object ppv);

    internal const uint CLSCTX_INPROC_SERVER = 0x1;
    internal static readonly Guid CLSID_GraphBuilder = new("e436ebb3-524f-11ce-9f53-0020af0ba770");
}

[GeneratedComInterface]
[Guid("56a8689f-0ad4-11ce-b03a-0020af0ba770")]
[InterfaceType(ComInterfaceType.InterfaceIsIUnknown)]
internal partial interface IFilterGraph
{
    void AddFilter(
        [MarshalAs(UnmanagedType.Interface)] object pFilter,
        [MarshalAs(UnmanagedType.LPWStr)] string pName);

    void RemoveFilter([MarshalAs(UnmanagedType.Interface)] object pFilter);

    void EnumFilters([MarshalAs(UnmanagedType.Interface)] out object ppEnum);

    // Remaining members are omitted
}

3. Build and run

Expected behavior

Undefined behaviors are not invoked. The process never crash. (Methods may throw a documented exception)

Actual behavior

Undefined behaviors are invoked. For example, in the above code, AccessViolationException may be thrown or the process may terminate abnormally.

Regression?

ComObject type was introduced at .NET 8 so this is NOT a regression.

Known Workarounds

We have two choises:

1. Whenever we call FinalRelease, we also call GC.SupressFinalize for a ComObject marshaled by UniqueComInterfaceMarshaller
2. Do not use FinalRelease at all, and make ComObject release at GC time.

Configuration

• Microsoft Visual Studio Community 2022 (64-bit) - Current Version 17.8.3
• Windows 10 Pro 64-bit19045.3930
• .NET 8.0.0
• Build setting: Debug and x86 build

Other information

If I use DllImportAttribute and ComImportAttribute attributes then undefined behaviors are not invoked:

using System;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices.Marshalling;
using System.Runtime.Versioning;

[SupportedOSPlatform("windows")]

internal partial class C
{
    static void Main()
    {
        Ok1();
        Ok2();
        Ok3();
    }

    static void Ok1()
    {
        int hr = CoCreateInstance(
            CLSID_GraphBuilder,
            IntPtr.Zero,
            CLSCTX_INPROC_SERVER,
            typeof(IFilterGraph).GUID,
            out object? obj);
        Marshal.ThrowExceptionForHR(hr);

        Marshal.FinalReleaseComObject(obj);
        ((IFilterGraph)obj).EnumFilters(out _); // throw InvalidComObjectException instead of invoke an undefined behavior
    }

    static void Ok2()
    {
        int hr = CoCreateInstance(
            CLSID_GraphBuilder,
            IntPtr.Zero,
            CLSCTX_INPROC_SERVER,
            typeof(IFilterGraph).GUID,
            out object? obj);
        Marshal.ThrowExceptionForHR(hr);

        Marshal.FinalReleaseComObject(obj);
        Marshal.FinalReleaseComObject(obj); // no-op
    }

    static void Ok3()
    {
        int hr = CoCreateInstance(
            CLSID_GraphBuilder,
            IntPtr.Zero,
            CLSCTX_INPROC_SERVER,
            typeof(IFilterGraph).GUID,
            out object? obj);
        Marshal.ThrowExceptionForHR(hr);

        Marshal.FinalReleaseComObject(obj);
        obj = null;

        GC.Collect();
        GC.WaitForPendingFinalizers(); // no-op
    }

    [DllImport("ole32.dll")]
    internal static extern int CoCreateInstance(
        in Guid rclsid,
        IntPtr pUnkOuter, // Actual type is IUnknown*, but change it for simplify
        uint dwClsContext,
        in Guid riid,
        [MarshalAs(UnmanagedType.Interface)] out object ppv);

    internal const uint CLSCTX_INPROC_SERVER = 0x1;
    internal static readonly Guid CLSID_GraphBuilder = new("e436ebb3-524f-11ce-9f53-0020af0ba770");
}

[ComImport]
[Guid("56a8689f-0ad4-11ce-b03a-0020af0ba770")]
[InterfaceType(ComInterfaceType.InterfaceIsIUnknown)]
internal partial interface IFilterGraph
{
    void AddFilter(
        [MarshalAs(UnmanagedType.Interface)] object pFilter,
        [MarshalAs(UnmanagedType.LPWStr)] string pName);

    void RemoveFilter([MarshalAs(UnmanagedType.Interface)] object pFilter);

    void EnumFilters([MarshalAs(UnmanagedType.Interface)] out object ppEnum);

    // Remaining members are omitted
}

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

In an API review about ComObject.FinalRelease, there is a comment that the second call should be a no-op and InvalidOperationException should be thrown the COM object is used after calling FinalRelease. In practice, however, undefined behaviors are invoked at the second call of FinalRelease for a ComObject marshaled by UniqueComInterfaceMarshaller<T>, at the use of the ComObject after the FinalRelease call, and at the invocation of GC for the ComObject after FinalRelease. For example, in the above situation, AccessViolationException may be thrown or the process may terminate abnormally.

Reproduction Steps

1. Write a following .csproj file:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <LangVersion>preview</LangVersion>
    <ImplicitUsings>disable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
  </PropertyGroup>

</Project>

2. Write a following code:

using System;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices.Marshalling;
using System.Runtime.Versioning;

[SupportedOSPlatform("windows")]

internal partial class C
{
    static void Main()
    {
        Bad1_UseAfterRelease();
        Bad2_DoubleReleaseExplicitly();
        Bad3_DoubleReleaseImplicitlyWhenFinalize();
        Ok_ButVeryUrgy();
    }

    static void Bad1_UseAfterRelease()
    {
        int hr = CoCreateInstance(
            CLSID_GraphBuilder,
            IntPtr.Zero,
            CLSCTX_INPROC_SERVER,
            typeof(IFilterGraph).GUID,
            out object? obj);
        Marshal.ThrowExceptionForHR(hr);

        ((ComObject)obj).FinalRelease();
        ((IFilterGraph)obj).EnumFilters(out _); // In my environment, the process will crash with code -1073740791, which means STATUS_STACK_BUFFER_OVERRUN
    }

    static void Bad2_DoubleReleaseExplicitly()
    {
        int hr = CoCreateInstance(
            CLSID_GraphBuilder,
            IntPtr.Zero,
            CLSCTX_INPROC_SERVER,
            typeof(IFilterGraph).GUID,
            out object? obj);
        Marshal.ThrowExceptionForHR(hr);

        ((ComObject)obj).FinalRelease();
        ((ComObject)obj).FinalRelease(); // In my environment, the process will crash with code -1073740940, which means "A heap has been corrupted"
    }

    static void Bad3_DoubleReleaseImplicitlyWhenFinalize()
    {
        int hr = CoCreateInstance(
            CLSID_GraphBuilder,
            IntPtr.Zero,
            CLSCTX_INPROC_SERVER,
            typeof(IFilterGraph).GUID,
            out object? obj);
        Marshal.ThrowExceptionForHR(hr);

        ((ComObject)obj).FinalRelease();
        obj = null;

        GC.Collect();
        GC.WaitForPendingFinalizers(); // In my environment, the process will crash with code -1073740940, which means "A heap has been corrupted"
    }

    static void Ok_ButVeryUrgy()
    {
        int hr = CoCreateInstance(
            CLSID_GraphBuilder,
            IntPtr.Zero,
            CLSCTX_INPROC_SERVER,
            typeof(IFilterGraph).GUID,
            out object? obj);
        Marshal.ThrowExceptionForHR(hr);

        ((ComObject)obj).FinalRelease();
        GC.SuppressFinalize(obj); // Very important suppression even if "CA1816: Call GC.SuppressFinalize correctly" occur
        obj = null;

        GC.Collect();
        GC.WaitForPendingFinalizers(); // no-op
    }

    [LibraryImport("ole32.dll")]
    internal static partial int CoCreateInstance(
        in Guid rclsid,
        IntPtr pUnkOuter, // Actual type is IUnknown*, but change it for simplify
        uint dwClsContext,
        in Guid riid,
        [MarshalUsing(typeof(UniqueComInterfaceMarshaller<object>))] out object ppv);

    internal const uint CLSCTX_INPROC_SERVER = 0x1;
    internal static readonly Guid CLSID_GraphBuilder = new("e436ebb3-524f-11ce-9f53-0020af0ba770");
}

[GeneratedComInterface]
[Guid("56a8689f-0ad4-11ce-b03a-0020af0ba770")]
[InterfaceType(ComInterfaceType.InterfaceIsIUnknown)]
internal partial interface IFilterGraph
{
    void AddFilter(
        [MarshalAs(UnmanagedType.Interface)] object pFilter,
        [MarshalAs(UnmanagedType.LPWStr)] string pName);

    void RemoveFilter([MarshalAs(UnmanagedType.Interface)] object pFilter);

    void EnumFilters([MarshalAs(UnmanagedType.Interface)] out object ppEnum);

    // Remaining members are omitted
}

3. Build and run

Expected behavior

Undefined behaviors are not invoked. The process never crash. (Methods may throw a documented exception)

Actual behavior

Undefined behaviors are invoked. For example, in the above code, AccessViolationException may be thrown or the process may terminate abnormally.

Regression?

ComObject type was introduced at .NET 8 so this is NOT a regression.

Known Workarounds

We have two choises:

1. Whenever we call FinalRelease, we also call GC.SupressFinalize for a ComObject marshaled by UniqueComInterfaceMarshaller
2. Do not use FinalRelease at all, and make ComObject release at GC time.

Configuration

• Microsoft Visual Studio Community 2022 (64-bit) - Current Version 17.8.3
• Windows 10 Pro 64-bit19045.3930
• .NET 8.0.0
• Build setting: Debug and x86 build

Other information

If I use DllImportAttribute and ComImportAttribute attributes then undefined behaviors are not invoked:

using System;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices.Marshalling;
using System.Runtime.Versioning;

[SupportedOSPlatform("windows")]

internal partial class C
{
    static void Main()
    {
        Ok1();
        Ok2();
        Ok3();
    }

    static void Ok1()
    {
        int hr = CoCreateInstance(
            CLSID_GraphBuilder,
            IntPtr.Zero,
            CLSCTX_INPROC_SERVER,
            typeof(IFilterGraph).GUID,
            out object? obj);
        Marshal.ThrowExceptionForHR(hr);

        Marshal.FinalReleaseComObject(obj);
        ((IFilterGraph)obj).EnumFilters(out _); // throw InvalidComObjectException instead of invoke an undefined behavior
    }

    static void Ok2()
    {
        int hr = CoCreateInstance(
            CLSID_GraphBuilder,
            IntPtr.Zero,
            CLSCTX_INPROC_SERVER,
            typeof(IFilterGraph).GUID,
            out object? obj);
        Marshal.ThrowExceptionForHR(hr);

        Marshal.FinalReleaseComObject(obj);
        Marshal.FinalReleaseComObject(obj); // no-op
    }

    static void Ok3()
    {
        int hr = CoCreateInstance(
            CLSID_GraphBuilder,
            IntPtr.Zero,
            CLSCTX_INPROC_SERVER,
            typeof(IFilterGraph).GUID,
            out object? obj);
        Marshal.ThrowExceptionForHR(hr);

        Marshal.FinalReleaseComObject(obj);
        obj = null;

        GC.Collect();
        GC.WaitForPendingFinalizers(); // no-op
    }

    [DllImport("ole32.dll")]
    internal static extern int CoCreateInstance(
        in Guid rclsid,
        IntPtr pUnkOuter, // Actual type is IUnknown*, but change it for simplify
        uint dwClsContext,
        in Guid riid,
        [MarshalAs(UnmanagedType.Interface)] out object ppv);

    internal const uint CLSCTX_INPROC_SERVER = 0x1;
    internal static readonly Guid CLSID_GraphBuilder = new("e436ebb3-524f-11ce-9f53-0020af0ba770");
}

[ComImport]
[Guid("56a8689f-0ad4-11ce-b03a-0020af0ba770")]
[InterfaceType(ComInterfaceType.InterfaceIsIUnknown)]
internal partial interface IFilterGraph
{
    void AddFilter(
        [MarshalAs(UnmanagedType.Interface)] object pFilter,
        [MarshalAs(UnmanagedType.LPWStr)] string pName);

    void RemoveFilter([MarshalAs(UnmanagedType.Interface)] object pFilter);

    void EnumFilters([MarshalAs(UnmanagedType.Interface)] out object ppEnum);

    // Remaining members are omitted
}

Author:	Tan90909090
Assignees:	-
Labels:	area-System.Runtime.InteropServices, untriaged, needs-area-label
Milestone:	-