Fix Interop/PInvoke/Miscellaneous/HandleRef tests under GCStress (#21131)
authorJeremy Koritzinsky <jkoritzinsky@gmail.com>
Sun, 25 Nov 2018 00:35:17 +0000 (18:35 -0600)
committerGitHub <noreply@github.com>
Sun, 25 Nov 2018 00:35:17 +0000 (18:35 -0600)
* Add GC.KeepAlive call in the IL stub when marshalling a HandleRef.

* Add a GC.KeepAlive call for the BoxedInt since it isn't kept alive when running GC Stress

* Reference HandleRef._handle via the binder instead of by field offset arithmetic

* Alignment

* Use Marshal.Alloc/FreeHGlobal instead of the weird no-pinning machinery that was failing before

* Update HandleRefTest.cs

src/vm/ilmarshalers.cpp
src/vm/mscorlib.h
tests/src/Interop/PInvoke/Miscellaneous/HandleRef/HandleRefTest.cs

index bf0cf3c..58319d8 100644 (file)
@@ -2548,6 +2548,7 @@ MarshalerOverrideStatus ILHandleRefMarshaler::ArgumentOverride(NDirectStubLinker
 
     ILCodeStream* pcsMarshal    = psl->GetMarshalCodeStream();
     ILCodeStream* pcsDispatch   = psl->GetDispatchCodeStream();
+    ILCodeStream* pcsUnmarshal  = psl->GetUnmarshalCodeStream();
 
     if (fManagedToNative && !byref)
     {
@@ -2556,10 +2557,15 @@ MarshalerOverrideStatus ILHandleRefMarshaler::ArgumentOverride(NDirectStubLinker
 
         // HandleRefs are valuetypes, so pinning is not needed.
         // The argument address is on the stack and will not move.
-        pcsDispatch->EmitLDARGA(argidx);
-        pcsDispatch->EmitLDC(offsetof(HANDLEREF, m_handle));
-        pcsDispatch->EmitADD();
-        pcsDispatch->EmitLDIND_I();
+        mdFieldDef handleField = pcsDispatch->GetToken(MscorlibBinder::GetField(FIELD__HANDLE_REF__HANDLE));
+        pcsDispatch->EmitLDARG(argidx);
+        pcsDispatch->EmitLDFLD(handleField);
+
+        mdFieldDef wrapperField = pcsUnmarshal->GetToken(MscorlibBinder::GetField(FIELD__HANDLE_REF__WRAPPER));
+        pcsUnmarshal->EmitLDARG(argidx);
+        pcsUnmarshal->EmitLDFLD(wrapperField);
+        pcsUnmarshal->EmitCALL(METHOD__GC__KEEP_ALIVE, 1, 0);
+
         return OVERRIDDEN;
     }
     else
index 768bf8f..26be735 100644 (file)
@@ -225,6 +225,10 @@ DEFINE_METHOD(CRITICAL_HANDLE,      GET_IS_INVALID,         get_IsInvalid,
 DEFINE_METHOD(CRITICAL_HANDLE,      DISPOSE,                Dispose,                    IM_RetVoid)
 DEFINE_METHOD(CRITICAL_HANDLE,      DISPOSE_BOOL,           Dispose,                    IM_Bool_RetVoid)
 
+DEFINE_CLASS(HANDLE_REF,            Interop,                HandleRef)
+DEFINE_FIELD(HANDLE_REF,            WRAPPER,                _wrapper)
+DEFINE_FIELD(HANDLE_REF,            HANDLE,                 _handle)
+
 DEFINE_CLASS(CRITICAL_FINALIZER_OBJECT, ConstrainedExecution, CriticalFinalizerObject)
 DEFINE_METHOD(CRITICAL_FINALIZER_OBJECT, FINALIZE,          Finalize,                   IM_RetVoid)
 
index 6f6ed52..6eba3cb 100644 (file)
@@ -8,11 +8,6 @@ using System.Reflection;
 using System.Text;
 using TestLibrary;
 
-class BoxedInt
-{
-    public int MyInt;
-}
-
 class HandleRefTest
 {
     [DllImport(@"HandleRefNative", CallingConvention = CallingConvention.Winapi)]
@@ -60,16 +55,7 @@ class HandleRefTest
             // stay rooted until the end of the method. 
             Console.WriteLine("TestNoGC");
 
-            // Keep the int boxed and pinned to prevent it from getting collected.
-            // That way, we can safely reference it from finalizers that run on shutdown.
-            BoxedInt boxedInt = new BoxedInt();
-            GCHandle.Alloc(boxedInt, GCHandleType.Normal);
-            int* int4Ptr;
-            fixed (int* tempIntPtr = &boxedInt.MyInt)
-            {
-                // Smuggle the pointer out of the fixed scope
-                int4Ptr = tempIntPtr;
-            }
+            int* int4Ptr = (int*)Marshal.AllocHGlobal(sizeof(int)); // We don't free this memory so we don't have to worry about a GC run between freeing and return (possible in a GCStress mode).
             Console.WriteLine("2");
             *int4Ptr = intManaged;
             CollectableClass collectableClass = new CollectableClass(int4Ptr);