From 9651ba8596dd904ca782ba204806697bb442e5d7 Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Wed, 16 Aug 2023 11:46:16 -0700 Subject: [PATCH 1/5] Implement test interfaces and marshallers and test them --- .../RcwAroundCcwTests.cs | 68 +++++ ...nmanagedToManagedCustomMarshallingTests.cs | 3 - .../ComInterfaces/IStatefulMarshalling.cs | 1 + .../IStatefulPinnedMarshalling.cs | 246 ++++++++++++++++-- ...tatelessCallerAllocateBufferMarshalling.cs | 121 +++++++-- 5 files changed, 402 insertions(+), 37 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs index 95ab9822d3d3d0..dc06f4cbc7dd7c 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs @@ -200,17 +200,33 @@ public void StatefulMarshalling() var obj = CreateWrapper(); var data = new StatefulType() { i = 42 }; + int preCallFreeCount = StatefulTypeMarshaller.FreeCount; obj.Method(data); Assert.Equal(42, data.i); + Assert.Equal(preCallFreeCount + 2, StatefulTypeMarshaller.FreeCount); + + preCallFreeCount = StatefulTypeMarshaller.FreeCount; obj.MethodIn(in data); Assert.Equal(42, data.i); + Assert.Equal(preCallFreeCount + 2, StatefulTypeMarshaller.FreeCount); + var oldData = data; + preCallFreeCount = StatefulTypeMarshaller.FreeCount; obj.MethodRef(ref data); + Assert.Equal(preCallFreeCount + 2, StatefulTypeMarshaller.FreeCount); Assert.True(oldData == data); // We want reference equality here + + preCallFreeCount = StatefulTypeMarshaller.FreeCount; obj.MethodOut(out data); + Assert.Equal(preCallFreeCount + 2, StatefulTypeMarshaller.FreeCount); Assert.Equal(1, data.i); + + preCallFreeCount = StatefulTypeMarshaller.FreeCount; Assert.Equal(1, obj.Return().i); + Assert.Equal(preCallFreeCount + 2, StatefulTypeMarshaller.FreeCount); + preCallFreeCount = StatefulTypeMarshaller.FreeCount; Assert.Equal(1, obj.ReturnPreserveSig().i); + Assert.Equal(preCallFreeCount + 2, StatefulTypeMarshaller.FreeCount); } [Fact] @@ -219,16 +235,68 @@ public void StatelessCallerAllocatedBufferMarshalling() var obj = CreateWrapper(); var data = new StatelessCallerAllocatedBufferType() { I = 42 }; + // ManagedToUnmanagedIn should use Caller Allocated Buffer and not allocate + StatelessCallerAllocatedBufferTypeMarshaller.DisableAllocations(); + + var numFreeCalls = StatelessCallerAllocatedBufferTypeMarshaller.FreeCount; obj.Method(data); + Assert.Equal(1 + numFreeCalls, StatelessCallerAllocatedBufferTypeMarshaller.FreeCount); Assert.Equal(42, data.I); + + numFreeCalls = StatelessCallerAllocatedBufferTypeMarshaller.FreeCount; obj.MethodIn(in data); + Assert.Equal(1 + numFreeCalls, StatelessCallerAllocatedBufferTypeMarshaller.FreeCount); Assert.Equal(42, data.I); + + // Other marshal modes will allocate + StatelessCallerAllocatedBufferTypeMarshaller.EnableAllocations(); + + numFreeCalls = StatelessCallerAllocatedBufferTypeMarshaller.FreeCount; obj.MethodRef(ref data); + Assert.Equal(2 + numFreeCalls, StatelessCallerAllocatedBufferTypeMarshaller.FreeCount); + StatelessCallerAllocatedBufferTypeMarshaller.AssertAllPointersFreed(); Assert.Equal(200, data.I); + + numFreeCalls = StatelessCallerAllocatedBufferTypeMarshaller.FreeCount; obj.MethodOut(out data); + Assert.Equal(1 + numFreeCalls, StatelessCallerAllocatedBufferTypeMarshaller.FreeCount); + StatelessCallerAllocatedBufferTypeMarshaller.AssertAllPointersFreed(); Assert.Equal(20, data.I); + + numFreeCalls = StatelessCallerAllocatedBufferTypeMarshaller.FreeCount; Assert.Equal(201, obj.Return().I); + Assert.Equal(1 + numFreeCalls, StatelessCallerAllocatedBufferTypeMarshaller.FreeCount); + + numFreeCalls = StatelessCallerAllocatedBufferTypeMarshaller.FreeCount; Assert.Equal(202, obj.ReturnPreserveSig().I); + Assert.Equal(1 + numFreeCalls, StatelessCallerAllocatedBufferTypeMarshaller.FreeCount); + } + + [Fact] + public void StatefulPinnedMarshalling() + { + var obj = CreateWrapper(); + var data = new StatefulPinnedType() { I = 4 }; + + // In parameters should always use pinning + //StatefulPinnedTypeMarshaller.ManagedToUnmanagedIn.DisableNonPinnedPath(); + + obj.Method(data); + Assert.Equal(4, data.I); + + obj.MethodIn(in data); + Assert.Equal(4, data.I); + + //StatefulPinnedTypeMarshaller.ManagedToUnmanagedIn.EnableNonPinnedPath(); + + obj.MethodOut(out data); + Assert.Equal(102, data.I); + + obj.MethodRef(ref data); + Assert.Equal(103, data.I); + + data = obj.Return(); + Assert.Equal(104, data.I); } [Fact] diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/UnmanagedToManagedCustomMarshallingTests.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/UnmanagedToManagedCustomMarshallingTests.cs index c88031d5c4ca68..8defdfd8c4e331 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/UnmanagedToManagedCustomMarshallingTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/UnmanagedToManagedCustomMarshallingTests.cs @@ -2,14 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Generic; using System.Linq; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.InteropServices.Marshalling; -using System.Text; using System.Threading; -using System.Threading.Tasks; using SharedTypes; using Xunit; using static ComInterfaceGenerator.Tests.UnmanagedToManagedCustomMarshallingTests; diff --git a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulMarshalling.cs b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulMarshalling.cs index 69f7e09a902d3d..47b6cc916c3384 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulMarshalling.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulMarshalling.cs @@ -50,6 +50,7 @@ internal struct StatefulNative [CustomMarshaller(typeof(StatefulType), MarshalMode.ManagedToUnmanagedRef, typeof(Bidirectional))] internal struct StatefulTypeMarshaller { + public static int FreeCount => Bidirectional.FreeCount + ManagedToUnmanaged.FreeCount + UnmanagedToManaged.FreeCount; internal struct Bidirectional { public static int FreeCount { get; private set; } diff --git a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulPinnedMarshalling.cs b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulPinnedMarshalling.cs index 1b5438cbdd6f25..b522973698f137 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulPinnedMarshalling.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulPinnedMarshalling.cs @@ -16,46 +16,258 @@ internal partial interface IStatefulPinnedMarshalling void MethodOut(out StatefulPinnedType param); void MethodRef(ref StatefulPinnedType param); StatefulPinnedType Return(); - [PreserveSig] - StatefulPinnedType ReturnPreserveSig(); + } + + [GeneratedComClass] + internal partial class StatefulPinnedMarshalling : IStatefulPinnedMarshalling + { + public void Method(StatefulPinnedType param) { param.I = 100; } + public void MethodIn(in StatefulPinnedType param) { param.I = 101; } + public void MethodOut(out StatefulPinnedType param) => param = new StatefulPinnedType() { I = 102 }; + public void MethodRef(ref StatefulPinnedType param) { param = new StatefulPinnedType() { I = 103 }; } + public StatefulPinnedType Return() => new StatefulPinnedType() { I = 104 }; } [NativeMarshalling(typeof(StatefulPinnedTypeMarshaller))] internal class StatefulPinnedType { + public int I; } - [CustomMarshaller(typeof(StatefulPinnedType), MarshalMode.Default, typeof(StatefulPinnedTypeMarshaller))] - internal struct StatefulPinnedTypeMarshaller + internal unsafe struct StatefulPinnedNative { + public int I; + } - public static int BufferSize => 64; - public ref int GetPinnableReference() => throw new System.NotImplementedException(); - public void FromManaged(StatefulPinnedType managed, Span buffer) + [CustomMarshaller(typeof(StatefulPinnedType), MarshalMode.ManagedToUnmanagedIn, typeof(ManagedToUnmanagedIn))] + [CustomMarshaller(typeof(StatefulPinnedType), MarshalMode.UnmanagedToManagedOut, typeof(UnmanagedToManagedOut))] + [CustomMarshaller(typeof(StatefulPinnedType), MarshalMode.UnmanagedToManagedIn, typeof(UnmanagedToManagedIn))] + [CustomMarshaller(typeof(StatefulPinnedType), MarshalMode.ManagedToUnmanagedOut, typeof(ManagedToUnmanagedOut))] + [CustomMarshaller(typeof(StatefulPinnedType), MarshalMode.ManagedToUnmanagedRef, typeof(ManagedToUnmanagedRef))] + [CustomMarshaller(typeof(StatefulPinnedType), MarshalMode.UnmanagedToManagedRef, typeof(UnmanagedToManagedRef))] + internal unsafe static class StatefulPinnedTypeMarshaller + { + public ref struct ManagedToUnmanagedIn { - throw new System.NotImplementedException(); + static bool s_mustPin; + public static void DisableNonPinnedPath() => s_mustPin = true; + public static void EnableNonPinnedPath() => s_mustPin = false; + + StatefulPinnedType _managed; + bool _hasManaged; + Span buffer; + nint _ptr; + bool _canFree; + + public void FromManaged(StatefulPinnedType managed) + { + _hasManaged = true; + _managed = managed; + buffer = new byte[sizeof(StatefulPinnedNative)]; + } + + public ref StatefulPinnedNative GetPinnableReference() + { + if (!_hasManaged) + throw new InvalidOperationException(); + StatefulPinnedNative native = new() { I = _managed.I }; + MemoryMarshal.Write(buffer, in native); + _canFree = true; + return ref MemoryMarshal.AsRef(buffer); + } + + public StatefulPinnedNative* ToUnmanaged() + { + if (s_mustPin) + throw new InvalidOperationException("Expected to pin, but is instead converting with default ToUnmanaged."); + if (!_hasManaged) + throw new InvalidOperationException(); + _ptr = Marshal.AllocHGlobal(sizeof(StatefulPinnedNative)); + _canFree = true; + *(StatefulPinnedNative*)_ptr = new StatefulPinnedNative() { I = _managed.I }; + return (StatefulPinnedNative*)_ptr; + } + + public void Free() + { + if (!_canFree) throw new InvalidOperationException(); + if (_ptr != 0) + Marshal.FreeHGlobal(_ptr); + } } - public nint ToUnmanaged() + public struct ManagedToUnmanagedOut { - throw new System.NotImplementedException(); + StatefulPinnedNative* _unmanaged; + bool _hasUnmanaged; + + public void FromUnmanaged(StatefulPinnedNative* unmanaged) + { + _unmanaged = unmanaged; + _hasUnmanaged = true; + } + + public StatefulPinnedType ToManaged() + { + if (!_hasUnmanaged) + throw new InvalidOperationException(); + return new StatefulPinnedType() { I = _unmanaged->I }; + } + + public void Free() + { + if (!_hasUnmanaged) + throw new InvalidOperationException(); + var ptr = (nint)_unmanaged; + if (ptr != 0) + Marshal.FreeHGlobal(ptr); + } } - public void FromUnmanaged(nint unmanaged) + public struct UnmanagedToManagedIn { - throw new System.NotImplementedException(); + StatefulPinnedNative* _unmanaged; + bool _hasUnmanaged; + public void FromUnmanaged(StatefulPinnedNative* unmanaged) + { + _unmanaged = unmanaged; + _hasUnmanaged = true; + } + + public StatefulPinnedType ToManaged() + { + if (!_hasUnmanaged) + throw new InvalidOperationException(); + return new StatefulPinnedType() { I = _unmanaged->I }; + } + + public void Free() + { + } } - public StatefulPinnedType ToManaged() + public struct UnmanagedToManagedOut { - throw new System.NotImplementedException(); + StatefulPinnedType _managed; + bool _hasManaged; + nint _ptr; + + public void FromManaged(StatefulPinnedType managed) + { + _hasManaged = true; + _managed = managed; + } + + public StatefulPinnedNative* ToUnmanaged() + { + if (!_hasManaged) + throw new InvalidOperationException(); + _ptr = Marshal.AllocHGlobal(sizeof(StatefulPinnedNative)); + *(StatefulPinnedNative*)_ptr = new StatefulPinnedNative() { I = _managed.I }; + return (StatefulPinnedNative*)_ptr; + } + + public void Free() + { + } } - public void Free() + + public struct ManagedToUnmanagedRef { - throw new System.NotImplementedException(); + StatefulPinnedNative* _unmanaged; + bool _hasUnmanaged; + public void FromUnmanaged(StatefulPinnedNative* unmanaged) + { + _unmanaged = unmanaged; + _hasUnmanaged = true; + } + + public StatefulPinnedType ToManaged() + { + if (!_hasUnmanaged) + throw new InvalidOperationException(); + return new StatefulPinnedType() { I = _unmanaged->I }; + } + + StatefulPinnedType _managed; + bool _hasManaged; + nint _ptr; + bool _hasAllocated; + + public void FromManaged(StatefulPinnedType managed) + { + _hasManaged = true; + _managed = managed; + } + + public StatefulPinnedNative* ToUnmanaged() + { + if (!_hasManaged) + throw new InvalidOperationException(); + _ptr = Marshal.AllocHGlobal(sizeof(StatefulPinnedNative)); + _hasAllocated = true; + *(StatefulPinnedNative*)_ptr = new StatefulPinnedNative(); + return (StatefulPinnedNative*)_ptr; + } + + public void Free() + { + if (_hasUnmanaged) + { + Marshal.FreeHGlobal((nint)_unmanaged); + } + else if (_hasAllocated) + { + Marshal.FreeHGlobal(_ptr); + } + } } + public struct UnmanagedToManagedRef + { + StatefulPinnedNative* _unmanaged; + bool _hasUnmanaged; + public void FromUnmanaged(StatefulPinnedNative* unmanaged) + { + _unmanaged = unmanaged; + _hasUnmanaged = true; + } + + public StatefulPinnedType ToManaged() + { + if (!_hasUnmanaged) + throw new InvalidOperationException(); + return new StatefulPinnedType() { I = _unmanaged->I }; + } + + StatefulPinnedType _managed; + bool _hasManaged; + nint _ptr; + bool _hasAllocated; - public void OnInvoked() { } + public void FromManaged(StatefulPinnedType managed) + { + _hasManaged = true; + _managed = managed; + } + + public StatefulPinnedNative* ToUnmanaged() + { + if (!_hasManaged) + throw new InvalidOperationException(); + _ptr = Marshal.AllocHGlobal(sizeof(StatefulPinnedNative)); + _hasAllocated = true; + *(StatefulPinnedNative*)_ptr = new StatefulPinnedNative(); + return (StatefulPinnedNative*)_ptr; + } + + public void Free() + { + if (_hasAllocated && _hasUnmanaged) + { + Marshal.FreeHGlobal((nint)_unmanaged); + } + } + } } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatelessCallerAllocateBufferMarshalling.cs b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatelessCallerAllocateBufferMarshalling.cs index 1b04704251bff1..f8b1174799b136 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatelessCallerAllocateBufferMarshalling.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatelessCallerAllocateBufferMarshalling.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.InteropServices.Marshalling; @@ -39,39 +40,125 @@ internal class StatelessCallerAllocatedBufferType public int I; } - [CustomMarshaller(typeof(StatelessCallerAllocatedBufferType), MarshalMode.Default, typeof(StatelessCallerAllocatedBufferTypeMarshaller))] + internal struct StatelessCallerAllocatedBufferNative + { + public int I; + } + + [CustomMarshaller(typeof(StatelessCallerAllocatedBufferType), MarshalMode.ManagedToUnmanagedRef, typeof(Bidirectional))] + [CustomMarshaller(typeof(StatelessCallerAllocatedBufferType), MarshalMode.UnmanagedToManagedRef, typeof(Bidirectional))] + [CustomMarshaller(typeof(StatelessCallerAllocatedBufferType), MarshalMode.ElementIn, typeof(Bidirectional))] + [CustomMarshaller(typeof(StatelessCallerAllocatedBufferType), MarshalMode.ElementOut, typeof(Bidirectional))] + [CustomMarshaller(typeof(StatelessCallerAllocatedBufferType), MarshalMode.ElementRef, typeof(Bidirectional))] + [CustomMarshaller(typeof(StatelessCallerAllocatedBufferType), MarshalMode.ManagedToUnmanagedOut, typeof(UnmanagedToManaged))] + [CustomMarshaller(typeof(StatelessCallerAllocatedBufferType), MarshalMode.UnmanagedToManagedIn, typeof(UnmanagedToManaged))] + [CustomMarshaller(typeof(StatelessCallerAllocatedBufferType), MarshalMode.ManagedToUnmanagedIn, typeof(ManagedToUnmanagedIn))] + [CustomMarshaller(typeof(StatelessCallerAllocatedBufferType), MarshalMode.UnmanagedToManagedOut, typeof(UnmanagedToManagedOut))] internal static unsafe class StatelessCallerAllocatedBufferTypeMarshaller { + static bool _canAllocate = true; + public static void DisableAllocations() => _canAllocate = false; + public static void EnableAllocations() => _canAllocate = true; + public static void AssertAllPointersFreed() + { + if (_ptrs.Any()) throw new InvalidOperationException(); + } + static HashSet _ptrs = new(); + public static int FreeCount { get; private set; } - public static int BufferSize => sizeof(int); - public static int* ConvertToUnmanaged(StatelessCallerAllocatedBufferType managed, Span buffer) + + public static class UnmanagedToManaged { - buffer[0] = managed.I; - return (int*)Unsafe.AsPointer(ref buffer[0]); + public static StatelessCallerAllocatedBufferType ConvertToManaged(StatelessCallerAllocatedBufferNative* unmanaged) + { + return new StatelessCallerAllocatedBufferType() { I = unmanaged->I }; + } + + public static void Free(StatelessCallerAllocatedBufferNative* unmanaged) + { + FreeCount++; + if (_ptrs.Contains((nint)unmanaged)) + { + Marshal.FreeHGlobal((nint)unmanaged); + _ptrs.Remove((nint)unmanaged); + } + } } - public static StatelessCallerAllocatedBufferType ConvertToManaged(int* unmanaged) + public static class ManagedToUnmanagedIn { - return new StatelessCallerAllocatedBufferType() { I = *unmanaged }; + public static int BufferSize => sizeof(StatelessCallerAllocatedBufferNative); + + public static StatelessCallerAllocatedBufferNative* ConvertToUnmanaged(StatelessCallerAllocatedBufferType managed, Span buffer) + { + var unmanaged = new StatelessCallerAllocatedBufferNative() { I = managed.I }; + MemoryMarshal.Write(buffer, in unmanaged); + return (StatelessCallerAllocatedBufferNative*)Unsafe.AsPointer(ref MemoryMarshal.AsRef(buffer)); + } + + public static void Free(StatelessCallerAllocatedBufferNative* unmanaged) + { + FreeCount++; + if (_ptrs.Contains((nint)unmanaged)) + { + Marshal.FreeHGlobal((nint)unmanaged); + _ptrs.Remove((nint)unmanaged); + } + } } - public static void Free(int* unmanaged) + public static class UnmanagedToManagedOut { - FreeCount++; - if (_ptrs.Contains((nint)unmanaged)) + public static StatelessCallerAllocatedBufferNative* ConvertToUnmanaged(StatelessCallerAllocatedBufferType managed) { - Marshal.FreeHGlobal((nint)unmanaged); - _ptrs.Remove((nint)unmanaged); + if (!_canAllocate) + throw new InvalidOperationException("Marshalling used default ConverToUnmanaged when CallerAllocatedBuffer was expected"); + nint ptr = Marshal.AllocHGlobal(sizeof(StatelessCallerAllocatedBufferNative)); + _ptrs.Add(ptr); + var structPtr = (StatelessCallerAllocatedBufferNative*)ptr; + structPtr->I = managed.I; + return structPtr; + } + + public static void Free(StatelessCallerAllocatedBufferNative* unmanaged) + { + FreeCount++; + if (_ptrs.Contains((nint)unmanaged)) + { + Marshal.FreeHGlobal((nint)unmanaged); + _ptrs.Remove((nint)unmanaged); + } } } - public static int* ConvertToUnmanaged(StatelessCallerAllocatedBufferType managed) + public static class Bidirectional { - nint ptr = Marshal.AllocHGlobal(sizeof(int)); - _ptrs.Add(ptr); - *(int*)ptr = managed.I; - return (int*)ptr; + public static StatelessCallerAllocatedBufferNative* ConvertToUnmanaged(StatelessCallerAllocatedBufferType managed) + { + if (!_canAllocate) + throw new InvalidOperationException("Marshalling used default ConverToUnmanaged when CallerAllocatedBuffer was expected"); + nint ptr = Marshal.AllocHGlobal(sizeof(StatelessCallerAllocatedBufferNative)); + _ptrs.Add(ptr); + var structPtr = (StatelessCallerAllocatedBufferNative*)ptr; + structPtr->I = managed.I; + return structPtr; + } + + public static StatelessCallerAllocatedBufferType ConvertToManaged(StatelessCallerAllocatedBufferNative* unmanaged) + { + return new StatelessCallerAllocatedBufferType() { I = unmanaged->I }; + } + + public static void Free(StatelessCallerAllocatedBufferNative* unmanaged) + { + FreeCount++; + if (_ptrs.Contains((nint)unmanaged)) + { + Marshal.FreeHGlobal((nint)unmanaged); + _ptrs.Remove((nint)unmanaged); + } + } } } } From f89fec09b11c47552762f46efa70dae9154f8fa8 Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Wed, 16 Aug 2023 11:51:34 -0700 Subject: [PATCH 2/5] Add skipped test for enforcing pinning path --- .../RcwAroundCcwTests.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs index dc06f4cbc7dd7c..34651a9424f0f8 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs @@ -272,6 +272,24 @@ public void StatelessCallerAllocatedBufferMarshalling() Assert.Equal(1 + numFreeCalls, StatelessCallerAllocatedBufferTypeMarshaller.FreeCount); } + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/90621")] + public void StatefulPinnedMarshalling_EnforcePinning() + { + var obj = CreateWrapper(); + var data = new StatefulPinnedType() { I = 4 }; + + StatefulPinnedTypeMarshaller.ManagedToUnmanagedIn.DisableNonPinnedPath(); + + obj.Method(data); + Assert.Equal(4, data.I); + + obj.MethodIn(in data); + Assert.Equal(4, data.I); + + StatefulPinnedTypeMarshaller.ManagedToUnmanagedIn.EnableNonPinnedPath(); + } + [Fact] public void StatefulPinnedMarshalling() { @@ -279,6 +297,7 @@ public void StatefulPinnedMarshalling() var data = new StatefulPinnedType() { I = 4 }; // In parameters should always use pinning + // https://github.com/dotnet/runtime/issues/90621 //StatefulPinnedTypeMarshaller.ManagedToUnmanagedIn.DisableNonPinnedPath(); obj.Method(data); From e7ee9997247e3b0c6ae0cfc306b0a1a40a44ad05 Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Wed, 16 Aug 2023 16:21:53 -0700 Subject: [PATCH 3/5] Update RcwAroundCcwTests.cs --- .../ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs index 34651a9424f0f8..b02517afccb6e0 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs @@ -296,18 +296,6 @@ public void StatefulPinnedMarshalling() var obj = CreateWrapper(); var data = new StatefulPinnedType() { I = 4 }; - // In parameters should always use pinning - // https://github.com/dotnet/runtime/issues/90621 - //StatefulPinnedTypeMarshaller.ManagedToUnmanagedIn.DisableNonPinnedPath(); - - obj.Method(data); - Assert.Equal(4, data.I); - - obj.MethodIn(in data); - Assert.Equal(4, data.I); - - //StatefulPinnedTypeMarshaller.ManagedToUnmanagedIn.EnableNonPinnedPath(); - obj.MethodOut(out data); Assert.Equal(102, data.I); From 36c7ad0e62c2ae5bfe2162be69f011872ed7087b Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Thu, 17 Aug 2023 09:50:11 -0700 Subject: [PATCH 4/5] Fix marshaller to actually copy value --- .../tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs | 2 +- .../SharedTypes/ComInterfaces/IStatefulPinnedMarshalling.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs index b02517afccb6e0..7f8ddc7fa6de1a 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs @@ -294,7 +294,7 @@ public void StatefulPinnedMarshalling_EnforcePinning() public void StatefulPinnedMarshalling() { var obj = CreateWrapper(); - var data = new StatefulPinnedType() { I = 4 }; + StatefulPinnedType data; obj.MethodOut(out data); Assert.Equal(102, data.I); diff --git a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulPinnedMarshalling.cs b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulPinnedMarshalling.cs index b522973698f137..4092cba92b7887 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulPinnedMarshalling.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulPinnedMarshalling.cs @@ -207,7 +207,7 @@ public void FromManaged(StatefulPinnedType managed) throw new InvalidOperationException(); _ptr = Marshal.AllocHGlobal(sizeof(StatefulPinnedNative)); _hasAllocated = true; - *(StatefulPinnedNative*)_ptr = new StatefulPinnedNative(); + *(StatefulPinnedNative*)_ptr = new StatefulPinnedNative() { I = _managed.I }; return (StatefulPinnedNative*)_ptr; } @@ -257,7 +257,7 @@ public void FromManaged(StatefulPinnedType managed) throw new InvalidOperationException(); _ptr = Marshal.AllocHGlobal(sizeof(StatefulPinnedNative)); _hasAllocated = true; - *(StatefulPinnedNative*)_ptr = new StatefulPinnedNative(); + *(StatefulPinnedNative*)_ptr = new StatefulPinnedNative() { I = _managed.I }; return (StatefulPinnedNative*)_ptr; } From 62ca9347fff241bae97b2878e0bd989d80de16e5 Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Thu, 17 Aug 2023 10:12:39 -0700 Subject: [PATCH 5/5] Fix pinned stateful marshaller to actually pin properly and use the pinned object --- .../RcwAroundCcwTests.cs | 10 +----- .../IStatefulPinnedMarshalling.cs | 34 +++++++++++++------ 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs index 7f8ddc7fa6de1a..765c26ffac59c5 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs @@ -273,8 +273,7 @@ public void StatelessCallerAllocatedBufferMarshalling() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/90621")] - public void StatefulPinnedMarshalling_EnforcePinning() + public void StatefulPinnedMarshalling() { var obj = CreateWrapper(); var data = new StatefulPinnedType() { I = 4 }; @@ -288,13 +287,6 @@ public void StatefulPinnedMarshalling_EnforcePinning() Assert.Equal(4, data.I); StatefulPinnedTypeMarshaller.ManagedToUnmanagedIn.EnableNonPinnedPath(); - } - - [Fact] - public void StatefulPinnedMarshalling() - { - var obj = CreateWrapper(); - StatefulPinnedType data; obj.MethodOut(out data); Assert.Equal(102, data.I); diff --git a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulPinnedMarshalling.cs b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulPinnedMarshalling.cs index 4092cba92b7887..eaac2608d8184a 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulPinnedMarshalling.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatefulPinnedMarshalling.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.InteropServices.Marshalling; @@ -58,41 +59,54 @@ public ref struct ManagedToUnmanagedIn Span buffer; nint _ptr; bool _canFree; + bool _isPinned; + ref StatefulPinnedNative _refNativeStruct; public void FromManaged(StatefulPinnedType managed) { _hasManaged = true; _managed = managed; - buffer = new byte[sizeof(StatefulPinnedNative)]; } public ref StatefulPinnedNative GetPinnableReference() { if (!_hasManaged) throw new InvalidOperationException(); - StatefulPinnedNative native = new() { I = _managed.I }; - MemoryMarshal.Write(buffer, in native); - _canFree = true; - return ref MemoryMarshal.AsRef(buffer); + buffer = new byte[sizeof(StatefulPinnedNative)]; + _isPinned = true; + _refNativeStruct = ref MemoryMarshal.AsRef(buffer); + return ref _refNativeStruct; } public StatefulPinnedNative* ToUnmanaged() { - if (s_mustPin) - throw new InvalidOperationException("Expected to pin, but is instead converting with default ToUnmanaged."); if (!_hasManaged) throw new InvalidOperationException(); - _ptr = Marshal.AllocHGlobal(sizeof(StatefulPinnedNative)); + _canFree = true; + if (_isPinned) + { + _refNativeStruct = new StatefulPinnedNative() { I = _managed.I }; + return (StatefulPinnedNative*)Unsafe.AsPointer(ref _refNativeStruct); + } + + if (s_mustPin) + throw new InvalidOperationException("Expected to pin, but is instead converting with default ToUnmanaged."); + + _ptr = Marshal.AllocHGlobal(sizeof(StatefulPinnedNative)); *(StatefulPinnedNative*)_ptr = new StatefulPinnedNative() { I = _managed.I }; return (StatefulPinnedNative*)_ptr; } public void Free() { - if (!_canFree) throw new InvalidOperationException(); - if (_ptr != 0) + if (!_canFree) + throw new InvalidOperationException(); + + if (!_isPinned && _ptr != 0) + { Marshal.FreeHGlobal(_ptr); + } } }