dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.26k stars 4.73k forks source link

Unmanaged Exception Interop on Unix #35017

Open tannergooding opened 4 years ago

tannergooding commented 4 years ago

There are a few issues/documentation that currently indicate unmanaged exception interop doesn't exist on Unix:

The latter of which indicates

the Unix ABI has no definition for exception handling

However, I find this confusing as the System V ABI (https://github.com/hjl-tools/x86-psABI/wiki/X86-psABI) does define this in Section 6.2 - Unwind Library Interface, as does the Itanium C++ ABI (https://itanium-cxx-abi.github.io/cxx-abi/abi-eh.html), both of which are used by code and more specifically C/C++ code on Unix platforms.

It was mentioned that Windows would be made consistent and the support for propagating exceptions was going to be dropped, but that was latter dropped in favor of compatibility. Given that it should be possible to handle and even propagate exception information across the boundaries and it would make the cross-platform behavior consistent, is it worth taking another look at this?

Dotnet-GitSync-Bot commented 4 years ago

I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label.

tannergooding commented 4 years ago

CC. @janvorli, @jkotas, @jkoritzinsky

jkotas commented 4 years ago

You can do C++ exception interop manually today by doing try+catch on one side, remap the exception to the other system and throw on the other side. It is a boiler plate code, but it gives you a full control over what you can do. For example, we do that in the crossgen2 compiler here: https://github.com/dotnet/runtime/blob/master/src/coreclr/src/tools/crossgen2/jitinterface/jitinterface.h#L200

I think it would be a fine idea to explore how to reduce this boilerplate code, e.g. by having opt-in mechanism that allows you to attach a custom exception re-mapper to specific PInvokes/reverse PInvokes. Xamarin does it for Objective C exception interop, but it is specific to Objective C and depends on Mono embedding APIs: https://github.com/xamarin/xamarin-macios/blob/master/runtime/EXCEPTIONS.md .

ceztko commented 9 months ago

remap the exception to the other system and throw on the other side [...] For example, we do that in the crossgen2 compiler here:

I'm updating the link of jitinterface.h#L200 since it got broken by updating the master.

I think it would be a fine idea to explore how to reduce this boilerplate code, e.g. by having opt-in mechanism that allows you to attach a custom exception re-mapper to specific PInvokes/reverse PInvokes. Xamarin does it for Objective C exception interop

Talking about propagating exception from unmanaged-managed boundary and removing the need for boilerplate, especially in the case of custom error handlers, I imagined having something like (tentative invented API) Marshal.SetManagedException(ex), and a boolean flag CheckExceptionOnExit in the DllImport/LibraryImport attributes. So one on .NET would expose native functionalities and define the error handler like the following:

public class MyLibrary
{
    static ErrorHandlerCallback? _errorHandler;

    [UnmanagedFunctionPointer(CallingConvention.Cdecl)]
    delegate void ErrorHandlerCallback([MarshalAs(UnmanagedType.LPUTF8Str)]string message);

    static MyLibrary()
    {
        _errorHandler = HandleError;
        SetErrorHandler(_errorHandler);
    }

    public static void Foo()
    {
        NativeFoo();
    }

    static void HandleError(string message)
    {
        // Set managed exception from [UnmanagedFunctionPointer] handler, to be thrown late by the runtime
        // NOTE: Tentative invented API
        Marshal.SetManagedException(new Exception(message));
    }

    // NOTE: Tentative invented API
    [DllImport("SharedLibrary", CheckExceptionOnExit = true, CallingConvention = CallingConvention.Cdecl)]
    static extern void NativeFoo();

    [DllImport("SharedLibrary", CallingConvention = CallingConvention.Cdecl)]
    static extern void SetErrorHandler(ErrorHandlerCallback callback);
}

And externally one would just try-catch on MyLibray.Foo().

MichalPetryka commented 9 months ago

Talking about propagating exception from unmanaged-managed boundary and removing the need for boilerplate, especially in the case of custom error handlers, I imagined having something like (tentative invented API) Marshal.SetManagedException(ex), and a boolean flag CheckExceptionOnExit in the DllImport/LibraryImport attributes.

You could make such wrapping yourself easily, there's no need to involve the runtime. There's an issue though that such exception storing wouldn't indicate a failure to native code which means you'd need to return some exit codes there. An example of manual implementation:

public static class Test
{
    [ThreadStatic]
    private static Exception exceptionStorage;

    [StackTraceHidden]
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static void RethrowNative()
    {
        Exception exception = exceptionStorage;
        if (exception == null)
            return;
        exceptionStorage = null;
        Rethrow(exception);

        [StackTraceHidden]
        static void Rethrow(Exception ex) => throw ex;
    }

    public static void Foo()
    {
        Bar(&Export);
        RethrowNative();
    }

    [UnmanagedCallersOnly(CallConvs = [typeof(CallConvCdecl)])]
    static int Export()
    {
        try
        {
            Throw();
            return 0;
        }
        catch (Exception ex)
        {
            exceptionStorage = ex;
            return 1;
        }
    }

    private static void Throw() => throw new Exception();

    [DllImport("Library", CallingConvention = CallingConvention.Cdecl)]
    static extern void Bar(delegate* unmanaged[Cdecl]<int> ptr);
}
jkotas commented 9 months ago

void HandleError(string message, IntPtr data)

Where would string message and IntPtr data come from?

ceztko commented 9 months ago

Where would string message and IntPtr data come from?

Sorry, forget about that IntPtr data (I've removed it in the sample above). Having an opaque data is just the classical C style approach to store some context in case of callbacks, but it's really unnecessary here so it was a bit of cargo cult. As per string message, that's obviously the error message that is passed when the ErrorHandlerCallback callback is called in the native part, hence the need to have the managed callback (or the Native AOT compiled one, as in the other similar issue) to be able to throw or set/"install" an exception in the correct runtime.

jkotas commented 9 months ago

when the ErrorHandlerCallback callback is called in the native part,

How would the runtime create this native part?

ceztko commented 9 months ago

when the ErrorHandlerCallback callback is called in the native part,

How would the runtime create this native part?

@jkotas I'm confused by your question, in the sense the use case I was describing is really the classical .NET managed wrapper on an existing native library, hence native part is 3rd party or user made. Just look at the proposed API in the sample above with this in mind.

ceztko commented 9 months ago

You could make such wrapping yourself easily, there's no need to involve the runtime. [...] An example of manual implementation

@MichalPetryka You are absolutely correct but that still needs a bit of boilerplate, which is the calling of RethrowNative() after all P/Invoke calls, and doing it becomes a bit more frustrating with native functions that return values. Having a support for such mechanism in the runtime, to be enabled in the DllImport/LibraryImport attribute, would look a bit more clean and less hacky to me but I guess it's also a matter of personal taste.

jkotas commented 9 months ago

How would the runtime create this native part?

@jkotas I'm confused by your question,

Ah sorry, I looked at your example again. It makes sense now.

DllImport/LibraryImport attribute

If we were to do something here, I expect that it would be via LibraryImport source generator.

ceztko commented 9 months ago

If we were to do something here, I expect that it would be via LibraryImport source generator.

It would be totally fine if interop improvements go first (or exclusively) to LibraryImport. How do you see the idea of a (thread local stored) exception to be thrown automatically at the return from P/Invoke calls with some level of support from the runtime?

jkotas commented 9 months ago

How do you see the idea of a (thread local stored) exception to be thrown automatically at the return from P/Invoke calls with some level of support from the runtime?

As in define the thread local in the runtime and expose getter/setter for it? It has miniscule benefit. I think the thread local can be generated by the LibraryImport source generator just fine.

ceztko commented 9 months ago

As in define the thread local in the runtime and expose getter/setter for it? It has miniscule benefit. I think the thread local can be generated by the LibraryImport source generator just fine.

Ok, but how do you "store" the exception to that thread local if there is no getter/setter?

jkotas commented 9 months ago

Ok, but how do you "store" the exception to that thread local if there is no getter/setter?

The source generated code would store into and load from the thread local variable that is generated by the source generator and that is internal to your assembly, similar to how https://github.com/dotnet/runtime/issues/35017#issuecomment-1927205228 does it.

MichalPetryka commented 9 months ago

The source generated code

I don't think the generator generates anything for UnmanagedCallersOnly/UnmanagedFunctionPointer that could store it in the callback?

jkotas commented 9 months ago

I don't think the generator generates anything for UnmanagedCallersOnly/UnmanagedFunctionPointer that could store it in the callback?

The reverse PInvoke support in interop marshaling source generator is tracked by #63590. (The examples mentioned here so far were for PInvoke.)

ceztko commented 9 months ago

The source generated code would store into and load from the thread local variable that is generated by the source generator and that is internal to your assembly, similar to how #35017 (comment) does it.

@jkotas But that example you linked still does some (little) boilerplate to be called after all P/Invoke calls. If you add some runtime support doing exception checking/throwing through the LibraryImport generator, will I then be able to safely throw from [UnmanagedFunctionPointer] delegates? Just to understand what you have in mind.

jkotas commented 9 months ago

will I then be able to safely throw from [UnmanagedFunctionPointer] delegates?

No, you would not be. The unmanaged side still needs to have the boiler plate code. There is no good way for the runtime to generate the unmanaged side of the boilerplate in a portable way (https://github.com/dotnet/runtime/issues/97952#issuecomment-1926537178).

ceztko commented 9 months ago

The unmanaged side still needs to have the boiler plate code.

Ah, the unmanaged part may still need some boiler plate, of course. I now understand that you often tried to grasp something about the unmanaged part, but no, I was always just referring to the boiler plate in the managed part.

Again the question is: assuming I will have the right boiler plate in the unmanaged part, with this support in the LibraryImport generator (and/or runtime) will I be able to have no boiler code in the boiler plate and throw from [UnmanagedFunctionPointer] delegates in the managed part?

For example one may imagine that I have function like this:

static void HandleError(string message)
{
    throw new Excpetion(message);
}

Then I assign it to a delegate that gets passed to unamanged part:

[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
    delegate void ErrorHandlerCallback([MarshalAs(UnmanagedType.LPUTF8Str)]string message);

Then the runtime/compiler in the trampoline it does something like:

static void trampoline_HandleError(string message)
{
    try
    {
        HandleError(message);
    }
    catch (Exception ex)
    {
        // ... Store the exception in a thread local variable, or in the stack somewhere 
    }
}

Then I may have a LibraryImport attribute with a CheckExceptionOnExit (tentative) that will do the boilerplate of checking if a managed exception is stored and throw it just in case.

[LibraryImport("SharedLibrary", CheckExceptionOnExit = true)]
[UnmanagedCallConv(CallConvs = new[] { typeof(CallConvCdecl) })]
static partial void Foo();

So I can call this Foo() method and have a managed exception correctly thrown when necessary. Will the system you suggest work like this? Sorry but I understand it only through actual (pseudo)code.

jkotas commented 9 months ago

Then the runtime/compiler in the trampoline it does something like:

static void trampoline_HandleError(string message)

This assumes that https://github.com/dotnet/runtime/issues/63590 is implemented. I would expect that there would be some gesture to trigger this behavior. It would not happen by default.

[LibraryImport("SharedLibrary", CheckExceptionOnExit = true)]
[UnmanagedCallConv(CallConvs = new[] { typeof(CallConvCdecl) })]
static partial void Foo();

I would expect this to be more general. All you need is that the marshalling source generator calls your method right after the raw PInvoke. There are other patterns you may want to do - call a method right before the raw invoke, keep some state between before and after the PInvoke. It looks very similar to what the argument marshallers do, so we may want to reuse the marshaller concepts here. The new attribute would then point to the name of the marshaller, something like [LibraryImport("SharedLibrary", MarshalUsing=typeof(MyExceptionMarshaller)].

ceztko commented 9 months ago

I would expect this to be more general. All you need is that the marshalling source generator calls your method right after the raw PInvoke. [...] It looks very similar to what the argument marshallers do, so we may want to reuse the marshaller concepts here. The new attribute would then point to the name of the marshaller

Ah, finally I got it! I didn't understand you had user made custom marshalling in mind. Then the [UnmanagedFunctionPointer] delegate could just set the exception in the storage of such user defined MyExceptionMarshaller. Looks great with the only possible concern on my side that in this way different libraries may all "waste" thread local storage, that's why I was suggesting the idea of globally defined one. Or you may consider of being able to store the exception in the stack, but then the LibraryImport directive should probably be very aware of the pattern (hence loosing generality).

Thank you for the replies and clarifications. I hope this feedback is seen as useful/productive, and I hope to see something materialize with this regard for .NET 9/10.