Fix ComObject finalizer crash from OLE DB error RCWs

src/libraries/System.Data.OleDb/src/OleDbError.cs

@@ -17,56 +17,74 @@ internal OleDbError(UnsafeNativeMethods.IErrorRecords errorRecords, int index)
         {
             OleDbHResult hr;
             int lcid = System.Globalization.CultureInfo.CurrentCulture.LCID;
-            UnsafeNativeMethods.IErrorInfo errorInfo = errorRecords.GetErrorInfo(index, lcid);
-            if (null != errorInfo)
-            {
-                hr = errorInfo.GetDescription(out this.message);
-
-                if (OleDbHResult.DB_E_NOLOCALE == hr)
-                {
-                    UnsafeNativeMethods.ReleaseComWrappersObject(errorInfo);
-                    lcid = Interop.Kernel32.GetUserDefaultLCID();
-                    errorInfo = errorRecords.GetErrorInfo(index, lcid);
 
-                    if (null != errorInfo)
-                    {
-                        hr = errorInfo.GetDescription(out this.message);
-                    }
-                }
-                if ((hr < 0) && ADP.IsEmpty(this.message))
-                {
-                    this.message = ODB.FailedGetDescription(hr);
-                }
+            // The RCWs for the per-record IErrorInfo and ISQLErrorInfo objects
+            // are thread affinitized, so we need to make sure we release them
+            // on the same thread that we created them on.
+            UnsafeNativeMethods.IErrorInfo? errorInfo = errorRecords.GetErrorInfo(index, lcid);
+            try
+            {
                 if (null != errorInfo)
                 {
-                    hr = errorInfo.GetSource(out this.source);
+                    hr = errorInfo.GetDescription(out this.message);
 
                     if (OleDbHResult.DB_E_NOLOCALE == hr)
                     {
                         UnsafeNativeMethods.ReleaseComWrappersObject(errorInfo);
+                        errorInfo = null;
                         lcid = Interop.Kernel32.GetUserDefaultLCID();
                         errorInfo = errorRecords.GetErrorInfo(index, lcid);
 
                         if (null != errorInfo)
                         {
-                            hr = errorInfo.GetSource(out this.source);
+                            hr = errorInfo.GetDescription(out this.message);
                         }
                     }
-                    if ((hr < 0) && ADP.IsEmpty(this.source))
+                    if ((hr < 0) && ADP.IsEmpty(this.message))
                     {
-                        this.source = ODB.FailedGetSource(hr);
+                        this.message = ODB.FailedGetDescription(hr);
+                    }
+                    if (null != errorInfo)
+                    {
+                        hr = errorInfo.GetSource(out this.source);
+
+                        if (OleDbHResult.DB_E_NOLOCALE == hr)
+                        {
+                            UnsafeNativeMethods.ReleaseComWrappersObject(errorInfo);
+                            errorInfo = null;
+                            lcid = Interop.Kernel32.GetUserDefaultLCID();
+                            errorInfo = errorRecords.GetErrorInfo(index, lcid);
+
+                            if (null != errorInfo)
+                            {
+                                hr = errorInfo.GetSource(out this.source);
+                            }
+                        }
+                        if ((hr < 0) && ADP.IsEmpty(this.source))
+                        {
+                            this.source = ODB.FailedGetSource(hr);
+                        }
                     }
-                    UnsafeNativeMethods.ReleaseComWrappersObject(errorInfo);
                 }
             }
+            finally
+            {
+                UnsafeNativeMethods.ReleaseComWrappersObject(errorInfo);
+            }
 
             UnsafeNativeMethods.ISQLErrorInfo sqlErrorInfo;
             errorRecords.GetCustomErrorObject(index, in ODB.IID_ISQLErrorInfo, out sqlErrorInfo);
 
             if (null != sqlErrorInfo)
             {
-                this.nativeError = sqlErrorInfo.GetSQLInfo(out this.sqlState);
-                UnsafeNativeMethods.ReleaseComWrappersObject(sqlErrorInfo);
+                try
+                {
+                    this.nativeError = sqlErrorInfo.GetSQLInfo(out this.sqlState);
+                }
+                finally
+                {
+                    UnsafeNativeMethods.ReleaseComWrappersObject(sqlErrorInfo);
+                }
             }
         }
 

src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/FreeThreadedStrategy.cs

@@ -1,6 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
+
 // Implementations of the COM strategy interfaces defined in Com.cs that we would want to ship (can be internal only if we don't want to allow users to provide their own implementations in v1).
 using System.Runtime.CompilerServices;
 
@@ -12,6 +14,7 @@ internal sealed unsafe class FreeThreadedStrategy : IIUnknownStrategy
 
         void* IIUnknownStrategy.CreateInstancePointer(void* unknown)
         {
+            AssertFreeThreaded(unknown);
             Marshal.AddRef((nint)unknown);
             return unknown;
         }
@@ -31,6 +34,78 @@ unsafe int IIUnknownStrategy.QueryInterface(void* thisPtr, in Guid handle, out v
         }
 
         unsafe int IIUnknownStrategy.Release(void* thisPtr)
-            => Marshal.Release((nint)thisPtr);
+        {
+            // Avoid checking if the instance is free-threaded here,
+            // since this method can be called from the GC finalizer thread
+            // and we need to QI, which may not be safe if the object is not free-threaded.
+            return Marshal.Release((nint)thisPtr);
+        }
+
+        // This strategy assumes every COM object it is given is free threaded (agile), so its
+        // IUnknown methods can be called from any thread; including Release on the GC finalizer
+        // thread.
+        [Conditional("DEBUG")]
+        private static void AssertFreeThreaded(void* thisPtr)
+        {
+#if DEBUG
+            if (OperatingSystem.IsWindows())
+            {
+                Debug.Assert(
+                    IsFreeThreaded(thisPtr),
+                    "A COM object used through FreeThreadedStrategy is not free threaded (agile).");
+            }
+#endif
+        }
+
+#if DEBUG
+        // Mirrors the built-in RCW's IUnkEntry::IsComponentFreeThreaded (src/coreclr/vm/comcache.cpp).
+        private static bool IsFreeThreaded(void* thisPtr)
+        {
+            // IID_IAgileObject {94EA2B94-E9CC-49E0-C0FF-EE64CA8F5B90}
+            Guid iidAgileObject = new(0x94ea2b94, 0xe9cc, 0x49e0, 0xc0, 0xff, 0xee, 0x64, 0xca, 0x8f, 0x5b, 0x90);
+            if (Marshal.QueryInterface((nint)thisPtr, iidAgileObject, out nint agile) >= 0)
+            {
+                Marshal.Release(agile);
+                return true;
+            }
+
+            // IID_IMarshal {00000003-0000-0000-C000-000000000046}
+            Guid iidMarshal = new(0x00000003, 0x0000, 0x0000, 0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x46);
+            if (Marshal.QueryInterface((nint)thisPtr, iidMarshal, out nint marshalUnk) >= 0)
+            {
+                try
+                {
+                    void* pMarshal = (void*)marshalUnk;
+
+                    // IMarshal::GetUnmarshalClass is the first IMarshal method (that is, 4th zero-indexed slot).
+                    // HRESULT GetUnmarshalClass(REFIID riid, void* pv, DWORD dwDestContext,
+                    //                           void* pvDestContext, DWORD mshlflags, CLSID* pCid)
+                    var getUnmarshalClass =
+                        (delegate* unmanaged[MemberFunction]<void*, Guid*, void*, uint, void*, uint, Guid*, int>)((*(void***)pMarshal)[3]);
+
+                    // IID_IUnknown {00000000-0000-0000-C000-000000000046}
+                    Guid iidUnknown = new(0x00000000, 0x0000, 0x0000, 0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x46);
+                    const uint MSHCTX_INPROC = 3;
+                    const uint MSHLFLAGS_NORMAL = 0;
+
+                    Guid unmarshalClass;
+                    int hr = getUnmarshalClass(pMarshal, &iidUnknown, null, MSHCTX_INPROC, null, MSHLFLAGS_NORMAL, &unmarshalClass);
+
+                    // CLSID_InProcFreeMarshaler {0000033A-0000-0000-C000-000000000046}
+                    Guid clsidFreeMarshaler = new(0x0000033a, 0x0000, 0x0000, 0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x46);
+                    if (hr >= 0 && unmarshalClass == clsidFreeMarshaler)
+                    {
+                        return true;
+                    }
+                }
+                finally
+                {
+                    Marshal.Release(marshalUnk);
+                }
+            }
+
+            return false;
+        }
+#endif
     }
 }