Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linear collection marshallers will generate calls to GetUnmanagedValuesDestination for 'out' variables during cleanup in ManagedToUnmanaged #89885

Closed
jtschuster opened this issue Aug 2, 2023 · 1 comment · Fixed by #90057

Comments

@jtschuster
Copy link
Member

According to https://github.com/dotnet/runtime/blob/main/docs/design/libraries/LibraryImportGenerator/UserTypeMarshallingV2.md#stateless-unmanaged-managed-1, it is not required to have this method since the value is only marshalled from unmanaged to managed. The unmarshal stage uses GetUnmanagedValuesSource, which cleanup should use as well.

The method

        void MethodOut(
            [MarshalUsing(CountElementName = nameof(size))] out StatelessCollection<StatelessType> pOut,
            out int size);
    void global::SharedTypes.ComInterfaces.IStatelessCollectionStatelessElement.MethodOut(out global::SharedTypes.ComInterfaces.StatelessCollection<global::SharedTypes.ComInterfaces.StatelessType> pOut, out int size)
    {
        var(__this, __vtable_native) = ((System.Runtime.InteropServices.Marshalling.IUnmanagedVirtualMethodTableProvider)this).GetVirtualMethodTableInfoForKey(typeof(global::SharedTypes.ComInterfaces.IStatelessCollectionStatelessElement));
        bool __invokeSucceeded = default;
        System.Runtime.CompilerServices.Unsafe.SkipInit(out pOut);
        System.Runtime.CompilerServices.Unsafe.SkipInit(out size);
        nint __pOut_native = default;
        int __invokeRetVal = default;
        // Setup - Perform required setup.
        int __pOut_native__numElements;
        System.Runtime.CompilerServices.Unsafe.SkipInit(out __pOut_native__numElements);
        try
        {
            // Pin - Pin data in preparation for calling the P/Invoke.
            fixed (int* __size_native = &size)
            {
                __invokeRetVal = ((delegate* unmanaged[MemberFunction]<void*, nint*, int*, int> )__vtable_native[6])(__this, &__pOut_native, __size_native);
            }

            __invokeSucceeded = true;
            System.GC.KeepAlive(this);
            // Unmarshal - Convert native data to managed data.
            System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(__invokeRetVal);
            __pOut_native__numElements = size;
            pOut = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.Default.AllocateContainerForManagedElements(__pOut_native, __pOut_native__numElements);
            {
                System.ReadOnlySpan<nint> __pOut_native__nativeSpan = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.Default.GetUnmanagedValuesSource(__pOut_native, __pOut_native__numElements);
                System.Span<global::SharedTypes.ComInterfaces.StatelessType> __pOut_native__managedSpan = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.Default.GetManagedValuesDestination(pOut);
                for (int __i0 = 0; __i0 < __pOut_native__numElements; ++__i0)
                {
                    __pOut_native__managedSpan[__i0] = global::SharedTypes.ComInterfaces.StatelessTypeMarshaller.ConvertToManaged(__pOut_native__nativeSpan[__i0]);
                }
            }
        }
        finally
        {
            if (__invokeSucceeded)
            {
                // CleanupCalleeAllocated - Perform cleanup of callee allocated resources.
                {
// ISSUE HERE
                    // Should call GetUnmanagedValuesSource
                    System.ReadOnlySpan<nint> __pOut_native__nativeSpan = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.Default.GetUnmanagedValuesDestination(__pOut_native, __pOut_native__numElements);
                    for (int __i0 = 0; __i0 < __pOut_native__nativeSpan.Length; ++__i0)
                    {
                        global::SharedTypes.ComInterfaces.StatelessTypeMarshaller.Free(__pOut_native__nativeSpan[__i0]);
                    }
                }

                __pOut_native__numElements = size;
                global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.Default.Free(__pOut_native);
            }
        }
    }
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 2, 2023
@ghost
Copy link

ghost commented Aug 2, 2023

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

Issue Details

According to https://github.com/dotnet/runtime/blob/main/docs/design/libraries/LibraryImportGenerator/UserTypeMarshallingV2.md#stateless-unmanaged-managed-1, it is not required to have this method since the value is only marshalled from unmanaged to managed. The unmarshal stage uses GetUnmanagedValuesSource, which cleanup should use as well.

The method

        void MethodOut(
            [MarshalUsing(CountElementName = nameof(size))] out StatelessCollection<StatelessType> pOut,
            out int size);
    void global::SharedTypes.ComInterfaces.IStatelessCollectionStatelessElement.MethodOut(out global::SharedTypes.ComInterfaces.StatelessCollection<global::SharedTypes.ComInterfaces.StatelessType> pOut, out int size)
    {
        var(__this, __vtable_native) = ((System.Runtime.InteropServices.Marshalling.IUnmanagedVirtualMethodTableProvider)this).GetVirtualMethodTableInfoForKey(typeof(global::SharedTypes.ComInterfaces.IStatelessCollectionStatelessElement));
        bool __invokeSucceeded = default;
        System.Runtime.CompilerServices.Unsafe.SkipInit(out pOut);
        System.Runtime.CompilerServices.Unsafe.SkipInit(out size);
        nint __pOut_native = default;
        int __invokeRetVal = default;
        // Setup - Perform required setup.
        int __pOut_native__numElements;
        System.Runtime.CompilerServices.Unsafe.SkipInit(out __pOut_native__numElements);
        try
        {
            // Pin - Pin data in preparation for calling the P/Invoke.
            fixed (int* __size_native = &size)
            {
                __invokeRetVal = ((delegate* unmanaged[MemberFunction]<void*, nint*, int*, int> )__vtable_native[6])(__this, &__pOut_native, __size_native);
            }

            __invokeSucceeded = true;
            System.GC.KeepAlive(this);
            // Unmarshal - Convert native data to managed data.
            System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(__invokeRetVal);
            __pOut_native__numElements = size;
            pOut = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.Default.AllocateContainerForManagedElements(__pOut_native, __pOut_native__numElements);
            {
                System.ReadOnlySpan<nint> __pOut_native__nativeSpan = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.Default.GetUnmanagedValuesSource(__pOut_native, __pOut_native__numElements);
                System.Span<global::SharedTypes.ComInterfaces.StatelessType> __pOut_native__managedSpan = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.Default.GetManagedValuesDestination(pOut);
                for (int __i0 = 0; __i0 < __pOut_native__numElements; ++__i0)
                {
                    __pOut_native__managedSpan[__i0] = global::SharedTypes.ComInterfaces.StatelessTypeMarshaller.ConvertToManaged(__pOut_native__nativeSpan[__i0]);
                }
            }
        }
        finally
        {
            if (__invokeSucceeded)
            {
                // CleanupCalleeAllocated - Perform cleanup of callee allocated resources.
                {
// ISSUE HERE
                    // Should call GetUnmanagedValuesSource
                    System.ReadOnlySpan<nint> __pOut_native__nativeSpan = global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.Default.GetUnmanagedValuesDestination(__pOut_native, __pOut_native__numElements);
                    for (int __i0 = 0; __i0 < __pOut_native__nativeSpan.Length; ++__i0)
                    {
                        global::SharedTypes.ComInterfaces.StatelessTypeMarshaller.Free(__pOut_native__nativeSpan[__i0]);
                    }
                }

                __pOut_native__numElements = size;
                global::SharedTypes.ComInterfaces.StatelessCollectionMarshaller<global::SharedTypes.ComInterfaces.StatelessType, nint>.Default.Free(__pOut_native);
            }
        }
    }
Author: jtschuster
Assignees: -
Labels:

area-System.Runtime.InteropServices

Milestone: -

@jtschuster jtschuster removed the untriaged New issue has not been triaged by the area owner label Aug 3, 2023
@jtschuster jtschuster added this to the 8.0.0 milestone Aug 3, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 4, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 7, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant