Unwrap RCWs that are passed to Marshal.GetIUnknownForObject when using the global marshalling ComWrappers instance (#115436)

jkoritzinsky · web-flow · commit ff7b6c57ebb8 · 2025-06-16T23:36:41.000Z
diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.CoreCLR.cs
@@ -67,11 +67,14 @@ internal static int CallICustomQueryInterface(ManagedObjectWrapperHolder holder,
 
         internal static IntPtr GetOrCreateComInterfaceForObjectWithGlobalMarshallingInstance(object obj)
         {
+            if (s_globalInstanceForMarshalling == null)
+            {
+                return IntPtr.Zero;
+            }
+
             try
             {
-                return s_globalInstanceForMarshalling is null
-                    ? IntPtr.Zero
-                    : s_globalInstanceForMarshalling.GetOrCreateComInterfaceForObject(obj, CreateComInterfaceFlags.TrackerSupport);
+                return ComInterfaceForObject(obj);
             }
             catch (ArgumentException)
             {
@@ -83,9 +86,14 @@ internal static IntPtr GetOrCreateComInterfaceForObjectWithGlobalMarshallingInst
 
         internal static object? GetOrCreateObjectForComInstanceWithGlobalMarshallingInstance(IntPtr comObject, CreateObjectFlags flags)
         {
+            if (s_globalInstanceForMarshalling == null)
+            {
+                return null;
+            }
+
             try
             {
-                return s_globalInstanceForMarshalling?.GetOrCreateObjectForComInstance(comObject, flags);
+                return ComObjectForInterface(comObject, flags);
             }
             catch (ArgumentNullException)
             {
diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/InteropHelpers.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/InteropHelpers.cs
@@ -499,7 +499,7 @@ public static object ConvertNativeComInterfaceToManaged(IntPtr pUnk)
 
 #if TARGET_WINDOWS
 #pragma warning disable CA1416
-            return ComWrappers.ComObjectForInterface(pUnk);
+            return ComWrappers.ComObjectForInterface(pUnk, CreateObjectFlags.TrackerObject | CreateObjectFlags.Unwrap);
 #pragma warning restore CA1416
 #else
             throw new PlatformNotSupportedException(SR.PlatformNotSupported_ComInterop);
diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Com.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Com.cs
@@ -295,7 +295,7 @@ public static object GetTypedObjectForIUnknown(IntPtr pUnk, Type t)
         [SupportedOSPlatform("windows")]
         public static object GetObjectForIUnknown(IntPtr pUnk)
         {
-            return ComWrappers.ComObjectForInterface(pUnk);
+            return ComWrappers.ComObjectForInterface(pUnk, CreateObjectFlags.TrackerObject | CreateObjectFlags.Unwrap);
         }
 
         [SupportedOSPlatform("windows")]
diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs
@@ -743,13 +743,17 @@ public void DisconnectTracker()
 
         internal static object? GetOrCreateObjectFromWrapper(ComWrappers wrapper, IntPtr externalComObject)
         {
-            if (s_globalInstanceForTrackerSupport != null && s_globalInstanceForTrackerSupport == wrapper)
+            if (wrapper is null)
+            {
+                return null;
+            }
+            if (s_globalInstanceForTrackerSupport == wrapper)
             {
                 return s_globalInstanceForTrackerSupport.GetOrCreateObjectForComInstance(externalComObject, CreateObjectFlags.TrackerObject);
             }
-            else if (s_globalInstanceForMarshalling != null && s_globalInstanceForMarshalling == wrapper)
+            else if (s_globalInstanceForMarshalling == wrapper)
             {
-                return ComObjectForInterface(externalComObject);
+                return ComObjectForInterface(externalComObject, CreateObjectFlags.TrackerObject | CreateObjectFlags.Unwrap);
             }
             else
             {
@@ -1470,7 +1474,12 @@ internal static IntPtr ComInterfaceForObject(object instance)
                 throw new NotSupportedException(SR.InvalidOperation_ComInteropRequireComWrapperInstance);
             }
 
-            return s_globalInstanceForMarshalling.GetOrCreateComInterfaceForObject(instance, CreateComInterfaceFlags.None);
+            if (TryGetComInstance(instance, out IntPtr comObject))
+            {
+                return comObject;
+            }
+
+            return s_globalInstanceForMarshalling.GetOrCreateComInterfaceForObject(instance, CreateComInterfaceFlags.TrackerSupport);
         }
 
         internal static unsafe IntPtr ComInterfaceForObject(object instance, Guid targetIID)
@@ -1489,15 +1498,14 @@ internal static unsafe IntPtr ComInterfaceForObject(object instance, Guid target
             return comObjectInterface;
         }
 
-        internal static object ComObjectForInterface(IntPtr externalComObject)
+        internal static object ComObjectForInterface(IntPtr externalComObject, CreateObjectFlags flags)
         {
             if (s_globalInstanceForMarshalling == null)
             {
                 throw new NotSupportedException(SR.InvalidOperation_ComInteropRequireComWrapperInstance);
             }
 
-            // TrackerObject support and unwrapping matches the built-in semantics that the global marshalling scenario mimics.
-            return s_globalInstanceForMarshalling.GetOrCreateObjectForComInstance(externalComObject, CreateObjectFlags.TrackerObject | CreateObjectFlags.Unwrap);
+            return s_globalInstanceForMarshalling.GetOrCreateObjectForComInstance(externalComObject, flags);
         }
 
         internal static IntPtr GetOrCreateTrackerTarget(IntPtr externalComObject)
diff --git a/src/tests/Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs b/src/tests/Interop/COM/ComWrappers/GlobalInstance/GlobalInstance.cs
@@ -49,6 +49,7 @@ extern public static int UpdateTestObjectAsInterface(
 
         private const string ManagedServerTypeName = "ConsumeNETServerTesting";
 
+        private const string IID_IUNKNOWN = "00000000-0000-0000-C000-000000000046";
         private const string IID_IDISPATCH = "00020400-0000-0000-C000-000000000046";
         private const string IID_IINSPECTABLE = "AF86E2E0-B12D-4c6a-9C5A-D7AA65101E90";
         class TestEx : Test
@@ -278,7 +279,7 @@ private static void ValidateMarshalAPIs(bool validateUseRegistered)
             var testObj = new Test();
             IntPtr comWrapper1 = Marshal.GetIUnknownForObject(testObj);
             Assert.NotEqual(IntPtr.Zero, comWrapper1);
-            Assert.Equal(testObj, registeredWrapper.LastComputeVtablesObject);
+            Assert.Same(testObj, registeredWrapper.LastComputeVtablesObject);
 
             IntPtr comWrapper2 = Marshal.GetIUnknownForObject(testObj);
             Assert.Equal(comWrapper1, comWrapper2);
@@ -295,7 +296,7 @@ private static void ValidateMarshalAPIs(bool validateUseRegistered)
                 var dispatchObj = new TestEx(IID_IDISPATCH);
                 IntPtr dispatchWrapper = Marshal.GetIDispatchForObject(dispatchObj);
                 Assert.NotEqual(IntPtr.Zero, dispatchWrapper);
-                Assert.Equal(dispatchObj, registeredWrapper.LastComputeVtablesObject);
+                Assert.Same(dispatchObj, registeredWrapper.LastComputeVtablesObject);
 
                 Console.WriteLine($" -- Validate Marshal.GetIDispatchForObject != Marshal.GetIUnknownForObject...");
                 IntPtr unknownWrapper = Marshal.GetIUnknownForObject(dispatchObj);
@@ -309,7 +310,7 @@ private static void ValidateMarshalAPIs(bool validateUseRegistered)
             object objWrapper1 = Marshal.GetObjectForIUnknown(trackerObjRaw);
             Assert.Equal(validateUseRegistered, objWrapper1 is FakeWrapper);
             object objWrapper2 = Marshal.GetObjectForIUnknown(trackerObjRaw);
-            Assert.Equal(objWrapper1, objWrapper2);
+            Assert.Same(objWrapper1, objWrapper2);
 
             Console.WriteLine($" -- Validate Marshal.GetUniqueObjectForIUnknown...");
 
@@ -319,6 +320,29 @@ private static void ValidateMarshalAPIs(bool validateUseRegistered)
             Assert.NotEqual(objWrapper1, objWrapper3);
 
             Marshal.Release(trackerObjRaw);
+
+            if (validateUseRegistered)
+            {
+                Console.WriteLine($" -- Validate Marshal.GetObjectForIUnknown and Marshal.GetIUnknownForObject unwrapping...");
+                // Validate that the object returned by Marshal.GetObjectForIUnknown is the same as the original object passed to
+                // Marshal.GetIUnknownForObject.
+                IntPtr comWrapper3 = Marshal.GetIUnknownForObject(testObj);
+                object unwrappedObj = Marshal.GetObjectForIUnknown(comWrapper3);
+                Assert.Same(testObj, unwrappedObj);
+
+                // Validate that the pointer returned by Marshal.GetIUnknownForObject is the same one that was passed into
+                // Marshal.GetObjectForIUnknown.
+                IntPtr trackerObj2 = MockReferenceTrackerRuntime.CreateTrackerObject();
+                Marshal.ThrowExceptionForHR(Marshal.QueryInterface(trackerObj2, Guid.Parse(IID_IUNKNOWN), out IntPtr trackerObj2Identity));
+                Marshal.Release(trackerObj2);
+
+                object trackerObjectWrapper = Marshal.GetObjectForIUnknown(trackerObj2);
+                IntPtr trackerObjUnwrapped = Marshal.GetIUnknownForObject(trackerObjectWrapper);
+                Assert.Equal(trackerObj2Identity, trackerObjUnwrapped);
+
+                Marshal.Release(trackerObj2Identity);
+                Marshal.Release(trackerObjUnwrapped);
+            }
         }
 
         private static void ValidatePInvokes(bool validateUseRegistered)
@@ -362,12 +386,12 @@ private static void ValidateInterfaceMarshaler<T>(UpdateTestObject<T> func, bool
 
             T retObj;
             int hr = func(testObj as T, value, out retObj);
-            Assert.Equal(testObj, GlobalComWrappers.Instance.LastComputeVtablesObject);
+            Assert.Same(testObj, GlobalComWrappers.Instance.LastComputeVtablesObject);
             if (shouldSucceed)
             {
                 Assert.True(retObj is Test);
                 Assert.Equal(value, testObj.GetValue());
-                Assert.Equal<object>(testObj, retObj);
+                Assert.Same(testObj, retObj);
             }
             else
             {

Original file line number	Diff line number	Diff line change
`@@ -67,11 +67,14 @@ internal static int CallICustomQueryInterface(ManagedObjectWrapperHolder holder,`
`67`	`67`
`68`	`68`	`internal static IntPtr GetOrCreateComInterfaceForObjectWithGlobalMarshallingInstance(object obj)`
`69`	`69`	`{`
	`70`	`+ if (s_globalInstanceForMarshalling == null)`
	`71`	`+ {`
	`72`	`+ return IntPtr.Zero;`
	`73`	`+ }`
	`74`	`+`
`70`	`75`	`try`
`71`	`76`	`{`
`72`		`- return s_globalInstanceForMarshalling is null`
`73`		`- ? IntPtr.Zero`
`74`		`- : s_globalInstanceForMarshalling.GetOrCreateComInterfaceForObject(obj, CreateComInterfaceFlags.TrackerSupport);`
	`77`	`+ return ComInterfaceForObject(obj);`
`75`	`78`	`}`
`76`	`79`	`catch (ArgumentException)`
`77`	`80`	`{`
`@@ -83,9 +86,14 @@ internal static IntPtr GetOrCreateComInterfaceForObjectWithGlobalMarshallingInst`
`83`	`86`
`84`	`87`	`internal static object? GetOrCreateObjectForComInstanceWithGlobalMarshallingInstance(IntPtr comObject, CreateObjectFlags flags)`
`85`	`88`	`{`
	`89`	`+ if (s_globalInstanceForMarshalling == null)`
	`90`	`+ {`
	`91`	`+ return null;`
	`92`	`+ }`
	`93`	`+`
`86`	`94`	`try`
`87`	`95`	`{`
`88`		`- return s_globalInstanceForMarshalling?.GetOrCreateObjectForComInstance(comObject, flags);`
	`96`	`+ return ComObjectForInterface(comObject, flags);`
`89`	`97`	`}`
`90`	`98`	`catch (ArgumentNullException)`
`91`	`99`	`{`
Original file line number	Diff line number	Diff line change
`@@ -295,7 +295,7 @@ public static object GetTypedObjectForIUnknown(IntPtr pUnk, Type t)`
`295`	`295`	`[SupportedOSPlatform("windows")]`
`296`	`296`	`public static object GetObjectForIUnknown(IntPtr pUnk)`
`297`	`297`	`{`
`298`		`- return ComWrappers.ComObjectForInterface(pUnk);`
	`298`	`+ return ComWrappers.ComObjectForInterface(pUnk, CreateObjectFlags.TrackerObject \| CreateObjectFlags.Unwrap);`
`299`	`299`	`}`
`300`	`300`
`301`	`301`	`[SupportedOSPlatform("windows")]`
Original file line number	Diff line number	Diff line change
`@@ -743,13 +743,17 @@ public void DisconnectTracker()`
`743`	`743`
`744`	`744`	`internal static object? GetOrCreateObjectFromWrapper(ComWrappers wrapper, IntPtr externalComObject)`
`745`	`745`	`{`
`746`		`- if (s_globalInstanceForTrackerSupport != null && s_globalInstanceForTrackerSupport == wrapper)`
	`746`	`+ if (wrapper is null)`
	`747`	`+ {`
	`748`	`+ return null;`
	`749`	`+ }`
	`750`	`+ if (s_globalInstanceForTrackerSupport == wrapper)`
`747`	`751`	`{`
`748`	`752`	`return s_globalInstanceForTrackerSupport.GetOrCreateObjectForComInstance(externalComObject, CreateObjectFlags.TrackerObject);`
`749`	`753`	`}`
`750`		`- else if (s_globalInstanceForMarshalling != null && s_globalInstanceForMarshalling == wrapper)`
	`754`	`+ else if (s_globalInstanceForMarshalling == wrapper)`
`751`	`755`	`{`
`752`		`- return ComObjectForInterface(externalComObject);`
	`756`	`+ return ComObjectForInterface(externalComObject, CreateObjectFlags.TrackerObject \| CreateObjectFlags.Unwrap);`
`753`	`757`	`}`
`754`	`758`	`else`
`755`	`759`	`{`
`@@ -1470,7 +1474,12 @@ internal static IntPtr ComInterfaceForObject(object instance)`
`1470`	`1474`	`throw new NotSupportedException(SR.InvalidOperation_ComInteropRequireComWrapperInstance);`
`1471`	`1475`	`}`
`1472`	`1476`
`1473`		`- return s_globalInstanceForMarshalling.GetOrCreateComInterfaceForObject(instance, CreateComInterfaceFlags.None);`
	`1477`	`+ if (TryGetComInstance(instance, out IntPtr comObject))`
	`1478`	`+ {`
	`1479`	`+ return comObject;`
	`1480`	`+ }`
	`1481`	`+`
	`1482`	`+ return s_globalInstanceForMarshalling.GetOrCreateComInterfaceForObject(instance, CreateComInterfaceFlags.TrackerSupport);`
`1474`	`1483`	`}`
`1475`	`1484`
`1476`	`1485`	`internal static unsafe IntPtr ComInterfaceForObject(object instance, Guid targetIID)`
`@@ -1489,15 +1498,14 @@ internal static unsafe IntPtr ComInterfaceForObject(object instance, Guid target`
`1489`	`1498`	`return comObjectInterface;`
`1490`	`1499`	`}`
`1491`	`1500`
`1492`		`- internal static object ComObjectForInterface(IntPtr externalComObject)`
	`1501`	`+ internal static object ComObjectForInterface(IntPtr externalComObject, CreateObjectFlags flags)`
`1493`	`1502`	`{`
`1494`	`1503`	`if (s_globalInstanceForMarshalling == null)`
`1495`	`1504`	`{`
`1496`	`1505`	`throw new NotSupportedException(SR.InvalidOperation_ComInteropRequireComWrapperInstance);`
`1497`	`1506`	`}`
`1498`	`1507`
`1499`		`- // TrackerObject support and unwrapping matches the built-in semantics that the global marshalling scenario mimics.`
`1500`		`- return s_globalInstanceForMarshalling.GetOrCreateObjectForComInstance(externalComObject, CreateObjectFlags.TrackerObject \| CreateObjectFlags.Unwrap);`
	`1508`	`+ return s_globalInstanceForMarshalling.GetOrCreateObjectForComInstance(externalComObject, flags);`
`1501`	`1509`	`}`
`1502`	`1510`
`1503`	`1511`	`internal static IntPtr GetOrCreateTrackerTarget(IntPtr externalComObject)`