Improve ConditionalWeakTable.Remove (dotnet/coreclr#8580)

Clear the key of the deleted entry to allow GC collect objects pointed to by it

Fixes dotnet/coreclr#8577

Commit migrated from https://github.com/dotnet/coreclr/commit/6ef31918fce2a9790e01f863e06575a9ca2ec77a

diff --git a/src/coreclr/src/mscorlib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs b/src/coreclr/src/mscorlib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs
index 5e6a3f4..9d9b61c 100644 (file)
--- a/src/coreclr/src/mscorlib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs
+++ b/src/coreclr/src/mscorlib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs
@@ -516,10 +516,16 @@ namespace System.Runtime.CompilerServices
                 int entryIndex = FindEntry(key, out value);
                 if (entryIndex != -1)
                 {
+                    ref Entry entry = ref _entries[entryIndex];
+
                     // We do not free the handle here, as we may be racing with readers who already saw the hash code.
                     // Instead, we simply overwrite the entry's hash code, so subsequent reads will ignore it.
                     // The handle will be free'd in Container's finalizer, after the table is resized or discarded.
-                    Volatile.Write(ref _entries[entryIndex].HashCode, -1);
+                    Volatile.Write(ref entry.HashCode, -1);
+
+                    // Also, clear the key to allow GC to collect objects pointed to by the entry
+                    entry.depHnd.SetPrimary(null);
+
                     return true;
                 }
 
@@ -840,6 +846,11 @@ namespace System.Runtime.CompilerServices
             nGetPrimaryAndSecondary(_handle, out primary, out secondary);
         }
 
+        public void SetPrimary(object primary)
+        {
+            nSetPrimary(_handle, primary);
+        }
+
         public void SetSecondary(object secondary)
         {
             nSetSecondary(_handle, secondary);
@@ -871,6 +882,9 @@ namespace System.Runtime.CompilerServices
         private static extern void nGetPrimaryAndSecondary(IntPtr dependentHandle, out object primary, out object secondary);
 
         [MethodImpl(MethodImplOptions.InternalCall)]
+        private static extern void nSetPrimary(IntPtr dependentHandle, object primary);
+
+        [MethodImpl(MethodImplOptions.InternalCall)]
         private static extern void nSetSecondary(IntPtr dependentHandle, object secondary);
 
         [MethodImpl(MethodImplOptions.InternalCall)]

diff --git a/src/coreclr/src/vm/comdependenthandle.cpp b/src/coreclr/src/vm/comdependenthandle.cpp
index 7a46f69..6535a80 100644 (file)
--- a/src/coreclr/src/vm/comdependenthandle.cpp
+++ b/src/coreclr/src/vm/comdependenthandle.cpp
@@ -74,11 +74,22 @@ FCIMPL3(VOID, DependentHandle::nGetPrimaryAndSecondary, OBJECTHANDLE handle, Obj
 }
 FCIMPLEND
 
+FCIMPL2(VOID, DependentHandle::nSetPrimary, OBJECTHANDLE handle, Object *_primary)
+{
+    FCALL_CONTRACT;
+
+    _ASSERTE(handle != NULL);
+
+    OBJECTREF primary(_primary);
+    StoreObjectInHandle(handle, primary);
+}
+FCIMPLEND
+
 FCIMPL2(VOID, DependentHandle::nSetSecondary, OBJECTHANDLE handle, Object *_secondary)
 {
     FCALL_CONTRACT;
 
-    _ASSERTE(handle != NULL && _secondary != NULL);
+    _ASSERTE(handle != NULL);
 
     OBJECTREF secondary(_secondary);
     SetDependentHandleSecondary(handle, secondary);

diff --git a/src/coreclr/src/vm/comdependenthandle.h b/src/coreclr/src/vm/comdependenthandle.h
index 29110c1..7192a4b 100644 (file)
--- a/src/coreclr/src/vm/comdependenthandle.h
+++ b/src/coreclr/src/vm/comdependenthandle.h
@@ -45,6 +45,7 @@ public:
     static FCDECL2(VOID,   nGetPrimary, OBJECTHANDLE handle, Object **outPrimary);
     static FCDECL3(VOID,   nGetPrimaryAndSecondary, OBJECTHANDLE handle, Object **outPrimary, Object **outSecondary);
     static FCDECL1(VOID,   nFree, OBJECTHANDLE handle);
+    static FCDECL2(VOID,   nSetPrimary, OBJECTHANDLE handle, Object *primary);
     static FCDECL2(VOID,   nSetSecondary, OBJECTHANDLE handle, Object *secondary);
 };
 

diff --git a/src/coreclr/src/vm/ecalllist.h b/src/coreclr/src/vm/ecalllist.h
index 03b675f..b110d0e 100644 (file)
--- a/src/coreclr/src/vm/ecalllist.h
+++ b/src/coreclr/src/vm/ecalllist.h
@@ -108,6 +108,7 @@ FCFuncStart(gDependentHandleFuncs)
     FCFuncElement("nGetPrimary",             DependentHandle::nGetPrimary)
     FCFuncElement("nGetPrimaryAndSecondary", DependentHandle::nGetPrimaryAndSecondary)
     FCFuncElement("nFree",                   DependentHandle::nFree)
+    FCFuncElement("nSetPrimary",             DependentHandle::nSetPrimary)
     FCFuncElement("nSetSecondary",           DependentHandle::nSetSecondary)
 FCFuncEnd()