Switch CoreCLR WeakReference to unified managed implementation (#77196)

* Delete managed CoreClr WR * all but com works * COM tests are passing * HandleTagBits const for NativeAot * Exclusive Set * fix * Use sign bit * Platforms not supporting COM can mask only one bit. * new approach * fix mono build * check for FEATURE_COMWRAPPERS too * stub NativeAOT support (NYI). * current * moved handle tags on the managed side to one location * Getter optimizations * Optimizations for Setter * accessibility of some members * ensure identity of the rehydrated RCW * make ComWeakRefToObject a QCall * delete unused pWeakReferenceOfTCanonMT and pWeakReferenceMT * byte-aligned * cleanup unreachable code * renamed WeakReferenceObject::m_Handle -> WeakReferenceObject::m_taggedHandle * Apply suggestions from code review Co-authored-by: Aaron Robinson <[email protected]> * some PR feedback * GetWeakHandle no longer cares about inlining. * turn ObjectToComWeakRef into a QCall * revert changes under coreclr\gc * added a note to eventually remove HNDTYPE_WEAK_NATIVE_COM * Update src/coreclr/gc/gcinterface.h Co-authored-by: Maoni Stephens <[email protected]> Co-authored-by: Aaron Robinson <[email protected]> Co-authored-by: Maoni Stephens <[email protected]>
dotnet · Nov 1, 2022 · 7d2de49 · 7d2de49
1 parent e0c9353
commit 7d2de49
Show file tree

Hide file tree

Showing 38 changed files with 540 additions and 1,257 deletions.
diff --git a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj
@@ -133,6 +133,7 @@
     <Compile Include="$(BclSourcesRoot)\System\Collections\Generic\Comparer.CoreCLR.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Collections\Generic\ComparerHelpers.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Collections\Generic\EqualityComparer.CoreCLR.cs" />
+    <Compile Include="$(BclSourcesRoot)\System\ComAwareWeakReference.CoreCLR.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Currency.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Delegate.CoreCLR.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Diagnostics\Debugger.cs" />
@@ -244,8 +245,6 @@
     <Compile Include="$(BclSourcesRoot)\System\TypeLoadException.CoreCLR.cs" />
     <Compile Include="$(BclSourcesRoot)\System\TypeNameParser.cs" />
     <Compile Include="$(BclSourcesRoot)\System\ValueType.cs" />
-    <Compile Include="$(BclSourcesRoot)\System\WeakReference.CoreCLR.cs" />
-    <Compile Include="$(BclSourcesRoot)\System\WeakReference.T.CoreCLR.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(FeatureComWrappers)' == 'true'">
     <Compile Include="$(BclSourcesRoot)\System\Runtime\InteropServices\ComWrappers.cs" />

diff --git a/src/coreclr/System.Private.CoreLib/src/System/ComAwareWeakReference.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/ComAwareWeakReference.CoreCLR.cs
@@ -0,0 +1,57 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+#if FEATURE_COMINTEROP || FEATURE_COMWRAPPERS
+namespace System
+{
+    internal sealed partial class ComAwareWeakReference
+    {
+        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ComWeakRefToObject")]
+        private static partial void ComWeakRefToObject(IntPtr pComWeakRef, long wrapperId, ObjectHandleOnStack retRcw);
+
+        internal static object? ComWeakRefToObject(IntPtr pComWeakRef, long wrapperId)
+        {
+            object? retRcw = null;
+            ComWeakRefToObject(pComWeakRef, wrapperId, ObjectHandleOnStack.Create(ref retRcw));
+            return retRcw;
+        }
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        internal static unsafe bool PossiblyComObject(object target)
+        {
+            // see: syncblk.h
+            const int IS_HASHCODE_BIT_NUMBER = 26;
+            const int BIT_SBLK_IS_HASHCODE = 1 << IS_HASHCODE_BIT_NUMBER;
+            const int BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX = 0x08000000;
+
+            fixed (byte* pRawData = &target.GetRawData())
+            {
+                // The header is 4 bytes before MT field on all architectures
+                int header = *(int*)(pRawData - sizeof(IntPtr) - sizeof(int));
+                // common case: target does not have a syncblock, so there is no interop info
+                return (header & (BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX | BIT_SBLK_IS_HASHCODE)) == BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX;
+            }
+        }
+
+        [MethodImpl(MethodImplOptions.InternalCall)]
+        internal static extern bool HasInteropInfo(object target);
+
+        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ObjectToComWeakRef")]
+        private static partial IntPtr ObjectToComWeakRef(ObjectHandleOnStack retRcw, out long wrapperId);
+
+        internal static nint ObjectToComWeakRef(object target, out long wrapperId)
+        {
+            if (HasInteropInfo(target))
+            {
+                return ObjectToComWeakRef(ObjectHandleOnStack.Create(ref target), out wrapperId);
+            }
+
+            wrapperId = 0;
+            return IntPtr.Zero;
+        }
+    }
+}
+#endif
diff --git a/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs
@@ -305,7 +305,7 @@ public static void KeepAlive(object? obj)
         //
         public static int GetGeneration(WeakReference wo)
         {
-            int result = GetGenerationWR(wo.m_handle);
+            int result = GetGenerationWR(wo.WeakHandle);
             KeepAlive(wo);
             return result;
         }

diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.CoreCLR.cs
@@ -8,7 +8,7 @@ namespace System.Runtime.InteropServices
     public partial struct GCHandle
     {
         [MethodImpl(MethodImplOptions.InternalCall)]
-        private static extern IntPtr InternalAlloc(object? value, GCHandleType type);
+        internal static extern IntPtr InternalAlloc(object? value, GCHandleType type);
 
         [MethodImpl(MethodImplOptions.InternalCall)]
         internal static extern void InternalFree(IntPtr handle);

diff --git a/src/coreclr/System.Private.CoreLib/src/System/WeakReference.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/WeakReference.CoreCLR.cs
diff --git a/src/coreclr/System.Private.CoreLib/src/System/WeakReference.T.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/WeakReference.T.CoreCLR.cs
diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp
@@ -7858,10 +7858,6 @@ void CALLBACK DacHandleWalker::EnumCallbackSOS(PTR_UNCHECKED_OBJECTREF handle, u
     data.Type = param->Type;
     if (param->Type == HNDTYPE_DEPENDENT)
         data.Secondary = GetDependentHandleSecondary(handle.GetAddr()).GetAddr();
-#ifdef FEATURE_COMINTEROP
-    else if (param->Type == HNDTYPE_WEAK_NATIVE_COM)
-        data.Secondary = HndGetHandleExtraInfo(handle.GetAddr());
-#endif // FEATURE_COMINTEROP
     else
         data.Secondary = 0;
     data.AppDomain = param->AppDomain;

diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp
@@ -7601,10 +7601,6 @@ UINT32 DacRefWalker::GetHandleWalkerMask()
     if ((mHandleMask & CorHandleWeakRefCount) || (mHandleMask & CorHandleStrongRefCount))
         result |= (1 << HNDTYPE_REFCOUNTED);
 #endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS || FEATURE_OBJCMARSHAL
-#if defined(FEATURE_COMINTEROP) || defined(FEATURE_COMWRAPPERS)
-    if (mHandleMask & CorHandleWeakNativeCom)
-        result |= (1 << HNDTYPE_WEAK_NATIVE_COM);
-#endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS
 
     if (mHandleMask & CorHandleStrongDependent)
         result |= (1 << HNDTYPE_DEPENDENT);
@@ -7778,11 +7774,6 @@ void CALLBACK DacHandleWalker::EnumCallbackDac(PTR_UNCHECKED_OBJECTREF handle, u
             data.i64ExtraData = refCnt;
             break;
 #endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS || FEATURE_OBJCMARSHAL
-#if defined(FEATURE_COMINTEROP) || defined(FEATURE_COMWRAPPERS)
-        case HNDTYPE_WEAK_NATIVE_COM:
-            data.dwType = (DWORD)CorHandleWeakNativeCom;
-            break;
-#endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS
 
         case HNDTYPE_DEPENDENT:
             data.dwType = (DWORD)CorHandleStrongDependent;

diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp
@@ -3216,9 +3216,6 @@ HRESULT ClrDataAccess::GetHandleEnum(ISOSHandleEnum **ppHandleEnum)
 #if defined(FEATURE_COMINTEROP) || defined(FEATURE_COMWRAPPERS) || defined(FEATURE_OBJCMARSHAL)
                             HNDTYPE_REFCOUNTED,
 #endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS || FEATURE_OBJCMARSHAL
-#if defined(FEATURE_COMINTEROP) || defined(FEATURE_COMWRAPPERS)
-                            HNDTYPE_WEAK_NATIVE_COM
-#endif // FEATURE_COMINTEROP
                             };
 
     return GetHandleEnumForTypes(types, ARRAY_SIZE(types), ppHandleEnum);
@@ -3257,9 +3254,6 @@ HRESULT ClrDataAccess::GetHandleEnumForGC(unsigned int gen, ISOSHandleEnum **ppH
 #if defined(FEATURE_COMINTEROP) || defined(FEATURE_COMWRAPPERS) || defined(FEATURE_OBJCMARSHAL)
                             HNDTYPE_REFCOUNTED,
 #endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS || FEATURE_OBJCMARSHAL
-#if defined(FEATURE_COMINTEROP) || defined(FEATURE_COMWRAPPERS)
-                            HNDTYPE_WEAK_NATIVE_COM
-#endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS
                             };
 
     DacHandleWalker *walker = new DacHandleWalker();

diff --git a/src/coreclr/gc/gcinterface.h b/src/coreclr/gc/gcinterface.h
@@ -465,6 +465,10 @@ typedef enum
      * code holding onto a native weak reference can always access an RCW to the
      * underlying COM object as long as it has not been released by all of its strong
      * references.
+     *
+     * NOTE: HNDTYPE_WEAK_NATIVE_COM is no longer used in the VM starting .NET 8
+     *       but we are keeping it here for backward compatibility purposes"
+     * 
      */
     HNDTYPE_WEAK_NATIVE_COM   = 9
 } HandleType;

diff --git a/src/coreclr/inc/dacvars.h b/src/coreclr/inc/dacvars.h
@@ -162,6 +162,9 @@ DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_pThreadClass, ::g_pThreadClass)
 DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_pPredefinedArrayTypes, ::g_pPredefinedArrayTypes)
 DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_TypedReferenceMT, ::g_TypedReferenceMT)
 
+DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_pWeakReferenceClass, ::g_pWeakReferenceClass)
+DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_pWeakReferenceOfTClass, ::g_pWeakReferenceOfTClass)
+
 #ifdef FEATURE_COMINTEROP
 DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_pBaseCOMObject, ::g_pBaseCOMObject)
 #endif

diff --git a/src/coreclr/nativeaot/Runtime/ObjectLayout.h b/src/coreclr/nativeaot/Runtime/ObjectLayout.h
@@ -136,5 +136,5 @@ static uintptr_t const MAX_STRING_LENGTH = 0x3FFFFFDF;
 class WeakReference : public Object
 {
 public:
-    uintptr_t    m_HandleAndKind;
+    uintptr_t    m_taggedHandle;
 };
diff --git a/src/coreclr/nativeaot/Runtime/gcrhenv.cpp b/src/coreclr/nativeaot/Runtime/gcrhenv.cpp
@@ -1181,10 +1181,15 @@ bool GCToEEInterface::EagerFinalized(Object* obj)
     // Managed code should not be running.
     ASSERT(GCHeapUtilities::GetGCHeap()->IsGCInProgressHelper());
 
+    // the lowermost 1 bit is reserved for storing additional info about the handle
+    const uintptr_t HandleTagBits = 1;
+
     WeakReference* weakRefObj = (WeakReference*)obj;
-    OBJECTHANDLE handle = (OBJECTHANDLE)(weakRefObj->m_HandleAndKind & ~(uintptr_t)1);
-    HandleType handleType = (weakRefObj->m_HandleAndKind & 1) ? HandleType::HNDTYPE_WEAK_LONG : HandleType::HNDTYPE_WEAK_SHORT;
-    weakRefObj->m_HandleAndKind &= (uintptr_t)1;
+    OBJECTHANDLE handle = (OBJECTHANDLE)(weakRefObj->m_taggedHandle & ~HandleTagBits);
+    _ASSERTE((weakRefObj->m_taggedHandle & 2) == 0);
+    HandleType handleType = (weakRefObj->m_taggedHandle & 1) ? HandleType::HNDTYPE_WEAK_LONG : HandleType::HNDTYPE_WEAK_SHORT;
+    // keep the bit that indicates whether this reference was tracking resurrection, clear the rest.
+    weakRefObj->m_taggedHandle &= HandleTagBits;
     GCHandleUtilities::GetGCHandleManager()->DestroyHandleOfType(handle, handleType);
     return true;
 }

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj b/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj
@@ -169,6 +169,7 @@
     <Compile Include="System\Collections\Generic\Comparer.NativeAot.cs" />
     <Compile Include="System\Collections\Generic\EqualityComparer.NativeAot.cs" />
     <Compile Include="System\Collections\Generic\EqualOnlyComparer.cs" />
+    <Compile Include="System\ComAwareWeakReference.NativeAot.cs" />
     <Compile Include="System\InvokeUtils.cs" />
     <Compile Include="System\IO\FileLoadException.NativeAot.cs" />
     <Compile Include="System\RuntimeMethodHandle.cs" />

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/ComAwareWeakReference.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/ComAwareWeakReference.NativeAot.cs
@@ -0,0 +1,31 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+
+#if FEATURE_COMINTEROP || FEATURE_COMWRAPPERS
+namespace System
+{
+    internal sealed partial class ComAwareWeakReference
+    {
+        internal static object? ComWeakRefToObject(IntPtr pComWeakRef, long wrapperId)
+        {
+            // NativeAOT support for COM WeakReference is NYI
+            throw new NotImplementedException();
+        }
+
+        internal static bool PossiblyComObject(object target)
+        {
+            // NativeAOT support for COM WeakReference is NYI
+            return false;
+        }
+
+        internal static IntPtr ObjectToComWeakRef(object target, out long wrapperId)
+        {
+            // NativeAOT support for COM WeakReference is NYI
+            wrapperId = 0;
+            return 0;
+        }
+    }
+}
+#endif
diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/GC.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/GC.NativeAot.cs
@@ -86,7 +86,9 @@ public static int GetGeneration(WeakReference wo)
         {
             // note - this throws an NRE if given a null weak reference. This isn't
             // documented, but it's the behavior of Desktop and CoreCLR.
-            object? obj = RuntimeImports.RhHandleGet(wo.Handle);
+            object? obj = RuntimeImports.RhHandleGet(wo.WeakHandle);
+            KeepAlive(wo);
+
             if (obj == null)
             {
                 throw new ArgumentNullException(nameof(wo));

diff --git a/src/coreclr/vm/appdomain.cpp b/src/coreclr/vm/appdomain.cpp
@@ -1384,6 +1384,9 @@ void SystemDomain::LoadBaseSystemClasses()
 
     g_pThreadClass = CoreLibBinder::GetClass(CLASS__THREAD);
 
+    g_pWeakReferenceClass = CoreLibBinder::GetClass(CLASS__WEAKREFERENCE);
+    g_pWeakReferenceOfTClass = CoreLibBinder::GetClass(CLASS__WEAKREFERENCEGENERIC);
+
 #ifdef FEATURE_COMINTEROP
     if (g_pConfig->IsBuiltInCOMSupported())
     {

diff --git a/src/coreclr/vm/appdomain.hpp b/src/coreclr/vm/appdomain.hpp
@@ -1064,12 +1064,6 @@ class BaseDomain
         WRAPPER_NO_CONTRACT;
         return ::CreateRefcountedHandle(m_handleStore, object);
     }
-
-    OBJECTHANDLE CreateNativeComWeakHandle(OBJECTREF object, NativeComWeakHandleInfo* pComWeakHandleInfo)
-    {
-        WRAPPER_NO_CONTRACT;
-        return ::CreateNativeComWeakHandle(m_handleStore, object, pComWeakHandleInfo);
-    }
 #endif // FEATURE_COMINTEROP || FEATURE_COMWRAPPERS
 
     OBJECTHANDLE CreateVariableHandle(OBJECTREF object, UINT type)

diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h
@@ -939,11 +939,12 @@ DEFINE_METHOD(GC,                   KEEP_ALIVE,             KeepAlive,
 DEFINE_METHOD(GC,                   COLLECT,                Collect,                    SM_RetVoid)
 DEFINE_METHOD(GC,                   WAIT_FOR_PENDING_FINALIZERS, WaitForPendingFinalizers, SM_RetVoid)
 
-DEFINE_CLASS_U(System,                 WeakReference,          WeakReferenceObject)
-DEFINE_FIELD_U(m_handle,               WeakReferenceObject,    m_Handle)
+DEFINE_CLASS_U(System,              WeakReference,          WeakReferenceObject)
+DEFINE_FIELD_U(_taggedHandle,       WeakReferenceObject,    m_taggedHandle)
 DEFINE_CLASS(WEAKREFERENCE,         System,                 WeakReference)
+DEFINE_CLASS(WEAKREFERENCEGENERIC,  System,                 WeakReference`1)
 
-DEFINE_CLASS_U(Threading,              WaitHandle,             WaitHandleBase)
+DEFINE_CLASS_U(Threading,           WaitHandle,             WaitHandleBase)
 DEFINE_FIELD_U(_waitHandle,         WaitHandleBase,         m_safeHandle)
 
 DEFINE_CLASS(DEBUGGER,              Diagnostics,            Debugger)