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

[2019-06] [runtime] Make cross-appdomain wrappers throw NIE for methods with ref struct args #14867

Merged
merged 4 commits into from Jun 7, 2019

Conversation

monojenkins
Copy link
Contributor

If we have a ref struct and a method that takes it as an argument:

[Serializable]
public ref struct R {
  public int i;
}

public interface I {
  public void M1 (R r, SomeClass o);
}

public class M : MarshalByRefObject, I {
  public void M1 (R r, SomeClass o) { ... }
}

If we create an instance of M in another domain and try to call it:

  I obj = (I)otherDomain.CreateInstanceAndUnwrap (typeof(M).Assembly.FullName,
  nameof(M));
  R r;
  r.i = 42;
  SomeClass o = ...;
  obj.M1 (r, o);

Mono will need to create a wrapper that can invoke M.M1 in the other domain. The way the wrapper works in mono_marshal_get_xappdomain_invoke is by creating an array of all the arguments that need to be serialized (see mono_get_xdomain_marshal_type) and then serializing the whole array (by calling out to System.Runtime.Remoting.RemotingServices.SerializeCallData). In the example it would make a two element array and try to serialize r and o into it. For valuetypes it normally does this by boxing the argument. However since R is a ref-struct, boxing it is not allowed.

This works on .NET Framework.

However for Mono we would need to serilize R without boxing it, which seems challenging with our current setup.

So for now we generate a wrapper that just throws a NotImplementedException if it is ever called.

This fixes #14809 (in that marshalling a TextWriter across domains works again) but in an unsatisfactory way (because you can't call any of the ReadOnlySpan<char> overloads on the transparent proxy object).


Also mark MethodInfo and MethodBase with [Serializable] again so that unit testing with NUnit.ApplicationDomain works again (they need to serialize method info for stack traces, I think)

Backport of #14863.

/cc @lambdageek

…truct arguments

If we have a ref struct and a method that takes it as an argument:

```
[Serializable]
public ref struct R {
  public int i;
}

public interface I {
  public void M1 (R r, SomeClass o);
}

public class M : MarshalByRefObject, I {
  public void M1 (R r, SomeClass o) { ... }
}
```

If we create an instance of M in another domain and try to call it:
```
  I obj = (I)otherDomain.CreateInstanceAndUnwrap (typeof(M).Assembly.FullName,
  nameof(M));
  R r;
  r.i = 42;
  SomeClass o = ...;
  obj.M1 (r, o);
```

Mono will need to create a wrapper that can invoke M.M1 in the other domain.
The way the wrapper works in `mono_marshal_get_xappdomain_invoke` is by
creating an array of all the arguments that need to be serialized (see
`mono_get_xdomain_marshal_type`) and then serializing the whole array (by
calling out to `System.Runtime.Remoting.RemotingServices.SerializeCallData`).
In the example it would make a two element array and try to serialize `r` and
`o` into it. For valuetypes it normally does this by boxing the argument.
However since `R` is a ref-struct, boxing it is not allowed.

This works on .NET Framework.

However for Mono we would need to serilize `R` without boxing it, which seems
challenging with our current setup.

So for now we generate a wrapper that just throws a NotImplementedException if
it is ever called.

This fixes mono#14809 (in that marshalling a
TextWriter across domains works again) but in an unsatisfactory way (because
you can't call any of the ReadOnlySpan<char> overloads on the transparent proxy
object).
@akoeplinger
Copy link
Member

@monojenkins commit apidiff

monojenkins added a commit to mono/api-snapshot that referenced this pull request Jun 7, 2019
@akoeplinger akoeplinger merged commit ddef564 into mono:2019-06 Jun 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants