Add GC.KeepAlive to COM paths in WeakReference (#88537)
authorMark Plesko <markples@microsoft.com>
Wed, 12 Jul 2023 16:32:23 +0000 (09:32 -0700)
committerGitHub <noreply@github.com>
Wed, 12 Jul 2023 16:32:23 +0000 (09:32 -0700)
Omission was noticed by @AustinWise. I reproed the failure and this fix for it on linux-x64.

Fixes #81362

src/libraries/System.Private.CoreLib/src/System/WeakReference.T.cs
src/libraries/System.Private.CoreLib/src/System/WeakReference.cs
src/tests/Interop/COM/ComWrappers/WeakReference/WeakReferenceTest.csproj

index 8ddca59..315f6b9 100644 (file)
@@ -110,6 +110,10 @@ namespace System
             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 @@ namespace System
                 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);
index 932dcb0..bff571a 100644 (file)
@@ -157,13 +157,22 @@ namespace System
                 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 @@ namespace System
                 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
index a710055..6839002 100644 (file)
@@ -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" />