The marshalling generators always use the length of the managed collection if it is only marshalled to unmanaged (and not unmarshalled to managed). However, during cleanup, they default to using 0 as the length when cleaning up an inner array of a 2D array. This causes the marshallers to not free any elements of the inner array on successful calls
Jagged arrays are technically an unsupported scenario, but this can cause a memory leak on successful calls to COM methods or LibraryImport methods. This issue is also present in .Net 7.
The fix for this would be to either require length parameters for parameters that are marshalled with marshallers that have a Free method (and use them -- we won't use them if they're provided now), or set length to __arrayMarshaller.GetManagedValuesSource().Length in these specific scenarios.
@jkoritzinsky @AaronRobinsonMSFT Should we try to get a fix in 8? I think it's probably okay since the behavior has been here since 7 with no issues, but what are your thoughts?
Source:
using System.Runtime.InteropServices;
using System.Runtime.InteropServices.Marshalling;
public static partial class PInvoke
{
[LibraryImport("asdfasdf")]
public static partial int Method(
[MarshalUsing(typeof(MyClassMarshaller), ElementIndirectionDepth = 2)]
[MarshalUsing(CountElementName = nameof(length))]
[MarshalUsing(CountElementName = nameof(widths), ElementIndirectionDepth =1)]
MyClass[][] arg,
int length,
int[] widths);
public class MyClass
{
}
[CustomMarshaller(typeof(MyClass), MarshalMode.Default, typeof(MyClassMarshaller))]
public static class MyClassMarshaller
{
public static nint ConvertToUnmanaged(MyClass myClass) => throw null;
public static MyClass ConverToManaged(nint nativeValue) => throw null;
public static void Free(nint unmaanged) => throw null;
}
}Generated:
public static partial int Method(global::PInvoke.MyClass[][] arg, int length, int[] widths)
{
...
finally {
...
// CleanupCallerAllocated - Perform cleanup of caller allocated resources.
{
global::System.ReadOnlySpan<global::System.IntPtr> __arg_native__nativeSpan = __arg_native__marshaller.GetUnmanagedValuesDestination();
for (int __i0 = 0; __i0 < __arg_native__lastIndexMarshalled; ++__i0)
{
int __arg_native__nativeSpan____i0__numElements;
global::System.Runtime.CompilerServices.Unsafe.SkipInit(out __arg_native__nativeSpan____i0__numElements);
__arg_native__nativeSpan____i0__numElements = 0; // Issue
{
global::System.ReadOnlySpan<nint> __arg_native__nativeSpan____i0__nativeSpan = global::System.Runtime.InteropServices.Marshalling.ArrayMarshaller<global::PInvoke.MyClass, nint>.GetUnmanagedValuesDestination((nint*)__arg_native__nativeSpan[__i0], __arg_native__nativeSpan____i0__numElements);
for (int __i1 = 0; __i1 < __arg_native__nativeSpan____i0__nativeSpan.Length; ++__i1)
{
// Will never execute
global::PInvoke.MyClassMarshaller.Free(__arg_native__nativeSpan____i0__nativeSpan[__i1]);
}
}
__arg_native__nativeSpan____i0__numElements = 0;
global::System.Runtime.InteropServices.Marshalling.ArrayMarshaller<global::PInvoke.MyClass, nint>.Free((nint*)__arg_native__nativeSpan[__i0]);
}
}
__arg_native__marshaller.Free();
}
The marshalling generators always use the length of the managed collection if it is only marshalled to unmanaged (and not unmarshalled to managed). However, during cleanup, they default to using 0 as the length when cleaning up an inner array of a 2D array. This causes the marshallers to not free any elements of the inner array on successful calls
Jagged arrays are technically an unsupported scenario, but this can cause a memory leak on successful calls to COM methods or LibraryImport methods. This issue is also present in .Net 7.
The fix for this would be to either require length parameters for parameters that are marshalled with marshallers that have a
Freemethod (and use them -- we won't use them if they're provided now), or set length to__arrayMarshaller.GetManagedValuesSource().Lengthin these specific scenarios.@jkoritzinsky @AaronRobinsonMSFT Should we try to get a fix in 8? I think it's probably okay since the behavior has been here since 7 with no issues, but what are your thoughts?
Source:
Generated: