Fix implementation of IFindReferenceTargetsCallback::FoundTrackerTarget (dotnet#115921)

Author: jkoritzinsky

src/coreclr/debug/daccess/request.cpp

@@ -5076,7 +5076,7 @@ namespace
 TADDR ClrDataAccess::GetIdentityForManagedObjectWrapper(TADDR mow)
 {
     PTR_ManagedObjectWrapper pMOW = dac_cast<PTR_ManagedObjectWrapper>(mow);
-    // Replicate the logic for ManagedObjectWrapper.As(IID_IUnknown)
+    // Replicate the logic for _wrapper.As(IID_IUnknown)
     if ((pMOW->GetFlags() & InteropLib::Com::CreateComInterfaceFlagsEx::CallerDefinedIUnknown) == InteropLib::Com::CreateComInterfaceFlagsEx::None)
     {
         // We have the standard IUnknown implementation, so grab it from its known location.
@@ -5173,7 +5173,7 @@ HRESULT ClrDataAccess::GetObjectComWrappersData(CLRDATA_ADDRESS objAddr, CLRDATA
                 for (unsigned int i = 0; i < count; i++)
                 {
                     MOWHOLDERREF pMOWRef = (MOWHOLDERREF)pListItems->GetAt(i);
-                    PTR_ManagedObjectWrapper pMOW = PTR_ManagedObjectWrapper(dac_cast<TADDR>(pMOWRef->ManagedObjectWrapper));
+                    PTR_ManagedObjectWrapper pMOW = PTR_ManagedObjectWrapper(dac_cast<TADDR>(pMOWRef->_wrapper));
 
                     // Now that we have the managed object wrapper, we need to figure out the COM identity of it.
                     TADDR pComIdentity = GetIdentityForManagedObjectWrapper(dac_cast<TADDR>(pMOW));

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.NativeAot.cs

@@ -164,13 +164,13 @@ internal static unsafe void WalkExternalTrackerObjects()
                     nativeObjectWrapper.TrackerObject != IntPtr.Zero)
                 {
                     FindReferenceTargetsCallback.s_currentRootObjectHandle = nativeObjectWrapper.ProxyHandle;
-                    if (IReferenceTracker.FindTrackerTargets(nativeObjectWrapper.TrackerObject, (IntPtr)Unsafe.AsPointer(in s_findReferencesTargetCallback)) != HResults.S_OK)
+                    int hr = IReferenceTracker.FindTrackerTargets(nativeObjectWrapper.TrackerObject, (IntPtr)Unsafe.AsPointer(in s_findReferencesTargetCallback));
+                    FindReferenceTargetsCallback.s_currentRootObjectHandle = default;
+                    if (hr < 0)
                     {
                         walkFailed = true;
-                        FindReferenceTargetsCallback.s_currentRootObjectHandle = default;
                         break;
                     }
-                    FindReferenceTargetsCallback.s_currentRootObjectHandle = default;
                 }
             }
 
@@ -227,16 +227,23 @@ private static unsafe int IFindReferenceTargetsCallback_FoundTrackerTarget(IntPt
         {
             if (referenceTrackerTarget == IntPtr.Zero)
             {
-                return HResults.E_INVALIDARG;
+                return HResults.E_POINTER;
+            }
+
+            object sourceObject = s_currentRootObjectHandle.Target!;
+
+            if (!TryGetObject(referenceTrackerTarget, out object? targetObject))
+            {
+                return HResults.S_FALSE;
             }
 
-            if (TryGetObject(referenceTrackerTarget, out object? foundObject))
+            if (sourceObject == targetObject)
             {
-                // Notify the runtime a reference path was found.
-                return TrackerObjectManager.AddReferencePath(s_currentRootObjectHandle.Target, foundObject) ? HResults.S_OK : HResults.S_FALSE;
+                return HResults.S_FALSE;
             }
 
-            return HResults.S_OK;
+            // Notify the runtime a reference path was found.
+            return TrackerObjectManager.AddReferencePath(sourceObject, targetObject) ? HResults.S_OK : HResults.S_FALSE;
         }
 
         internal struct ReferenceTargetsVftbl

src/coreclr/vm/corelib.h

@@ -462,7 +462,8 @@ DEFINE_FIELD(COMWRAPPERS, NAITVE_OBJECT_WRAPPER_TABLE, s_nativeObjectWrapperTabl
 DEFINE_FIELD(COMWRAPPERS, ALL_MANAGED_OBJECT_WRAPPER_TABLE, s_allManagedObjectWrapperTable)
 
 DEFINE_CLASS_U(Interop, ComWrappers+ManagedObjectWrapperHolder, ManagedObjectWrapperHolderObject)
-DEFINE_FIELD_U(_wrapper, ManagedObjectWrapperHolderObject, ManagedObjectWrapper)
+DEFINE_FIELD_U(_wrappedObject, ManagedObjectWrapperHolderObject, _wrappedObject)
+DEFINE_FIELD_U(_wrapper, ManagedObjectWrapperHolderObject, _wrapper)
 DEFINE_CLASS_U(Interop, ComWrappers+NativeObjectWrapper, NativeObjectWrapperObject)
 DEFINE_FIELD_U(_comWrappers, NativeObjectWrapperObject, _comWrappers)
 DEFINE_FIELD_U(_externalComObject, NativeObjectWrapperObject, _externalComObject)

src/coreclr/vm/interoplibinterface_comwrappers.cpp

@@ -453,16 +453,23 @@ namespace InteropLibImports
         CONTRACTL_END;
 
         // Get the external object's managed wrapper
-        ::OBJECTHANDLE srcHandle = static_cast<::OBJECTHANDLE>(targetHandle);
+        ::OBJECTHANDLE srcHandle = static_cast<::OBJECTHANDLE>(sourceHandle);
         OBJECTREF source = ObjectFromHandle(srcHandle);
 
         // Get the target of the external object's reference.
         ::OBJECTHANDLE tgtHandle = static_cast<::OBJECTHANDLE>(targetHandle);
-        OBJECTREF target = ObjectFromHandle(tgtHandle );
+        MOWHOLDERREF holder = (MOWHOLDERREF)ObjectFromHandle(tgtHandle);
 
-        // Return if the target has been collected or these are the same object.
-        if (target == NULL
-            || source->PassiveGetSyncBlock() == target->PassiveGetSyncBlock())
+        // Return if the holder has been collected
+        if (holder == NULL)
+        {
+            return S_FALSE;
+        }
+
+        OBJECTREF target = holder->_wrappedObject;
+
+        // Return if these are the same object.
+        if (source == target)
         {
             return S_FALSE;
         }
@@ -496,7 +503,7 @@ bool ComWrappersNative::IsManagedObjectComWrapper(_In_ OBJECTREF managedObjectWr
 
     MOWHOLDERREF holder = (MOWHOLDERREF)managedObjectWrapperHolderRef;
 
-    *pIsRooted = InteropLib::Com::IsRooted(holder->ManagedObjectWrapper);
+    *pIsRooted = InteropLib::Com::IsRooted(holder->_wrapper);
 
     return true;
 }

src/coreclr/vm/interoplibinterface_comwrappers.h

@@ -73,9 +73,9 @@ class ManagedObjectWrapperHolderObject : public Object
     friend class ClrDataAccess;
 private:
     OBJECTREF _releaser;
-    OBJECTREF _wrappedObject;
 public:
-    DPTR(InteropLib::ABI::ManagedObjectWrapperLayout) ManagedObjectWrapper;
+    OBJECTREF _wrappedObject;
+    DPTR(InteropLib::ABI::ManagedObjectWrapperLayout) _wrapper;
 };
 
 class NativeObjectWrapperObject : public Object