From e496d458a2077c599fd8755a7a94e5b5a7a6fcfa Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 1 May 2023 04:31:27 -0700 Subject: [PATCH 1/2] ComWrappers test that allocate object can't inline There are some JIT stress settings that aggressively inline functions, regardless of size/complexity. This inlining can perturb the subtle GC assumptions inherent in the COM wrappers tests. --- src/tests/Interop/COM/ComWrappers/API/Program.cs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/tests/Interop/COM/ComWrappers/API/Program.cs b/src/tests/Interop/COM/ComWrappers/API/Program.cs index 2fd4869851b613..16f13bd9b50744 100644 --- a/src/tests/Interop/COM/ComWrappers/API/Program.cs +++ b/src/tests/Interop/COM/ComWrappers/API/Program.cs @@ -123,6 +123,7 @@ static void ForceGC() } } + [MethodImpl(MethodImplOptions.NoInlining)] static void ValidateComInterfaceCreation() { Console.WriteLine($"Running {nameof(ValidateComInterfaceCreation)}..."); @@ -156,6 +157,7 @@ static void ValidateComInterfaceCreation() Assert.Equal(0, count); } + [MethodImpl(MethodImplOptions.NoInlining)] static void ValidateComInterfaceCreationRoundTrip() { Console.WriteLine($"Running {nameof(ValidateComInterfaceCreationRoundTrip)}..."); @@ -212,6 +214,7 @@ static IntPtr CreateObjectAndGetComInterface() // Just because one use of a COM interface returned from GetOrCreateComInterfaceForObject // hits zero ref count does not mean future calls to GetOrCreateComInterfaceForObject // should return an unusable object. + [MethodImpl(MethodImplOptions.NoInlining)] static void ValidateCreatingAComInterfaceForObjectAfterTheFirstIsFree() { Console.WriteLine($"Running {nameof(ValidateCreatingAComInterfaceForObjectAfterTheFirstIsFree)}..."); @@ -247,6 +250,7 @@ unsafe static void CallSetValue(TestComWrappers wrappers, Test testInstance, int } } + [MethodImpl(MethodImplOptions.NoInlining)] static void ValidateFallbackQueryInterface() { Console.WriteLine($"Running {nameof(ValidateFallbackQueryInterface)}..."); @@ -311,25 +315,26 @@ static void ValidateCreateObjectGcBehavior() IntPtr trackerObjRaw = MockReferenceTrackerRuntime.CreateTrackerObject(); // Create the first native object wrapper and run the GC. - CreateObject(); - GC.Collect(); + CreateObject(cw, trackerObjRaw); + ForceGC(); // Try to create another wrapper for the same object. The above GC // may have collected parts of the ComWrapper cache, but this should // still work. - CreateObject(); + CreateObject(cw, trackerObjRaw); ForceGC(); Marshal.Release(trackerObjRaw); [MethodImpl(MethodImplOptions.NoInlining)] - void CreateObject() + static void CreateObject(ComWrappers cw, IntPtr trackerObj) { - var obj = (ITrackerObjectWrapper)cw.GetOrCreateObjectForComInstance(trackerObjRaw, CreateObjectFlags.None); + var obj = (ITrackerObjectWrapper)cw.GetOrCreateObjectForComInstance(trackerObj, CreateObjectFlags.None); Assert.NotNull(obj); } } + [MethodImpl(MethodImplOptions.NoInlining)] static void ValidateMappingAPIs() { Console.WriteLine($"Running {nameof(ValidateMappingAPIs)}..."); @@ -383,6 +388,7 @@ static void ValidateMappingAPIs() Marshal.Release(unmanagedObjIUnknown); } + [MethodImpl(MethodImplOptions.NoInlining)] static void ValidateWrappersInstanceIsolation() { Console.WriteLine($"Running {nameof(ValidateWrappersInstanceIsolation)}..."); From 90832100cae213a19ab039cfb7a92270ccb841d8 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 1 May 2023 08:04:40 -0700 Subject: [PATCH 2/2] Update test name and comments --- .../Interop/COM/ComWrappers/API/Program.cs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/tests/Interop/COM/ComWrappers/API/Program.cs b/src/tests/Interop/COM/ComWrappers/API/Program.cs index 16f13bd9b50744..1a2b71ba2f881a 100644 --- a/src/tests/Interop/COM/ComWrappers/API/Program.cs +++ b/src/tests/Interop/COM/ComWrappers/API/Program.cs @@ -304,10 +304,11 @@ static void ValidateCreateObjectCachingScenario() Assert.NotEqual(trackerObj1, trackerObj3); } - // Make sure that if one wrapper is GCed, another can be created. - static void ValidateCreateObjectGcBehavior() + // Verify that if a GC nulls the contents of a weak GCHandle but has not yet + // run finializers to remove that GCHandle from the cache, the state of the system is valid. + static void ValidateCreateObjectWeakHandleCacheCleanUp() { - Console.WriteLine($"Running {nameof(ValidateCreateObjectCachingScenario)}..."); + Console.WriteLine($"Running {nameof(ValidateCreateObjectWeakHandleCacheCleanUp)}..."); var cw = new TestComWrappers(); @@ -316,11 +317,15 @@ static void ValidateCreateObjectGcBehavior() // Create the first native object wrapper and run the GC. CreateObject(cw, trackerObjRaw); - ForceGC(); + + // Only attempt to run the GC, don't wait for the finalizer. We do this + // because of the multiple phase clean-up for ComWrappers caches. + // See weak GC handles in the NativeAOT scenario. + GC.Collect(); // Try to create another wrapper for the same object. The above GC - // may have collected parts of the ComWrapper cache, but this should - // still work. + // may have collected parts of the ComWrapper cache, but not fully + // cleared the contents of the cache. CreateObject(cw, trackerObjRaw); ForceGC(); @@ -814,7 +819,7 @@ static int Main() ValidateCreatingAComInterfaceForObjectAfterTheFirstIsFree(); ValidateFallbackQueryInterface(); ValidateCreateObjectCachingScenario(); - ValidateCreateObjectGcBehavior(); + ValidateCreateObjectWeakHandleCacheCleanUp(); ValidateMappingAPIs(); ValidateWrappersInstanceIsolation(); ValidatePrecreatedExternalWrapper();