Add GC.KeepAlive to COM paths in WeakReference (#88537)

Omission was noticed by @AustinWise. I reproed the failure and this fix for it on linux-x64. Fixes #81362
dotnet · Jul 12, 2023 · a3c21b9 · a3c21b9
1 parent 82bf906
commit a3c21b9
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 6 deletions.
diff --git a/src/libraries/System.Private.CoreLib/src/System/WeakReference.T.cs b/src/libraries/System.Private.CoreLib/src/System/WeakReference.T.cs
@@ -110,6 +110,10 @@ public void SetTarget(T target)
             if ((th & ComAwareBit) != 0 || comInfo != null)
             {
                 ComAwareWeakReference.SetTarget(ref _taggedHandle, target, comInfo);
+
+                // must keep the instance alive as long as we use the handle.
+                GC.KeepAlive(this);
+
                 return;
             }
 #endif
@@ -133,13 +137,22 @@ private T? Target
                 if (th == 0)
                     return default;
 
+                T? target;
+
 #if FEATURE_COMINTEROP || FEATURE_COMWRAPPERS
                 if ((th & ComAwareBit) != 0)
-                    return Unsafe.As<T?>(ComAwareWeakReference.GetTarget(th));
+                {
+                    target = Unsafe.As<T?>(ComAwareWeakReference.GetTarget(th));
+
+                    // must keep the instance alive as long as we use the handle.
+                    GC.KeepAlive(this);
+
+                    return target;
+                }
 #endif
 
                 // unsafe cast is ok as the handle cannot be destroyed and recycled while we keep the instance alive
-                T? target = Unsafe.As<T?>(GCHandle.InternalGet(th));
+                target = Unsafe.As<T?>(GCHandle.InternalGet(th));
 
                 // must keep the instance alive as long as we use the handle.
                 GC.KeepAlive(this);

diff --git a/src/libraries/System.Private.CoreLib/src/System/WeakReference.cs b/src/libraries/System.Private.CoreLib/src/System/WeakReference.cs
@@ -157,13 +157,22 @@ public virtual object? Target
                 if (th == 0)
                     return default;
 
+                object? target;
+
 #if FEATURE_COMINTEROP || FEATURE_COMWRAPPERS
                 if ((th & ComAwareBit) != 0)
-                    return ComAwareWeakReference.GetTarget(th);
+                {
+                    target = ComAwareWeakReference.GetTarget(th);
+
+                    // must keep the instance alive as long as we use the handle.
+                    GC.KeepAlive(this);
+
+                    return target;
+                }
 #endif
 
                 // unsafe cast is ok as the handle cannot be destroyed and recycled while we keep the instance alive
-                object? target = GCHandle.InternalGet(th);
+                target = GCHandle.InternalGet(th);
 
                 // must keep the instance alive as long as we use the handle.
                 GC.KeepAlive(this);
@@ -186,6 +195,10 @@ public virtual object? Target
                 if ((th & ComAwareBit) != 0 || comInfo != null)
                 {
                     ComAwareWeakReference.SetTarget(ref _taggedHandle, value, comInfo);
+
+                    // must keep the instance alive as long as we use the handle.
+                    GC.KeepAlive(this);
+
                     return;
                 }
 #endif

diff --git a/src/tests/Interop/COM/ComWrappers/WeakReference/WeakReferenceTest.csproj b/src/tests/Interop/COM/ComWrappers/WeakReference/WeakReferenceTest.csproj
@@ -4,8 +4,6 @@
     <!-- Registers global instances of ComWrappers -->
     <UnloadabilityIncompatible>true</UnloadabilityIncompatible>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
-    <!-- Temporarily disabled due to https://github.com/dotnet/runtime/issues/81362 -->
-    <GCStressIncompatible>true</GCStressIncompatible>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="WeakReferenceTest.cs" />