From 89b9305b5b453447ac563e90f6bc5a3167dca4ea Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 16 Jul 2026 11:21:02 -0700 Subject: [PATCH 1/4] Release OLE DB error RCWs on the calling thread OleDbError's constructor created per-record IErrorInfo and ISQLErrorInfo RCWs but released them only on the success path. A provider returning a failure HRESULT from a non-[PreserveSig] call (GetErrorInfo, GetSQLInfo) threw mid-extraction, letting the RCW escape to ~ComObject() and be released on the GC finalizer thread. For a thread-affinitized OLE DB error object that off-thread Marshal.Release faults (0xC0000005). Wrap both RCWs in try/finally so they are always FinalRelease'd on the calling thread regardless of exceptions. Fixes #125221 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../System.Data.OleDb/src/OleDbError.cs | 70 ++++++++++++------- 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/src/libraries/System.Data.OleDb/src/OleDbError.cs b/src/libraries/System.Data.OleDb/src/OleDbError.cs index 2e81f631773d41..8168947cc87551 100644 --- a/src/libraries/System.Data.OleDb/src/OleDbError.cs +++ b/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); + } } } From e44ff83eb1d6dab4744671e0c5610e800790e414 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 16 Jul 2026 11:48:33 -0700 Subject: [PATCH 2/4] Assert COM objects are free threaded in FreeThreadedStrategy (DEBUG) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Marshalling/FreeThreadedStrategy.cs | 76 ++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/FreeThreadedStrategy.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/FreeThreadedStrategy.cs index 2127bf44998696..479a5b25383466 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/FreeThreadedStrategy.cs +++ b/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,12 +14,14 @@ internal sealed unsafe class FreeThreadedStrategy : IIUnknownStrategy void* IIUnknownStrategy.CreateInstancePointer(void* unknown) { + AssertFreeThreaded(unknown); Marshal.AddRef((nint)unknown); return unknown; } unsafe int IIUnknownStrategy.QueryInterface(void* thisPtr, in Guid handle, out void* ppObj) { + AssertFreeThreaded(thisPtr); int hr = Marshal.QueryInterface((nint)thisPtr, handle, out nint ppv); if (hr < 0) { @@ -31,6 +35,76 @@ unsafe int IIUnknownStrategy.QueryInterface(void* thisPtr, in Guid handle, out v } unsafe int IIUnknownStrategy.Release(void* thisPtr) - => Marshal.Release((nint)thisPtr); + { + AssertFreeThreaded(thisPtr); + 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)((*(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 } } From 16d765dc5e2bea9db5628bb661c2d0762080f9b6 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 16 Jul 2026 13:12:32 -0700 Subject: [PATCH 3/4] Remove free-threaded assertion from QueryInterface and Release methods to ensure safety when called from the GC finalizer thread --- .../InteropServices/Marshalling/FreeThreadedStrategy.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/FreeThreadedStrategy.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/FreeThreadedStrategy.cs index 479a5b25383466..cf9e44332881f3 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/FreeThreadedStrategy.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/FreeThreadedStrategy.cs @@ -21,7 +21,6 @@ internal sealed unsafe class FreeThreadedStrategy : IIUnknownStrategy unsafe int IIUnknownStrategy.QueryInterface(void* thisPtr, in Guid handle, out void* ppObj) { - AssertFreeThreaded(thisPtr); int hr = Marshal.QueryInterface((nint)thisPtr, handle, out nint ppv); if (hr < 0) { @@ -36,7 +35,9 @@ unsafe int IIUnknownStrategy.QueryInterface(void* thisPtr, in Guid handle, out v unsafe int IIUnknownStrategy.Release(void* thisPtr) { - AssertFreeThreaded(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); } From b46bc14ee129a879a6b2907e86f7a3ab76157f46 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 16 Jul 2026 13:25:15 -0700 Subject: [PATCH 4/4] Update delegate type for GetUnmarshalClass in FreeThreadedStrategy to use MemberFunction attribute --- .../Runtime/InteropServices/Marshalling/FreeThreadedStrategy.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/FreeThreadedStrategy.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/FreeThreadedStrategy.cs index cf9e44332881f3..5ad47205aeb5e7 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/FreeThreadedStrategy.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/FreeThreadedStrategy.cs @@ -81,7 +81,7 @@ private static bool IsFreeThreaded(void* thisPtr) // HRESULT GetUnmarshalClass(REFIID riid, void* pv, DWORD dwDestContext, // void* pvDestContext, DWORD mshlflags, CLSID* pCid) var getUnmarshalClass = - (delegate* unmanaged)((*(void***)pMarshal)[3]); + (delegate* unmanaged[MemberFunction])((*(void***)pMarshal)[3]); // IID_IUnknown {00000000-0000-0000-C000-000000000046} Guid iidUnknown = new(0x00000000, 0x0000, 0x0000, 0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x46);