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

There is no way to make a reflection call to a method with ref parameters while preserving aliasing nature of ref #15663

Closed
bartdesmet opened this issue Nov 8, 2015 · 6 comments
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Reflection help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@bartdesmet
Copy link
Contributor

VSadov: while passing parameters byref can be modeled through passing mutable object[] + copybacks. It is not the same as truly passing arguments byref.
Some methods, like ones from Interlocked family, rely on aliasing nature of ref and the difference is observable.

Since all the calls in Interpreter are done via reflection, we cannot interpret certain calls correctly due to insufficient APIs.

==============

Unlike the expression compiler where address-obtaining instructions are used to call by reference, the expression interpreter has to perform copy operations in and out of an arguments array upon calling the Invoke method. As a result, calls to constructs like Interlocked methods don't have the intended effect. An example repro is shown below:

var inc = ReflectionUtils.MethodInfoOf((int x) => Interlocked.Increment(ref x));
var run = ReflectionUtils.MethodInfoOf(() => Task.Run(default(Action)));
var all = ReflectionUtils.MethodInfoOf(() => Task.WaitAll(default(Task), default(Task)));

var p = Expression.Parameter(typeof(int));
var n = Expression.Parameter(typeof(int));
var i = Expression.Parameter(typeof(int));
var b = Expression.Label();

var l =
    Expression.Block(
        new[] { i },
        Expression.Loop(
            Expression.Block(
                Expression.IfThen(
                    Expression.Equal(i, n),
                    Expression.Break(b)
                ),
                Expression.Call(inc, p),
                Expression.PostIncrementAssign(i)
            ),
            b
        )
    );

var a = Expression.Lambda<Action>(l);
var t = Expression.Call(run, a);

var e =
    Expression.Block(
        new[] { p },
        Expression.Call(all, Expression.NewArrayInit(typeof(Task), t, t)),
        p
    );

var f = Expression.Lambda<Func<int, int>>(e, n).Compile();

One would expect f(N) to return 2 * N. This is the case for the compiler, but not for the interpreter.

@bartdesmet
Copy link
Contributor Author

Unfortunately, the Invoke method on MethodBase has little flexibility to deal with this, or I should be missing something obvious. We'd have to be able to pass a T& in an object[] slot for the arguments passed to Invoke. Maybe TypedReference provides a way out if we'd be able to pass it and if reflection would understand it.

This is similar to (but deeper than) the Type.Missing interpretation restriction we've encountered as well. Both may need some reflection API work in order for the interpreter to function correctly.

@JonHanna
Copy link
Contributor

I wonder if it would be worth special-casing Interlocked. On the one hand it wouldn't be rare to have entire projects where the only calls that required such atomic behaviour were calls to Interlocked methods. On the other, it could feel rather arbitrary to someone who really needs an atomic call to another method, and particularly frustrating if they'd taken pains to make their own such method work well in the sort of contexts that need atomicity.

@VSadov
Copy link
Member

VSadov commented Nov 29, 2016

This bug is not actionable on Linq.Expression side. there are no reflection APIs that would allow to call method with ref parameters while preserving aliasing nature of refs.
We need a bug against FX specifically requesting such API.

@VSadov VSadov changed the title Expression interpreter can't guarantee atomic operations on by-ref parameters There is no way to make a reflection call to a method with ref parameters while preserving aliasing nature of ref Nov 29, 2016
@VSadov VSadov removed their assignment Nov 29, 2016
@JonHanna
Copy link
Contributor

As we can expect such methods to become more popular now they have direct support in C# 7, those that return ref should also be considered.

@karelz
Copy link
Member

karelz commented Feb 28, 2017

Next step: We need proper design and formal API proposal.
Complexity: difficult/expert-level.

@ghost
Copy link

ghost commented Sep 11, 2018

This is being tracked in https://github.com/dotnet/corefx/issues/14021.

@ghost ghost closed this as completed Sep 11, 2018
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 3.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 4, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Reflection help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

No branches or pull requests

5 participants