Add new SafeHandleMarshaller type to provide out-of-generator marshalling support...
authorJeremy Koritzinsky <jekoritz@microsoft.com>
Wed, 3 May 2023 18:28:53 +0000 (11:28 -0700)
committerGitHub <noreply@github.com>
Wed, 3 May 2023 18:28:53 +0000 (11:28 -0700)
Fixes #74035

We can't remove the built-in marshalling support from the generator yet, but once the out-of-band packages we ship don't support .NET 6. we can remove the built-in support that emits the marshalling code in the stub. I believe the .NET 9 packages won't support .NET 6, so once we snap for .NET 9 and update how we ship the packages, we can clean this up.

This PR also adds a requested feature to the SafeHandle marshaller: If the call to native code fails, we'll call Dispose() on the pre-allocated handle to avoid leaking it to the finalizer queue.

docs/design/libraries/LibraryImportGenerator/Compatibility.md
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/SafeHandleMarshaller.cs [new file with mode: 0644]
src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Microsoft.Interop.SourceGeneration.csproj
src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/SafeHandleMarshallingInfoProvider.cs
src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypeNames.cs
src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/SafeHandleTests.cs
src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CompileFails.cs
src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs
src/libraries/System.Runtime/ref/System.Runtime.cs

index 27cf3fa..cd2e31f 100644 (file)
@@ -2,7 +2,13 @@
 
 Documentation on compatibility guidance and the current state. The version headings act as a rolling delta between the previous version.
 
-## Version 2
+## Version 3 (.NET 8)
+
+### Safe Handles
+
+Due to trimming issues with NativeAOT's implementation of `Activator.CreateInstance`, we have decided to change our recommendation of providing a public parameterless constructor for `ref`, `out`, and return scenarios to a requirement. We already required a parameterless constructor of some visibility, so changing to a requirement matches our design principles of taking breaking changes to make interop more understandable and enforce more of our best practices instead of going out of our way to provide backward compatibility at increasing costs.
+
+## Version 2 (.NET 7 Release)
 
 The focus of version 2 is to support all repos that make up the .NET Product, including ASP.NET Core and Windows Forms, as well as all packages in dotnet/runtime.
 
@@ -11,7 +17,7 @@ The focus of version 2 is to support all repos that make up the .NET Product, in
 Support for user-defined type marshalling in the source-generated marshalling is described in [UserTypeMarshallingV2.md](UserTypeMarshallingV2.md). This support replaces the designs specified in [StructMarshalling.md](StructMarshalling.md) and [SpanMarshallers.md](SpanMarshallers.md).
 
 
-## Version 1
+## Version 1 (.NET 6 Prototype and .NET 7 Previews)
 
 The focus of version 1 is to support `NetCoreApp`. This implies that anything not needed by `NetCoreApp` is subject to change.
 
index b27df31..14df2cd 100644 (file)
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\Marshalling\NativeMarshallingAttribute.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\Marshalling\PointerArrayMarshaller.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\Marshalling\ReadOnlySpanMarshaller.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\Marshalling\SafeHandleMarshaller.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\Marshalling\SpanMarshaller.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\Marshalling\Utf16StringMarshaller.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\Marshalling\Utf8StringMarshaller.cs" />
diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/SafeHandleMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/SafeHandleMarshaller.cs
new file mode 100644 (file)
index 0000000..0e392d1
--- /dev/null
@@ -0,0 +1,199 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Diagnostics.CodeAnalysis;
+
+namespace System.Runtime.InteropServices.Marshalling
+{
+    /// <summary>
+    /// A marshaller for <see cref="SafeHandle"/>-derived types that marshals the handle following the lifetime rules for <see cref="SafeHandle"/>s.
+    /// </summary>
+    /// <typeparam name="T">The <see cref="SafeHandle"/>-derived type.</typeparam>
+    [CustomMarshaller(typeof(CustomMarshallerAttribute.GenericPlaceholder), MarshalMode.ManagedToUnmanagedIn, typeof(SafeHandleMarshaller<>.ManagedToUnmanagedIn))]
+    [CustomMarshaller(typeof(CustomMarshallerAttribute.GenericPlaceholder), MarshalMode.ManagedToUnmanagedRef, typeof(SafeHandleMarshaller<>.ManagedToUnmanagedRef))]
+    [CustomMarshaller(typeof(CustomMarshallerAttribute.GenericPlaceholder), MarshalMode.ManagedToUnmanagedOut, typeof(SafeHandleMarshaller<>.ManagedToUnmanagedOut))]
+    public static class SafeHandleMarshaller<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] T> where T : SafeHandle
+    {
+        /// <summary>
+        /// Custom marshaller to marshal a <see cref="SafeHandle"/> as its underlying handle value.
+        /// </summary>
+        public struct ManagedToUnmanagedIn
+        {
+            private bool _addRefd;
+            private T? _handle;
+
+            /// <summary>
+            /// Initializes the marshaller from a managed handle.
+            /// </summary>
+            /// <param name="handle">The managed handle.</param>
+            public void FromManaged(T handle)
+            {
+                _handle = handle;
+                handle.DangerousAddRef(ref _addRefd);
+            }
+
+            /// <summary>
+            /// Get the unmanaged handle.
+            /// </summary>
+            /// <returns>The unmanaged handle.</returns>
+            public IntPtr ToUnmanaged() => _handle!.DangerousGetHandle();
+
+            /// <summary>
+            /// Release any references keeping the managed handle alive.
+            /// </summary>
+            public void Free()
+            {
+                if (_addRefd)
+                {
+                    _handle!.DangerousRelease();
+                }
+            }
+        }
+
+        /// <summary>
+        /// Custom marshaller to marshal a <see cref="SafeHandle"/> as its underlying handle value.
+        /// </summary>
+        public struct ManagedToUnmanagedRef
+        {
+            private bool _addRefd;
+            private bool _callInvoked;
+            private T? _handle;
+            private IntPtr _originalHandleValue;
+            private T _newHandle;
+            private T? _handleToReturn;
+
+            /// <summary>
+            /// Create the marshaller in a default state.
+            /// </summary>
+            public ManagedToUnmanagedRef()
+            {
+                _addRefd = false;
+                _callInvoked = false;
+                // SafeHandle ref marshalling has always required parameterless constructors,
+                // but it has never required them to be public.
+                // We construct the handle now to ensure we don't cause an exception
+                // before we are able to capture the unmanaged handle after the call.
+                _newHandle = Activator.CreateInstance<T>()!;
+            }
+
+            /// <summary>
+            /// Initialize the marshaller from a managed handle.
+            /// </summary>
+            /// <param name="handle">The managed handle</param>
+            public void FromManaged(T handle)
+            {
+                _handle = handle;
+                handle.DangerousAddRef(ref _addRefd);
+                _originalHandleValue = handle.DangerousGetHandle();
+            }
+
+            /// <summary>
+            /// Retrieve the unmanaged handle.
+            /// </summary>
+            /// <returns>The unmanaged handle</returns>
+            public IntPtr ToUnmanaged() => _originalHandleValue;
+
+            /// <summary>
+            /// Initialize the marshaller from an unmanaged handle.
+            /// </summary>
+            /// <param name="value">The unmanaged handle.</param>
+            public void FromUnmanaged(IntPtr value)
+            {
+                if (value == _originalHandleValue)
+                {
+                    _handleToReturn = _handle;
+                }
+                else
+                {
+                    Marshal.InitHandle(_newHandle, value);
+                    _handleToReturn = _newHandle;
+                }
+            }
+
+            /// <summary>
+            /// Notify the marshaller that the native call has been invoked.
+            /// </summary>
+            public void OnInvoked()
+            {
+                _callInvoked = true;
+            }
+
+            /// <summary>
+            /// Retrieve the managed handle from the marshaller.
+            /// </summary>
+            /// <returns>The managed handle.</returns>
+            public T ToManagedFinally() => _handleToReturn!;
+
+            /// <summary>
+            /// Free any resources and reference counts owned by the marshaller.
+            /// </summary>
+            public void Free()
+            {
+                if (_addRefd)
+                {
+                    _handle!.DangerousRelease();
+                }
+
+                // If we never invoked the call, then we aren't going to use the
+                // new handle. Dispose it now to avoid clogging up the finalizer queue
+                // unnecessarily.
+                if (!_callInvoked)
+                {
+                    _newHandle.Dispose();
+                }
+            }
+        }
+
+        /// <summary>
+        /// Custom marshaller to marshal a <see cref="SafeHandle"/> as its underlying handle value.
+        /// </summary>
+        public struct ManagedToUnmanagedOut
+        {
+            private bool _initialized;
+            private T _newHandle;
+
+            /// <summary>
+            /// Create the marshaller in a default state.
+            /// </summary>
+            public ManagedToUnmanagedOut()
+            {
+                _initialized = false;
+                // SafeHandle out marshalling has always required parameterless constructors,
+                // but it has never required them to be public.
+                // We construct the handle now to ensure we don't cause an exception
+                // before we are able to capture the unmanaged handle after the call.
+                _newHandle = Activator.CreateInstance<T>()!;
+            }
+
+            /// <summary>
+            /// Initialize the marshaller from an unmanaged handle.
+            /// </summary>
+            /// <param name="value">The unmanaged handle.</param>
+            public void FromUnmanaged(IntPtr value)
+            {
+                _initialized = true;
+                Marshal.InitHandle(_newHandle, value);
+            }
+
+            /// <summary>
+            /// Retrieve the managed handle from the marshaller.
+            /// </summary>
+            /// <returns>The managed handle.</returns>
+            public T ToManaged() => _newHandle;
+
+            /// <summary>
+            /// Free any resources and reference counts owned by the marshaller.
+            /// </summary>
+            public void Free()
+            {
+                // If we never captured the handle value, then we aren't going to use the
+                // new handle. Dispose it now to avoid clogging up the finalizer queue
+                // unnecessarily.
+                if (!_initialized)
+                {
+                    _newHandle!.Dispose();
+                }
+            }
+        }
+    }
+}
index 0309793..a5c2f2a 100644 (file)
@@ -3,8 +3,10 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Text;
 using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.DotnetRuntime.Extensions;
 
 namespace Microsoft.Interop
 {
@@ -19,11 +21,13 @@ namespace Microsoft.Interop
     public sealed class SafeHandleMarshallingInfoProvider : ITypeBasedMarshallingInfoProvider
     {
         private readonly Compilation _compilation;
+        private readonly INamedTypeSymbol _safeHandleMarshallerType;
         private readonly ITypeSymbol _containingScope;
 
         public SafeHandleMarshallingInfoProvider(Compilation compilation, ITypeSymbol containingScope)
         {
             _compilation = compilation;
+            _safeHandleMarshallerType = compilation.GetBestTypeByMetadataName(TypeNames.System_Runtime_InteropServices_Marshalling_SafeHandleMarshaller_Metadata);
             _containingScope = containingScope;
         }
 
@@ -47,6 +51,7 @@ namespace Microsoft.Interop
 
         public MarshallingInfo GetMarshallingInfo(ITypeSymbol type, int indirectionDepth, UseSiteAttributeProvider useSiteAttributes, GetMarshallingInfoCallback marshallingInfoCallback)
         {
+            bool hasDefaultConstructor = false;
             bool hasAccessibleDefaultConstructor = false;
             if (type is INamedTypeSymbol named && !named.IsAbstract && named.InstanceConstructors.Length > 0)
             {
@@ -54,12 +59,45 @@ namespace Microsoft.Interop
                 {
                     if (ctor.Parameters.Length == 0)
                     {
+                        hasDefaultConstructor = ctor.DeclaredAccessibility == Accessibility.Public;
                         hasAccessibleDefaultConstructor = _compilation.IsSymbolAccessibleWithin(ctor, _containingScope);
                         break;
                     }
                 }
             }
-            return new SafeHandleMarshallingInfo(hasAccessibleDefaultConstructor, type.IsAbstract);
+
+            // If we don't have the SafeHandleMarshaller<T> type, then we'll use the built-in support in the generator.
+            // This support will be removed when dotnet/runtime doesn't build any packages for platforms below .NET 8
+            // as the downlevel support is dotnet/runtime specific.
+            if (_safeHandleMarshallerType is null)
+            {
+                return new SafeHandleMarshallingInfo(hasAccessibleDefaultConstructor, type.IsAbstract);
+            }
+
+            INamedTypeSymbol entryPointType = _safeHandleMarshallerType.Construct(type);
+            if (!ManualTypeMarshallingHelper.TryGetValueMarshallersFromEntryType(
+                entryPointType,
+                type,
+                _compilation,
+                out CustomTypeMarshallers? marshallers))
+            {
+                return NoMarshallingInfo.Instance;
+            }
+
+            // If the SafeHandle-derived type doesn't have a default constructor or is abstract,
+            // we only support managed-to-unmanaged marshalling
+            if (!hasDefaultConstructor || type.IsAbstract)
+            {
+                marshallers = marshallers.Value with
+                {
+                    Modes = ImmutableDictionary<MarshalMode, CustomTypeMarshallerData>.Empty
+                        .Add(
+                            MarshalMode.ManagedToUnmanagedIn,
+                            marshallers.Value.GetModeOrDefault(MarshalMode.ManagedToUnmanagedIn))
+                };
+            }
+
+            return new NativeMarshallingAttributeInfo(ManagedTypeInfo.CreateTypeInfoForTypeSymbol(entryPointType), marshallers.Value);
         }
     }
 }
index 4db27ba..43a458a 100644 (file)
@@ -136,5 +136,7 @@ namespace Microsoft.Interop
         public const string GeneratedComClassAttribute = "System.Runtime.InteropServices.Marshalling.GeneratedComClassAttribute";
         public const string ComExposedClassAttribute = "System.Runtime.InteropServices.Marshalling.ComExposedClassAttribute";
         public const string IComExposedClass = "System.Runtime.InteropServices.Marshalling.IComExposedClass";
+
+        public const string System_Runtime_InteropServices_Marshalling_SafeHandleMarshaller_Metadata = "System.Runtime.InteropServices.Marshalling.SafeHandleMarshaller`1";
     }
 }
index 1ae443c..2d9a2cb 100644 (file)
@@ -12,7 +12,7 @@ namespace LibraryImportGenerator.IntegrationTests
     {
         public partial class NativeExportsSafeHandle : SafeHandleZeroOrMinusOneIsInvalid
         {
-            private NativeExportsSafeHandle() : base(ownsHandle: true)
+            public NativeExportsSafeHandle() : base(ownsHandle: true)
             { }
 
             protected override bool ReleaseHandle()
index 613588f..4a948f5 100644 (file)
@@ -113,6 +113,12 @@ namespace LibraryImportGenerator.UnitTests
             // Abstract SafeHandle type by reference
             yield return new object[] { ID(), CodeSnippets.BasicParameterWithByRefModifier("ref", "System.Runtime.InteropServices.SafeHandle"), 1, 0 };
 
+            // SafeHandle array
+            yield return new object[] { ID(), CodeSnippets.MarshalAsArrayParametersAndModifiers("Microsoft.Win32.SafeHandles.SafeFileHandle"), 5, 0 };
+
+            // SafeHandle with private constructor by ref or out
+            yield return new object[] { ID(), CodeSnippets.SafeHandleWithCustomDefaultConstructorAccessibility(privateCtor: true), 3, 0 };
+
             // Collection with constant and element size parameter
             yield return new object[] { ID(), CodeSnippets.MarshalUsingCollectionWithConstantAndElementCount, 2, 0 };
 
index f92f137..9eb21ba 100644 (file)
@@ -174,7 +174,6 @@ namespace LibraryImportGenerator.UnitTests
             yield return new[] { ID(), CodeSnippets.BasicParametersAndModifiers("Microsoft.Win32.SafeHandles.SafeFileHandle") };
             yield return new[] { ID(), CodeSnippets.BasicParameterByValue("System.Runtime.InteropServices.SafeHandle") };
             yield return new[] { ID(), CodeSnippets.SafeHandleWithCustomDefaultConstructorAccessibility(privateCtor: false) };
-            yield return new[] { ID(), CodeSnippets.SafeHandleWithCustomDefaultConstructorAccessibility(privateCtor: true) };
 
             // Custom type marshalling
             CustomStructMarshallingCodeSnippets customStructMarshallingCodeSnippets = new(new CodeSnippets());
index d7a9301..5c85d0a 100644 (file)
@@ -13492,6 +13492,63 @@ namespace System.Runtime.InteropServices.Marshalling
             public static System.Span<TUnmanagedElement> GetUnmanagedValuesDestination(TUnmanagedElement* unmanaged, int numElements) { throw null; }
         }
     }
+
+    [System.Runtime.InteropServices.Marshalling.CustomMarshallerAttribute(typeof(System.Runtime.InteropServices.Marshalling.CustomMarshallerAttribute.GenericPlaceholder),
+    System.Runtime.InteropServices.Marshalling.MarshalMode.ManagedToUnmanagedIn,
+    typeof(System.Runtime.InteropServices.Marshalling.SafeHandleMarshaller<>.ManagedToUnmanagedIn))]
+    [System.Runtime.InteropServices.Marshalling.CustomMarshallerAttribute(typeof(System.Runtime.InteropServices.Marshalling.CustomMarshallerAttribute.GenericPlaceholder),
+    System.Runtime.InteropServices.Marshalling.MarshalMode.ManagedToUnmanagedRef,
+    typeof(System.Runtime.InteropServices.Marshalling.SafeHandleMarshaller<>.ManagedToUnmanagedRef))]
+    [System.Runtime.InteropServices.Marshalling.CustomMarshallerAttribute(typeof(System.Runtime.InteropServices.Marshalling.CustomMarshallerAttribute.GenericPlaceholder),
+    System.Runtime.InteropServices.Marshalling.MarshalMode.ManagedToUnmanagedOut,
+    typeof(System.Runtime.InteropServices.Marshalling.SafeHandleMarshaller<>.ManagedToUnmanagedOut))]
+    public static class SafeHandleMarshaller<[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] T> where T : SafeHandle
+    {
+        public struct ManagedToUnmanagedIn
+        {
+            private int _dummyPrimitive;
+            private T _handle;
+            public void FromManaged(T handle) { }
+
+            public nint ToUnmanaged() { throw null; }
+
+            public void Free() { }
+        }
+
+        public struct ManagedToUnmanagedRef
+        {
+            private int _dummyPrimitive;
+            private T _handle;
+
+            public ManagedToUnmanagedRef() { }
+
+            public void FromManaged(T handle) { }
+
+            public nint ToUnmanaged() { throw null; }
+
+            public void FromUnmanaged(nint value) { }
+
+            public void OnInvoked() { }
+
+            public T ToManagedFinally() { throw null; }
+
+            public void Free() { }
+        }
+
+        public struct ManagedToUnmanagedOut
+        {
+            private int _dummyPrimitive;
+            private T _newHandle;
+            public ManagedToUnmanagedOut() { }
+
+            public void FromUnmanaged(nint value) { }
+
+            public T ToManaged() { throw null; }
+
+            public void Free() { }
+        }
+    }
+
     [System.CLSCompliant(false)]
     [System.Runtime.InteropServices.Marshalling.CustomMarshallerAttribute(typeof(System.Span<>),
         System.Runtime.InteropServices.Marshalling.MarshalMode.Default,