Marshalling a collection of elements that require two-stage marshalling does not preserve the marshaller state?

[f2bo]

I'm creating a marshaller for a structure that represents a scatter-gather list. It contains a linked list of entries, each with a ReadOnlyMemory<byte> member and a reference to the next element in the list, much like ReadOnlySequence<T>.

However, to simplify the code and explanation, I'll replace it with a linear collection of structures that have a single ReadOnlyMemory<T> property.

[NativeMarshalling(typeof(EntryMarshaller))]
public struct Entry
{
    public ReadOnlyMemory<byte> Data { get; set; }
}

The native representation for the structure above is the following.

public unsafe struct NativeEntry
{
    public void* data;
    public int length;
}

I defined a custom marshaller to marshal each entry in the collection. The memory requires pinning, so it's implemented as a two-stage marshaller that holds a MemoryHandle field for the Data property. The handle is disposed in the FreeNative method.

[CustomTypeMarshaller(typeof(Entry),
    Direction = CustomTypeMarshallerDirection.In,
    BufferSize = 128,
    Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.TwoStageMarshalling)]
public unsafe struct EntryMarshaller
{
    private NativeEntry _value;
    private MemoryHandle _handle;

    public EntryMarshaller(Entry managed)
    {
        _handle = managed.Data.Pin();
        _value = new NativeEntry
        {
            data = _handle.Pointer,
            length = managed.Data.Length
        };
    }

    public NativeEntry ToNativeValue() => _value;

    public void FromNativeValue(NativeEntry value) => _value = value;

    public void FreeNative() =>_handle.Dispose();
}

Additionally, there's a marshaller for the collection itself.

[CustomTypeMarshaller(typeof(ReadOnlySpan<Entry>),
    CustomTypeMarshallerKind.LinearCollection,
    Direction = CustomTypeMarshallerDirection.In,
    Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.CallerAllocatedBuffer | CustomTypeMarshallerFeatures.TwoStageMarshalling,
    BufferSize = 128)]
public unsafe ref struct EntryCollectionMarshaller
{
    private ReadOnlySpan<Entry> _managed;
    private IntPtr _nativeMemory;
    private Span<byte> _buffer;

    public EntryCollectionMarshaller(ReadOnlySpan<Entry> managed, int nativeElementSize)
        : this(managed, default, nativeElementSize)
    {
    }

    public EntryCollectionMarshaller(ReadOnlySpan<Entry> managed, Span<byte> buffer, int nativeElementSize)
    {
        _managed = managed;

        int requiredBufferSize = managed.Length * nativeElementSize;
        if (requiredBufferSize > buffer.Length)
        {
            _nativeMemory = Marshal.AllocCoTaskMem(requiredBufferSize);
            _buffer = new Span<byte>((void*)_nativeMemory, requiredBufferSize);
        }
        else
        {
            _nativeMemory = IntPtr.Zero;
            _buffer = buffer[0..requiredBufferSize];
        }
    }

    public ReadOnlySpan<Entry> GetManagedValuesSource() => _managed;

    public Span<byte> GetNativeValuesDestination() => _buffer;

    public ref NativeEntry GetPinnableReference() => ref MemoryMarshal.AsRef<NativeEntry>(_buffer);

    public NativeEntry* ToNativeValue() => (NativeEntry*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(_buffer));

    public void FreeNative() => Marshal.FreeCoTaskMem(_nativeMemory);
}

Lastly, I define a LibraryImport method with a ReadOnlySpan<Entry> parameter.

[LibraryImport(NativeLibraryName, EntryPoint = "use_entries")]
public static partial void UseEntries([MarshalUsing(typeof(EntryCollectionMarshaller))] ReadOnlySpan<Entry> entries, int length);

So far, so good. The generated stub marshals the entries parameter correctly. However, an issue occurs during the cleanup stage, when the memory handles need to be disposed. The snippet below is the section of code that is generated.

The problem is that it creates a new instance of the EntryMarshaller for each element in the collection and initializes it using FromNativeValue passing in the corresponding entry read from the native buffer, thus losing any state (i.e. the memory handle) from the marshaller that was previously used in the marshalling phase.

//
// Cleanup
//
{
    System.Span<global::SharedTypes.NativeEntry> __entries_gen_native__marshaller__nativeSpan = System.Runtime.InteropServices.MemoryMarshal.Cast<byte, global::SharedTypes.NativeEntry>(__entries_gen_native__marshaller.GetNativeValuesDestination());
    for (int __i0 = 0; __i0 < __entries_gen_native__marshaller__nativeSpan.Length; ++__i0)
    {
        global::SharedTypes.EntryMarshaller __entries_gen_native__marshaller__nativeSpan____i0__marshaller = default;
        __entries_gen_native__marshaller__nativeSpan____i0__marshaller.FromNativeValue(__entries_gen_native__marshaller__nativeSpan[__i0]);
        __entries_gen_native__marshaller__nativeSpan____i0__marshaller.FreeNative();
    }
}

__entries_gen_native__marshaller.FreeNative();

I'm not sure whether I'm doing this correctly, probably not, but I don't see how to release the native resources unless the original marshallers are preserved.

[jkotas]

FromNativeValue is meant to initialize the whole marshaller state that totally not obvious from the name.

My proposal would be to rename it to InitializeFromNativeValue to make it more obvious that it is initializing the whole marshaller.

Also see #69031 that is proposing to introduce a symmetric method for explicit initialization from managed value.

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I'm creating a marshaller for a structure that represents a scatter-gather list. It contains a linked list of entries, each with a ReadOnlyMemory<byte> member and a reference to the next element in the list, much like ReadOnlySequence<T>.

However, to simplify the code and explanation, I'll replace it with a linear collection of structures that have a single ReadOnlyMemory<T> property.

[NativeMarshalling(typeof(EntryMarshaller))]
public struct Entry
{
    public ReadOnlyMemory<byte> Data { get; set; }
}

The native representation for the structure above is the following.

public unsafe struct NativeEntry
{
    public void* data;
    public int length;
}

I defined a custom marshaller to marshal each entry in the collection. The memory requires pinning, so it's implemented as a two-stage marshaller that holds a MemoryHandle field for the Data property. The handle is disposed in the FreeNative method.

[CustomTypeMarshaller(typeof(Entry),
    Direction = CustomTypeMarshallerDirection.In,
    BufferSize = 128,
    Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.TwoStageMarshalling)]
public unsafe struct EntryMarshaller
{
    private NativeEntry _value;
    private MemoryHandle _handle;

    public EntryMarshaller(Entry managed)
    {
        _handle = managed.Data.Pin();
        _value = new NativeEntry
        {
            data = _handle.Pointer,
            length = managed.Data.Length
        };
    }

    public NativeEntry ToNativeValue() => _value;

    public void FromNativeValue(NativeEntry value) => _value = value;

    public void FreeNative() =>_handle.Dispose();
}

Additionally, there's a marshaller for the collection itself.

[CustomTypeMarshaller(typeof(ReadOnlySpan<Entry>),
    CustomTypeMarshallerKind.LinearCollection,
    Direction = CustomTypeMarshallerDirection.In,
    Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.CallerAllocatedBuffer | CustomTypeMarshallerFeatures.TwoStageMarshalling,
    BufferSize = 128)]
public unsafe ref struct EntryCollectionMarshaller
{
    private ReadOnlySpan<Entry> _managed;
    private IntPtr _nativeMemory;
    private Span<byte> _buffer;

    public EntryCollectionMarshaller(ReadOnlySpan<Entry> managed, int nativeElementSize)
        : this(managed, default, nativeElementSize)
    {
    }

    public EntryCollectionMarshaller(ReadOnlySpan<Entry> managed, Span<byte> buffer, int nativeElementSize)
    {
        _managed = managed;

        int requiredBufferSize = managed.Length * nativeElementSize;
        if (requiredBufferSize > buffer.Length)
        {
            _nativeMemory = Marshal.AllocCoTaskMem(requiredBufferSize);
            _buffer = new Span<byte>((void*)_nativeMemory, requiredBufferSize);
        }
        else
        {
            _nativeMemory = IntPtr.Zero;
            _buffer = buffer[0..requiredBufferSize];
        }
    }

    public ReadOnlySpan<Entry> GetManagedValuesSource() => _managed;

    public Span<byte> GetNativeValuesDestination() => _buffer;

    public ref NativeEntry GetPinnableReference() => ref MemoryMarshal.AsRef<NativeEntry>(_buffer);

    public NativeEntry* ToNativeValue() => (NativeEntry*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(_buffer));

    public void FreeNative() => Marshal.FreeCoTaskMem(_nativeMemory);
}

Lastly, I define a LibraryImport method with a ReadOnlySpan<Entry> parameter.

[LibraryImport(NativeLibraryName, EntryPoint = "use_entries")]
public static partial void UseEntries([MarshalUsing(typeof(EntryCollectionMarshaller))] ReadOnlySpan<Entry> entries, int length);

So far, so good. The generated stub marshals the entries parameter correctly. However, an issue occurs during the cleanup stage, when the memory handles need to be disposed. The snippet below is the section of code that is generated.

The problem is that it creates a new instance of the EntryMarshaller for each element in the collection and initializes it using FromNativeValue passing in the corresponding entry read from the native buffer, thus losing any state (i.e. the memory handle) from the marshaller that was previously used in the marshalling phase.

//
// Cleanup
//
{
    System.Span<global::SharedTypes.NativeEntry> __entries_gen_native__marshaller__nativeSpan = System.Runtime.InteropServices.MemoryMarshal.Cast<byte, global::SharedTypes.NativeEntry>(__entries_gen_native__marshaller.GetNativeValuesDestination());
    for (int __i0 = 0; __i0 < __entries_gen_native__marshaller__nativeSpan.Length; ++__i0)
    {
        global::SharedTypes.EntryMarshaller __entries_gen_native__marshaller__nativeSpan____i0__marshaller = default;
        __entries_gen_native__marshaller__nativeSpan____i0__marshaller.FromNativeValue(__entries_gen_native__marshaller__nativeSpan[__i0]);
        __entries_gen_native__marshaller__nativeSpan____i0__marshaller.FreeNative();
    }
}

__entries_gen_native__marshaller.FreeNative();

I'm not sure whether I'm doing this correctly, probably not, but I don't see how to release the native resources unless the original marshallers are preserved.

Author:	f2bo
Assignees:	-
Labels:	area-System.Runtime.InteropServices, area-Interop-coreclr, untriaged
Milestone:	-

[f2bo]

Yes. I undestand that.

However, the marshalling code writes a contiguous collection of NativeEntry elements into the native buffer. In order to restore the entire marshaller state, the data type of the parameter for FromNativeValue (or, as you suggest InitializeFromNativeValue) for each entry in the collection would have to be EntryMarshaller instead of NativeEntry, which is not blittable. This is the part that I don't quite see how to achieve with the current design.

[jkoritzinsky]

Yeah that is a problem within the current design. There's no mechanism to preserve the state of the marshaller structure between the marshal and unmarshal (and cleanup) stages when in a collection. I considered explicitly changing how we use the marshaller types in the non-collection scenario to make the behavior symmetric, but I never got around to it.

I think this is an interesting scenario that we should try to explore, as some cases (SafeHandle field marshalling comes to mind) may require a feature like this. I think we'll want to make it opt-in at the marshaller type level though as it won't be zero-cost and the overhead would be prohibitively expensive (and it might be difficult/impossible to do for ref struct marshaller types).

[f2bo]

and it might be difficult/impossible to do for ref struct marshaller types

True. I'd been experimenting with possible solutions and it does complicate things considerably.

[AaronRobinsonMSFT]

@f2bo We've incorporated the feedback here into a new API shape. I'm going to close this specific issue because I believe we have come up with a solution to be clearer in that proposal. Let's continue the conversation there if there are further issues.

#70859