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

Marshal.GetDelegateForFunction feature parity request #13197

Open
migueldeicaza opened this issue Aug 1, 2019 · 8 comments
Open

Marshal.GetDelegateForFunction feature parity request #13197

migueldeicaza opened this issue Aug 1, 2019 · 8 comments

Comments

@migueldeicaza
Copy link
Contributor

Hello team,

Marshal.GetDelegateForFunctionPointer thows an exception when the provided parameter is an Action<T>. Given that the actual function signature is known, this limitation seems unnecessary.

Mono currently allows this idiom, without throwing an exception, and is very convenient, to avoid producing a bunch of delegates for every combination of parameters during interop.

@jkoritzinsky
Copy link
Member

cc: @AaronRobinsonMSFT

@AaronRobinsonMSFT
Copy link
Member

@migueldeicaza To confirm, an example of the scenario in question would be:

class Dispatch<T>
{
    private readonly Action<T> act;
    public Dispatch(IntPtr fptr)
    {
        this.act = Marshal.GetDelegateForFunctionPointer<Action<T>>(fptr);
    }
    public void Exec(T t)
    {
        this.act(t);
    }
}

new Dispatch<int>(...).Exec(5);

If so, this is related to not permitting generic delegate types. That behavior is unfortunately documented as such which may make changing this behavior tough. Not against the change at all, simply unsure how much passionate people are about the current semantics.

Hi @terrajobst @richlander @jkotas, how would we feel about changing the semantics of this function?

@jkotas
Copy link
Member

jkotas commented Aug 1, 2019

Generics are not supported in regular interop signatures accross the board. Related discussion: See #4547 or #9136.

I think it is reasonable to relax some of these limitations for blittable types. I do not think it makes sense to relax it everywhere for everything. It would be a lot of runtime complexity for limited value.

I would recommend to start with relaxing the limitations for arguments of regular DllImport and function pointers that are getting a first-class C# support soon (https://github.com/dotnet/csharplang/blob/master/proposals/function-pointers.md). WIP PR dotnet/coreclr#23899 has started some of this work.

Once it is relaxed for regular DllImport and function pointers, we can then see whether we need to go further. My hypothesis is that the function pointers will quickly become the way to do interop because of they will be faster, more light-weight and avoid "collected delegate" trap. If my hypothesis is correct, fixing the limitations in Marshal.GetDelegateForFunctionPointer may be unnecessary.

@migueldeicaza
Copy link
Contributor Author

The actual use case:

void DoStuff (Delegate d)
{
    var x = Marshal.GetDelegateForFunctionPointer (d);
}

void method (int a, int b) {}

DoStuff ((Action<int,int>) method);

@hamarb123
Copy link
Contributor

This could (not sure how many people rely on this feature of mono) be a more significant problem as people will (presumably) have to / want to transition from the Mono runtime to the core runtime in the future. I think this should be prioritised for .NET 7 / 8 (whenever the transition is required, but also earlier the better).

Luckily for me though, I don't think I'm actively using my workaround anywhere at the moment (my workaround calls GetDelegateForFunctionPointerInternal (with every check except the one we're discussing here) with a fallback to the actual method for when it's fixed in the future, hopefully the internal method won't be renamed between now and then).

@jkotas
Copy link
Member

jkotas commented Jan 4, 2022

my workaround calls GetDelegateForFunctionPointerInternal

Calling internal runtime methods is unsupported. We reserve the right to change the internal runtime methods anytime. Also, this workaround won't behave correctly in all cases.

@hamarb123
Copy link
Contributor

hamarb123 commented Jan 4, 2022

Calling internal runtime methods is unsupported. We reserve the right to change the internal runtime methods anytime.

Yes, I'm aware. I'm just hoping that it won't be renamed before this change hopefully happens. If it does, I'll just have to deal with it.

Also, this workaround won't behave correctly in all cases.

I'm aware of that too, (I think) I've only ever used it (in the past) for Func & Action with blittable types for all parameters - this seems like something that is likely to work, and I don't think I had any issues (except possibly when the native function expected a float, but I provided a wrapped float in a struct, but that could have even been after I changed to function pointers, or caused by something else). I'm now using autogenerated code (including non-generic delegates, I did this before code generators) that use function pointers instead.

Also, just another quick question, why is Marshal.GetDelegateForFunctionPointer(IntPtr ptr, Type t) (and presumably other methods similar to this) hidden? Is it going to be unsupported in the future?

@jkotas
Copy link
Member

jkotas commented Jan 4, 2022

why is Marshal.GetDelegateForFunctionPointer(IntPtr ptr, Type t) (and presumably other methods similar to this) hidden?

These methods are not AOT friendly. They are hidden to lead people to the AOT-friendly generic equivalents.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

6 participants