From 116fe2519b620eeb3dcf4ec0035ef7243ba58540 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 9 Dec 2024 11:43:40 -0800 Subject: [PATCH 1/3] Do not wait for finalizers on tracker host callbacks --- src/coreclr/interop/trackerobjectmanager.cpp | 5 ++++- .../System/Runtime/InteropServices/ComWrappers.NativeAot.cs | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/coreclr/interop/trackerobjectmanager.cpp b/src/coreclr/interop/trackerobjectmanager.cpp index d4302054baedba..6740410064039f 100644 --- a/src/coreclr/interop/trackerobjectmanager.cpp +++ b/src/coreclr/interop/trackerobjectmanager.cpp @@ -84,7 +84,10 @@ namespace STDMETHODIMP HostServices::ReleaseDisconnectedReferenceSources() { - return InteropLibImports::WaitForRuntimeFinalizerForExternal(); + // This could lead to deadlock if finalizer thread is trying to get back to this thread, because we are + // not pumping anymore. Disable this for now. See: https://github.com/dotnet/runtime/issues/109538. + // return InteropLibImports::WaitForRuntimeFinalizerForExternal(); + return S_OK; } STDMETHODIMP HostServices::NotifyEndOfReferenceTrackingOnThread() diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs index cc33e85d73917c..8dc77720057521 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs @@ -1420,7 +1420,9 @@ internal static unsafe int IReferenceTrackerHost_ReleaseDisconnectedReferenceSou { try { - GC.WaitForPendingFinalizers(); + // This could lead to deadlock if finalizer thread is trying to get back to this thread, because we are + // not pumping anymore. Disable this for now. See: https://github.com/dotnet/runtime/issues/109538. + // GC.WaitForPendingFinalizers(); return HResults.S_OK; } catch (Exception e) From 6efbb2849f22f3d32d1b809b5980e2e5935d9a17 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 9 Dec 2024 14:00:01 -0800 Subject: [PATCH 2/3] Apply suggestions from PR review --- src/coreclr/interop/trackerobjectmanager.cpp | 4 ++-- .../System/Runtime/InteropServices/ComWrappers.NativeAot.cs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/interop/trackerobjectmanager.cpp b/src/coreclr/interop/trackerobjectmanager.cpp index 6740410064039f..0df78164906dcc 100644 --- a/src/coreclr/interop/trackerobjectmanager.cpp +++ b/src/coreclr/interop/trackerobjectmanager.cpp @@ -84,9 +84,9 @@ namespace STDMETHODIMP HostServices::ReleaseDisconnectedReferenceSources() { - // This could lead to deadlock if finalizer thread is trying to get back to this thread, because we are + // We'd like to call InteropLibImports::WaitForRuntimeFinalizerForExternal() here, but this could + // lead to deadlock if the finalizer thread is trying to get back to this thread, because we are // not pumping anymore. Disable this for now. See: https://github.com/dotnet/runtime/issues/109538. - // return InteropLibImports::WaitForRuntimeFinalizerForExternal(); return S_OK; } diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs index 8dc77720057521..4b78c25799be1d 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs @@ -1420,9 +1420,9 @@ internal static unsafe int IReferenceTrackerHost_ReleaseDisconnectedReferenceSou { try { - // This could lead to deadlock if finalizer thread is trying to get back to this thread, because we are - // not pumping anymore. Disable this for now. See: https://github.com/dotnet/runtime/issues/109538. - // GC.WaitForPendingFinalizers(); + // We'd like to call GC.WaitForPendingFinalizers() here, but this could lead to deadlock + // if the finalizer thread is trying to get back to this thread, because we are not pumping + // anymore. Disable this for now. See: https://github.com/dotnet/runtime/issues/109538. return HResults.S_OK; } catch (Exception e) From 278524a85064361ea06942eca5787095e2ae38f9 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 9 Dec 2024 14:01:17 -0800 Subject: [PATCH 3/3] Remove unnecessary try/catch --- .../InteropServices/ComWrappers.NativeAot.cs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs index 4b78c25799be1d..b4d70b66c2a937 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs @@ -1418,17 +1418,10 @@ internal static unsafe int IReferenceTrackerHost_DisconnectUnusedReferenceSource [UnmanagedCallersOnly] internal static unsafe int IReferenceTrackerHost_ReleaseDisconnectedReferenceSources(IntPtr pThis) { - try - { - // We'd like to call GC.WaitForPendingFinalizers() here, but this could lead to deadlock - // if the finalizer thread is trying to get back to this thread, because we are not pumping - // anymore. Disable this for now. See: https://github.com/dotnet/runtime/issues/109538. - return HResults.S_OK; - } - catch (Exception e) - { - return Marshal.GetHRForException(e); - } + // We'd like to call GC.WaitForPendingFinalizers() here, but this could lead to deadlock + // if the finalizer thread is trying to get back to this thread, because we are not pumping + // anymore. Disable this for now. See: https://github.com/dotnet/runtime/issues/109538. + return HResults.S_OK; } [UnmanagedCallersOnly]