Merge pull request #2541 from shrah/master

Implement out SafeHandle marshaller

3 files changed, 94 insertions, 18 deletions

diff --git a/src/Common/src/TypeSystem/Interop/IL/Marshaller.cs b/src/Common/src/TypeSystem/Interop/IL/Marshaller.cs
index abfec6ab7..7b6f5cb7d 100644
--- a/src/Common/src/TypeSystem/Interop/IL/Marshaller.cs
+++ b/src/Common/src/TypeSystem/Interop/IL/Marshaller.cs
@@ -55,7 +55,10 @@ namespace Internal.TypeSystem.Interop
         /// <returns>The  created Marshaller</returns>
         public static Marshaller CreateMarshaller(TypeDesc parameterType, PInvokeMethodData pInvokeMethodData, ParameterMetadata pInvokeParameterdata)
         {
-            MarshallerKind marshallerKind = GetMarshallerKind(parameterType, pInvokeParameterdata.MarshalAsDescriptor, pInvokeMethodData, pInvokeParameterdata.Return);
+            MarshallerKind marshallerKind = GetMarshallerKind(parameterType, 
+                                                pInvokeParameterdata.MarshalAsDescriptor, 
+                                                pInvokeMethodData, 
+                                                pInvokeParameterdata);
             
             // Create the marshaller based on MarshallerKind
             Marshaller marshaller = Marshaller.CreateMarshallerInternal(marshallerKind);
@@ -92,9 +95,9 @@ namespace Internal.TypeSystem.Interop
                     throw new NotSupportedException();
             }
         }
-        private static MarshallerKind GetMarshallerKind(TypeDesc type, MarshalAsDescriptor marshalAs, PInvokeMethodData PInvokeMethodData, bool isReturn)
+        private static MarshallerKind GetMarshallerKind(TypeDesc type, MarshalAsDescriptor marshalAs, PInvokeMethodData PInvokeMethodData, ParameterMetadata paramMetadata)
         {
-            if (isReturn)
+            if (paramMetadata.Return)
             {
                 if (type.IsVoid)
                 {
@@ -184,6 +187,21 @@ namespace Internal.TypeSystem.Interop
             {
                 return MarshallerKind.SafeHandle;
             }
+
+            // Temporary fix for out SafeHandle scenario
+            // TODO: handle in,out,ref properly
+            if (paramMetadata.Out && !paramMetadata.In)
+            {
+                ByRefType byRefType = type as ByRefType;
+                if (byRefType != null)
+                {
+                    if (PInvokeMethodData.IsSafeHandle(byRefType.ParameterType))
+                    {
+                        return MarshallerKind.SafeHandle;
+                    }
+                }
+            }
+
             throw new NotSupportedException();
         }
         #endregion
@@ -366,28 +384,68 @@ namespace Internal.TypeSystem.Interop
     {
         protected override TypeDesc MarshalArgument(TypeDesc managedType, ILEmitter emitter, ILCodeStream marshallingCodeStream, ILCodeStream unmarshallingCodeStream)
         {
+            // we don't support [IN,OUT] together yet, either IN or OUT
+            Debug.Assert(!(PInvokeParameterMetadata.Out && PInvokeParameterMetadata.In));
+
             var safeHandleType = PInvokeMethodData.SafeHandleType;
 
-            var vAddRefed = emitter.NewLocal(PInvokeMethodData.Context.GetWellKnownType(WellKnownType.Boolean));
-            var vSafeHandle = emitter.NewLocal(managedType);
+            if (PInvokeParameterMetadata.Out)
+            {
+                // 1) If this is an output parameter we need to preallocate a SafeHandle to wrap the new native handle value. We
+                //    must allocate this before the native call to avoid a failure point when we already have a native resource
+                //    allocated. We must allocate a new SafeHandle even if we have one on input since both input and output native
+                //    handles need to be tracked and released by a SafeHandle.
+                // 2) Initialize a local IntPtr that will be passed to the native call. 
+                // 3) After the native call, the new handle value is written into the output SafeHandle and that SafeHandle
+                //    is propagated back to the caller.
+
+                Debug.Assert(managedType is ByRefType);
+                var vOutArg = emitter.NewLocal(managedType);
+                marshallingCodeStream.EmitStLoc(vOutArg);
+
+                TypeDesc resolvedType = ((ByRefType)managedType).ParameterType;
+
+                var nativeType = PInvokeMethodData.Context.GetWellKnownType(WellKnownType.IntPtr).MakeByRefType();
+                var vPinnedOutValue = emitter.NewLocal(nativeType, true);
+                var vSafeHandle = emitter.NewLocal(resolvedType);
+                marshallingCodeStream.Emit(ILOpcode.newobj, emitter.NewToken(resolvedType.GetDefaultConstructor()));
+                marshallingCodeStream.EmitStLoc(vSafeHandle);
+                marshallingCodeStream.EmitLdLoca(vPinnedOutValue);
+
+                unmarshallingCodeStream.EmitLdLoc(vSafeHandle);
+                unmarshallingCodeStream.EmitLdLoc(vPinnedOutValue);
+                unmarshallingCodeStream.Emit(ILOpcode.call, emitter.NewToken(
+                    PInvokeMethodData.SafeHandleType.GetKnownMethod("SetHandle", null)));
 
-            marshallingCodeStream.EmitStLoc(vSafeHandle);
+                unmarshallingCodeStream.EmitLdLoc(vOutArg);
+                unmarshallingCodeStream.EmitLdLoc(vSafeHandle);
+                unmarshallingCodeStream.Emit(ILOpcode.stind_i);
 
-            marshallingCodeStream.EmitLdLoc(vSafeHandle);
-            marshallingCodeStream.EmitLdLoca(vAddRefed);
-            marshallingCodeStream.Emit(ILOpcode.call, emitter.NewToken(
-                safeHandleType.GetKnownMethod("DangerousAddRef", null)));
+                return nativeType;
+            }
+            else
+            {
+                var vAddRefed = emitter.NewLocal(PInvokeMethodData.Context.GetWellKnownType(WellKnownType.Boolean));
+                var vSafeHandle = emitter.NewLocal(managedType);
 
-            marshallingCodeStream.EmitLdLoc(vSafeHandle);
-            marshallingCodeStream.Emit(ILOpcode.call, emitter.NewToken(
-                safeHandleType.GetKnownMethod("DangerousGetHandle", null)));
+                marshallingCodeStream.EmitStLoc(vSafeHandle);
 
-            // TODO: This should be inside finally block and only executed it the handle was addrefed
-            unmarshallingCodeStream.EmitLdLoc(vSafeHandle);
-            unmarshallingCodeStream.Emit(ILOpcode.call, emitter.NewToken(
-                safeHandleType.GetKnownMethod("DangerousRelease", null)));
+                marshallingCodeStream.EmitLdLoc(vSafeHandle);
+                marshallingCodeStream.EmitLdLoca(vAddRefed);
+                marshallingCodeStream.Emit(ILOpcode.call, emitter.NewToken(
+                    safeHandleType.GetKnownMethod("DangerousAddRef", null)));
 
-            return PInvokeMethodData.Context.GetWellKnownType(WellKnownType.IntPtr);
+                marshallingCodeStream.EmitLdLoc(vSafeHandle);
+                marshallingCodeStream.Emit(ILOpcode.call, emitter.NewToken(
+                    safeHandleType.GetKnownMethod("DangerousGetHandle", null)));
+
+                // TODO: This should be inside finally block and only executed it the handle was addrefed
+                unmarshallingCodeStream.EmitLdLoc(vSafeHandle);
+                unmarshallingCodeStream.Emit(ILOpcode.call, emitter.NewToken(
+                    safeHandleType.GetKnownMethod("DangerousRelease", null)));
+
+                return PInvokeMethodData.Context.GetWellKnownType(WellKnownType.IntPtr);
+            }
         }
 
         protected override TypeDesc MarshalReturn(TypeDesc managedType, ILEmitter emitter, ILCodeStream marshallingCodeStream, ILCodeStream returnValueMarshallingCodeStream )
diff --git a/tests/src/Simple/PInvoke/PInvoke.cs b/tests/src/Simple/PInvoke/PInvoke.cs
index 1e7a6fa99..ed7f53fb1 100644
--- a/tests/src/Simple/PInvoke/PInvoke.cs
+++ b/tests/src/Simple/PInvoke/PInvoke.cs
@@ -36,6 +36,9 @@ namespace PInvoke
         [DllImport("*", CallingConvention = CallingConvention.StdCall)]
         public static extern bool SafeHandleTest(SafeMemoryHandle sh1, Int64 sh1Value);
 
+        [DllImport("*", CallingConvention = CallingConvention.StdCall)]
+        public static extern int SafeHandleOutTest(out SafeMemoryHandle sh1);
+
         [DllImport("*", CallingConvention = CallingConvention.StdCall, SetLastError = true)]
         public static extern bool LastErrorTest();
 
@@ -117,6 +120,12 @@ namespace PInvoke
             long val =  hndIntPtr.ToInt64(); //return the 64-bit value associated with hnd
 
             ThrowIfNotEquals(true, SafeHandleTest(hnd, val), "SafeHandle marshalling failed.");
+
+            Console.WriteLine("Testing marshalling out SafeHandle");
+            SafeMemoryHandle hnd2;
+            val = SafeHandleOutTest(out hnd2);
+            int val2 = unchecked((int)hnd2.DangerousGetHandle().ToInt64());
+            ThrowIfNotEquals(val, val2, "SafeHandle out marshalling failed");
         }
     }
 
diff --git a/tests/src/Simple/PInvoke/PInvokeNative.cpp b/tests/src/Simple/PInvoke/PInvokeNative.cpp
index ca66eafb9..8c70cbb6e 100644
--- a/tests/src/Simple/PInvoke/PInvokeNative.cpp
+++ b/tests/src/Simple/PInvoke/PInvokeNative.cpp
@@ -116,3 +116,12 @@ DLL_EXPORT bool __stdcall SafeHandleTest(HANDLE sh, long shValue)
 {
     return (long)((size_t)(sh)) == shValue;
 }
+
+DLL_EXPORT long __stdcall SafeHandleOutTest(HANDLE **sh)
+{
+    if (sh == NULL) 
+        return -1;
+
+    *sh = (HANDLE *)malloc(100);
+    return (long)((size_t)(*sh));
+}