Add test verifying reliable SafeHandle unmarshalling (#24959)

* Add test ensuring we correctly unmarshal a SafeHandle even when the stub throws an exception.

* PR Feedback.

8 files changed, 99 insertions, 161 deletions

diff --git a/tests/src/Interop/CMakeLists.txt b/tests/src/Interop/CMakeLists.txt
index b8dc69c31c..a203a9bb3f 100644
--- a/tests/src/Interop/CMakeLists.txt
+++ b/tests/src/Interop/CMakeLists.txt
@@ -71,7 +71,7 @@ if(WIN32)
     add_subdirectory(PInvoke/Variant)
     add_subdirectory(PInvoke/Varargs)
     add_subdirectory(PInvoke/SafeHandles)
-    add_subdirectory(PInvoke/SafeHandles/ReleaseHandle)
+    add_subdirectory(PInvoke/SafeHandles/ReliableUnmarshal)
     add_subdirectory(PInvoke/SafeHandles/Interface)
     add_subdirectory(PInvoke/NativeCallManagedComVisible)
     # Windows-only due to bug (fixed as part of dotnet/coreclr#21415)
diff --git a/tests/src/Interop/PInvoke/SafeHandles/ReleaseHandle/CMakeLists.txt b/tests/src/Interop/PInvoke/SafeHandles/ReleaseHandle/CMakeLists.txt
deleted file mode 100644
index 53a02888ad..0000000000
--- a/tests/src/Interop/PInvoke/SafeHandles/ReleaseHandle/CMakeLists.txt
+++ /dev/null
@@ -1,13 +0,0 @@
- 
-cmake_minimum_required (VERSION 2.6) 
-project (PInvoke_SafeHandle_ReleaseHandle) 
-include ("${CLR_INTEROP_TEST_ROOT}/Interop.cmake") 
-set(SOURCES 
-    ReleaseHandleNative.cpp 
-) 
-# Additional files to reference: 
-#    ReleaseHandleNative.def 
-# add the executable 
-add_library (PInvoke_SafeHandle_ReleaseHandle SHARED ${SOURCES}) 
-# add the install targets 
-install (TARGETS PInvoke_SafeHandle_ReleaseHandle DESTINATION bin) 
diff --git a/tests/src/Interop/PInvoke/SafeHandles/ReleaseHandle/ReleaseHandleNative.cpp b/tests/src/Interop/PInvoke/SafeHandles/ReleaseHandle/ReleaseHandleNative.cpp
deleted file mode 100644
index cbf76d69c3..0000000000
--- a/tests/src/Interop/PInvoke/SafeHandles/ReleaseHandle/ReleaseHandleNative.cpp
+++ /dev/null
@@ -1,42 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <windows.h>
-#include <xplatform.h>
-#include <platformdefines.h>
-
-bool g_myResourceReleaseMethodCalled = false;
-
-extern "C" DLL_EXPORT void __stdcall MyResourceReleaseMethod(HANDLE hnd)
-{
-    g_myResourceReleaseMethodCalled = true;
-    
-    //call CloseHandle to actually release the handle corresponding to the SafeFileHandle
-    CloseHandle(hnd);
-}
-
-extern "C" DLL_EXPORT bool GetMyResourceReleaseMethodCalled()
-{
-    return g_myResourceReleaseMethodCalled;
-}
-
-extern "C" DLL_EXPORT void ResetMyResourceReleaseMethodCalled()
-{
-    g_myResourceReleaseMethodCalled = false;
-}
-
-extern "C" DLL_EXPORT void __stdcall SHReleasing_OutParams(IUnknown* ppIUnknFOO, HANDLE* phnd, IUnknown** ppIUnknBAR, int* pInt)
-{
-    //initialize the hnd out param
-    *phnd = (HANDLE)123;
-
-    //initialize the IUnknBAR out param
-    *ppIUnknBAR = ppIUnknFOO;
-    ppIUnknFOO->AddRef(); //addref Foo
-
-    //initialize the int out param
-    *pInt = 123;
-}
diff --git a/tests/src/Interop/PInvoke/SafeHandles/ReleaseHandle/ReleaseHandleTest.cs b/tests/src/Interop/PInvoke/SafeHandles/ReleaseHandle/ReleaseHandleTest.cs
deleted file mode 100644
index b8e16c7d47..0000000000
--- a/tests/src/Interop/PInvoke/SafeHandles/ReleaseHandle/ReleaseHandleTest.cs
+++ /dev/null
@@ -1,102 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-using System;
-using System.IO;
-using System.Runtime.InteropServices;
-using System.Runtime.ConstrainedExecution;
-using SafeHandlesTests;
-using TestLibrary;
-
-class SafeFileHandle : SafeHandle //SafeHandle subclass
-{
-    private static readonly IntPtr _invalidHandleValue = new IntPtr(-1);
-
-    //0 or -1 considered invalid
-    public override bool IsInvalid
-    {
-        get { return handle == IntPtr.Zero || handle == _invalidHandleValue; }
-    }
-
-    [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)]
-    [DllImport("PInvoke_SafeHandle_ReleaseHandle")]
-    private static extern bool MyResourceReleaseMethod(IntPtr handle);
-
-    //default constructor which just calls the base class constructor
-    public SafeFileHandle()
-        : base(IntPtr.Zero, true)
-    {
-    }
-
-    override protected bool ReleaseHandle()
-    {
-        //	this method will not actually call any resource releasing API method
-        //	since the out/ref SFH param is not actually initialized to an OS allocated
-        //	HANDLE---instead the unmanaged side just initializes/changes it to some integer;
-        //	If a resource releasing API method like CloseHandle were called then
-        //	it would return false and an unhandled exception would be thrown by the
-        //	runtime indicating that the release method failed
-        MyResourceReleaseMethod(handle);
-        return true;
-    }
-
-} //end of SafeFileHandle class
-
-class Foo
-{
-    int FooMethod(int x, int y) { return x + y; }
-}
-
-class Bar
-{
-    void BarMethod() { }
-}
-
-internal class SHReleasingTester
-{
-    [DllImport("PInvoke_SafeHandle_ReleaseHandle")]
-    private static extern void SHReleasing_OutParams(
-        [MarshalAs(UnmanagedType.Interface)]Foo foo,
-        out SafeFileHandle sh,
-        [MarshalAs(UnmanagedType.Interface)]out Bar bar, out int x);
-
-    [DllImport("PInvoke_SafeHandle_ReleaseHandle")]
-    [return:MarshalAs(UnmanagedType.I1)]private static extern bool GetMyResourceReleaseMethodCalled();
-
-    [DllImport("PInvoke_SafeHandle_ReleaseHandle")]
-    private static extern void ResetMyResourceReleaseMethodCalled();
-
-    public static int Main()
-    {
-        try{
-            Console.WriteLine("SHReleasing_OutParams");
-            SafeFileHandle sh;
-            Foo foo = new Foo();
-            Bar bar;
-            int x;
-
-            ResetMyResourceReleaseMethodCalled();
-
-            //this unmanaged method will try to set the out Bar parameter to a Foo type
-            //this should cause an InvalidCastException on the way back from unmanaged
-            Assert.Throws<InvalidCastException>(() => SHReleasing_OutParams(foo, out sh, out bar, out x), "SHReleasing_OutParams");
-
-            //force the finalizer for the SFH param to run
-            Console.WriteLine("\tForcing finalizer for the SFH param to run...");
-            sh = null;
-            GC.Collect();
-            GC.WaitForPendingFinalizers();
-
-            Assert.IsTrue(GetMyResourceReleaseMethodCalled(), "MyResourceReleaseMethod was NOT called");
-
-            return 100;
-        }
-        catch (Exception e)
-        {
-            Console.WriteLine($"Test Failure: {e}");
-            return 101;
-        }
-    }
-} 
-
diff --git a/tests/src/Interop/PInvoke/SafeHandles/ReliableUnmarshal/CMakeLists.txt b/tests/src/Interop/PInvoke/SafeHandles/ReliableUnmarshal/CMakeLists.txt
new file mode 100644
index 0000000000..9707076e97
--- /dev/null
+++ b/tests/src/Interop/PInvoke/SafeHandles/ReliableUnmarshal/CMakeLists.txt
@@ -0,0 +1,8 @@
+cmake_minimum_required (VERSION 2.6) 
+project (ReliableUnmarshal) 
+include ("${CLR_INTEROP_TEST_ROOT}/Interop.cmake") 
+set(SOURCES 
+  ReliableUnmarshalNative.cpp 
+)
+add_library (ReliableUnmarshalNative SHARED ${SOURCES}) 
+install (TARGETS ReliableUnmarshalNative DESTINATION bin) 
diff --git a/tests/src/Interop/PInvoke/SafeHandles/ReliableUnmarshal/ReliableUnmarshalNative.cpp b/tests/src/Interop/PInvoke/SafeHandles/ReliableUnmarshal/ReliableUnmarshalNative.cpp
new file mode 100644
index 0000000000..a7d42fef00
--- /dev/null
+++ b/tests/src/Interop/PInvoke/SafeHandles/ReliableUnmarshal/ReliableUnmarshalNative.cpp
@@ -0,0 +1,16 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <xplatform.h>
+#include <platformdefines.h>
+
+using FAKE_HANDLE = HANDLE;
+
+extern "C" void DLL_EXPORT GetFakeHandle(void* value, FAKE_HANDLE* pHandle, void** pCookie)
+{
+    *pHandle = (FAKE_HANDLE)value;
+    *pCookie = (void*)4567; // the value here does not matter. It just needs to not be nullptr.
+}
diff --git a/tests/src/Interop/PInvoke/SafeHandles/ReliableUnmarshal/ReliableUnmarshalTest.cs b/tests/src/Interop/PInvoke/SafeHandles/ReliableUnmarshal/ReliableUnmarshalTest.cs
new file mode 100644
index 0000000000..47509b3964
--- /dev/null
+++ b/tests/src/Interop/PInvoke/SafeHandles/ReliableUnmarshal/ReliableUnmarshalTest.cs
@@ -0,0 +1,72 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.IO;
+using System.Runtime.InteropServices;
+using TestLibrary;
+
+class FakeHandle : SafeHandle
+{
+    public override bool IsInvalid => handle == IntPtr.Zero;
+
+    public FakeHandle()
+        : base(IntPtr.Zero, true)
+    {
+    }
+
+    override protected bool ReleaseHandle()
+    {
+        handle = IntPtr.Zero;
+        return true;
+    }
+}
+
+public class ThrowingCustomMarshaler : ICustomMarshaler
+{
+    public void CleanUpManagedData(object ManagedObj) { }
+    public void CleanUpNativeData(IntPtr pNativeData) { }
+
+    public int GetNativeDataSize() => IntPtr.Size;
+
+    public IntPtr MarshalManagedToNative(object ManagedObj) => throw new NotImplementedException();
+    public object MarshalNativeToManaged(IntPtr pNativeData)
+    {
+        // Cause an exception during the unmarshal phase of the IL stub.
+        throw new InvalidOperationException();
+    }
+
+    public static ICustomMarshaler GetInstance(string cookie) => new ThrowingCustomMarshaler();
+}
+
+internal class ReliableUnmarshalNative
+{
+    // We're using a custom marshaler here to cause an exception during the unmarshal phase of the IL stub after successfully returning from native code.
+    [DllImport(nameof(ReliableUnmarshalNative))]
+    public static extern void GetFakeHandle(IntPtr value, out FakeHandle handle, [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(ThrowingCustomMarshaler), MarshalCookie = "")] out object cookie);
+}
+
+public class ReliableUnmarshalTest
+{
+    public static int Main()
+    {
+        try
+        {
+            // Test that our SafeHandle-derived object has its underlying handle set after a P/Invoke
+            // even if there's an exception during the unmarshal phase.
+            IntPtr value = (IntPtr)123;
+            FakeHandle h = new FakeHandle();
+
+            Assert.Throws<InvalidOperationException>(() => ReliableUnmarshalNative.GetFakeHandle(value, out h, out var cookie));
+
+            Assert.AreEqual(value, h.DangerousGetHandle());
+        }
+        catch (System.Exception ex)
+        {
+            Console.WriteLine(ex);
+            return 101;
+        }
+        return 100;
+    }
+}
diff --git a/tests/src/Interop/PInvoke/SafeHandles/ReleaseHandle/ReleaseHandleTest.csproj b/tests/src/Interop/PInvoke/SafeHandles/ReliableUnmarshal/ReliableUnmarshalTest.csproj
index 54fa726b2a..d674179f97 100644
--- a/tests/src/Interop/PInvoke/SafeHandles/ReleaseHandle/ReleaseHandleTest.csproj
+++ b/tests/src/Interop/PInvoke/SafeHandles/ReliableUnmarshal/ReliableUnmarshalTest.csproj
@@ -4,7 +4,7 @@
   <PropertyGroup>
     <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
     <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
-    <AssemblyName>ReleaseHandleTest</AssemblyName>
+    <AssemblyName>ReliableUnmarshalTest</AssemblyName>
     <SchemaVersion>2.0</SchemaVersion>
     <ProjectGuid>{F1E66554-8C8E-4141-85CF-D0CD6A0CD0B0}</ProjectGuid>
     <OutputType>Exe</OutputType>
@@ -24,8 +24,7 @@
     </CodeAnalysisDependentAssemblyPaths>
   </ItemGroup>
   <ItemGroup>
-    <Compile Include="ReleaseHandleTest.cs" />
-    <Compile Include="..\*.cs" />
+    <Compile Include="ReliableUnmarshalTest.cs" />
   </ItemGroup>
   <ItemGroup>
     <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />