Proposal: allow exception marshallers via '[return: MarshalUsing]' on generated COM interfaces methods without '[PreserveSig]'

[Sergio0694]

Overview

This came up in microsoft/CsWinRT#1858. We'd like for the COM generator to support [return: MarshalUsing] to use custom marshallers for exceptions, so that CsWinRT could correctly allow using the necessary error propagation logic from generated COM interfaces. Currently, trying the code below doesn't do anything (but also it doesn't emit any warnings, it just silently does nothing).

API proposal

class GeneratedComInterfaceAttribute
{
+     public Type? ExceptionToUnmanagedMarshaller { get; set; }
}

Use case example

Users will be able to implement COM interfaces like this, implemented on WinRT-exposed types:

[Guid("6234C2F7-9917-469F-BDB4-3E8C630598AF")]
[GeneratedComInterface(
    Options = ComInterfaceOptions.ManagedObjectWrapper,
    ExceptionToUnmanagedMarshaller = typeof(ExceptionHelpersHResultMarshaller))]
public partial interface IFoo
{
    void Bar();
}

With ExceptionHelpersHResultMarshaller being a WinRT-compatible marshaller type we'll include in CsWinRT.

Risks

None that I can think of.

Alternatives

Developers can work around this by manually writing the entire vtable backend (eg. here).
This is extremely clunky and error prone.

[dotnet-policy-service]

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

[jkotas]

What does this look like for methods with return value?

[Sergio0694]

My understanding is there would be no difference, because at the ABI level, the return is always an HRESULT anyway. So the generated ABI method will still always return an int __retVal local. This proposal is simply to allow overriding the default ExceptionAsHResultMarshaller<> type. Basically whenever that would be used by default by the COM generator, users should be able to indicate a custom equivalent type. Then the way the COM generator uses it would be identical to however ExceptionAsHResultMarshaller<> is already used today.

Eg. example codegen for:

[Guid("6234C2F7-9917-469F-BDB4-3E8C630598AF")]
[GeneratedComInterface(Options = ComInterfaceOptions.ManagedObjectWrapper)]
public partial interface IFoo
{
    double Bar();
}

You get:

file unsafe partial interface InterfaceImplementation
{
    [global::System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute(CallConvs = new[] { typeof(global::System.Runtime.CompilerServices.CallConvMemberFunction) })]
    internal static int ABI_Bar(global::System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch* __this_native, double* __invokeRetValUnmanaged__param)
    {
        global::IFoo @this = default;
        ref double __invokeRetValUnmanaged = ref *__invokeRetValUnmanaged__param;
        double __invokeRetVal = default;
        int __retVal = default;
        try
        {
            // Unmarshal - Convert native data to managed data.
            @this = global::System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch.GetInstance<global::IFoo>(__this_native);
            __invokeRetVal = @this.Bar();
            // NotifyForSuccessfulInvoke - Keep alive any managed objects that need to stay alive across the call.
            __retVal = 0; // S_OK
            // Marshal - Convert managed data to native data.
            __invokeRetValUnmanaged = __invokeRetVal;
        }
        catch (global::System.Exception __exception)
        {
            __retVal = global::System.Runtime.InteropServices.Marshalling.ExceptionAsHResultMarshaller<int>.ConvertToUnmanaged(__exception);
        }

        return __retVal;
    }
}

We want to override that

__retVal = global::System.Runtime.InteropServices.Marshalling.ExceptionAsHResultMarshaller<int>.ConvertToUnmanaged(__exception);

[jkotas]

What about properties that return strings or other types that need marshalling? I can write this today:

public partial interface IFoo
{
    [return: MarshalUsing(typeof(BStrStringMarshaller))]
    string Bar();
}

If I want to override the exception marshaling too, do I specify two return marshalling directives? Is it always going to non-ambiguous how the two marshalling directives should be applied?

[Sergio0694]

I see what you mean. Right, that syntax was meant to be an example, but the real thing would likely need some other attribute targeting methods that would allow to indicate an exception marshaller. I just don't know which exact API surface it would require. If we want to brainstorm what an actual API proposal to bring to review would look like, I'd imagine a starting point could maybe be:

namespace System.Runtime.InteropServices.Marshalling
{
    [AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
    public sealed class ExceptionMarshallerAttribute : Attribute
    {
        public ExceptionMarshallerAttribute(MarshalMode marshalMode, Type marshallerType);

        public MarshalMode MarshalMode { get; }
        public Type MarshallerType { get; }
    }
}

And then you'd use it as follows?

public partial interface IFoo
{
    [ExceptionMarshaller(MarshalMode.ManagedToUnmanagedIn, typeof(ExceptionHelpersHResultMarshaller<int>))]
    void Bar();
}

cc. @jkoritzinsky for thoughts, since you also mentioned we'd need a new API for this likely 🙂

EDIT: not entirely sure if we need MarshalMode, since it seems implied by the context? Maybe this is sufficient?

namespace System.Runtime.InteropServices.Marshalling
{
    [AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
    public sealed class ExceptionMarshallerAttribute : Attribute
    {
        public ExceptionMarshallerAttribute(Type marshallerType);

        public Type MarshallerType { get; }
    }
}

And then you just do:

public partial interface IFoo
{
    [ExceptionMarshaller(typeof(ExceptionHelpersHResultMarshaller<int>))]
    void Bar();
}

[dongle-the-gadget]

I believe we should have a default exception handler for IErrorInfo, as well, since it looks like generated COM doesn't support that yet.

[jkoritzinsky]

We've talked about this on the team and here's the API shape we thought of:

class GeneratedComInterfaceAttribute
{
+     public Type? ExceptionToUnmanagedMarshaller { get; set; }
}

This marshaller would be used to marshal an Exception to the unmanaged return value. We want to put it on the interface as a whole as it's very likely that a given interface wants to handle errors consistently, and we feel that COM interfaces that use PreserveSig with a non-HRESULT return type is rare enough that it wouldn't overlap with the scenarios for this property, at least for V1.

A marshaller type that is assigned to ExceptionToUnmanagedMarshaller would need to have the UnmanagedToManagedOut marshal mode, with a managed type of Exception and an unmanaged type of the unmanaged return of the methods on the interface (so int 99% of the time).

If we feel that the non-int scenarios are common enough, we can revise this API shape and add a member-level option.

I believe we should have a default exception handler for IErrorInfo, as well, since it looks like generated COM doesn't support that yet.

Source-Generated COM uses the built-in IErrorInfo support, so on CoreCLR we do set the IErrorInfo when you're not using trimming. We don't set it on NativeAOT today. Do you have a scenario where you'd like us to set it in these cases?

[AaronRobinsonMSFT]

Should be Type?, right?

[jkoritzinsky]

Yep, updating now.

[Sergio0694]

This looks good to me, thank you for taking a look! We can add our own marshaller type for this for WinRT scenarios in CsWinRT, and then direct people to using that. We could even add an analyzer to warn if someone uses a generated COM interface on a WinRT type without specifying this marshaller, to help with discoverability. Basically the scenario from this issue would be like:

[Guid("6234C2F7-9917-469F-BDB4-3E8C630598AF")]
[GeneratedComInterface(
    Options = ComInterfaceOptions.ManagedObjectWrapper,
    ExceptionToUnmanagedMarshaller = typeof(ExceptionHelpersHResultMarshaller))]
public partial interface IFoo
{
    void Bar();
}

Where ExceptionHelpersHResultMarshaller would be our own marshaller somewhere in WinRT.*.

@jkoritzinsky what else are we missing to make this proposal ready for review? 🙂

[jkoritzinsky]

I think we just need to update the top post into the API review format and then we can mark this as api-ready-for-review API is ready for review, it is NOT ready for implementation

[AaronRobinsonMSFT]

@jkoritzinsky what else are we missing to make this proposal ready for review? 🙂

Stakeholder sign off :)

[Sergio0694]

Thank you! I've updated the first post with the updated API proposal 😄

[bartonjs]

Video

• Looks good as proposed.
• For a reference example of what one of these types would be, see ExceptionAsHResultMarshaller<T>

namespace System.Runtime.InteropServices.Marshalling;

public partial class GeneratedComInterfaceAttribute
{
    public Type? ExceptionToUnmanagedMarshaller { get; set; }
}