Skip to content

Commit e288589

Browse files
authored
[Java.Interop] Add JniRuntime.JniValueManager.GetPeer() (#1295)
Context: https://github.com/dotnet/android/pull/9630/files#r1891090085 Context: dotnet/android#9716 As part of NativeAOT prototyping within dotnet/android, we need to update [`Java.Lang.Object.GetObject<T>()`][0] so that it uses `Java.Interop.JniRuntime.JniValueManager` APIs instead of its own `TypeManager.CreateInstance()` invocation, as `TypeManager.CreateInstance()` hits P/Invokes which don't currently work in the NativeAOT sample environment. However, update it to *what*? The obvious thing to do would be to use `JniRuntime.JniValueManager.GetValue()`: partial class Object { internal static IJavaPeerable? GetObject (IntPtr handle, JniHandleOwnership transfer, Type? type = null) { var r = PeekObject (handle, type); if (r != null) { JNIEnv.DeleteRef (handle, transfer); return r; } var reference = new JniObjectReference (handle); r = (IJavaPeerable) JNIEnvInit.ValueManager.GetValue ( ref reference, JniObjectReferenceOptions.Copy, type); JNIEnv.DeleteRef (handle, transfer); return r; } } The problem is that this blows up good: <System.InvalidCastException: Arg_InvalidCastException at Java.Lang.Object.GetObject(IntPtr , JniHandleOwnership , Type ) at Android.Runtime.JNIEnv.<CreateNativeArrayElementToManaged>g__GetObject|74_11(IntPtr , Type ) at Android.Runtime.JNIEnv.<>c.<CreateNativeArrayElementToManaged>b__74_9(Type type, IntPtr source, Int32 index) at Android.Runtime.JNIEnv.GetObjectArray(IntPtr , Type[] ) at Java.InteropTests.JnienvTest.<>c__DisplayClass26_0.<MoarThreadingTests>b__1()> because somewhere in that stack trace we have a `java.lang.Integer` instance, and `.GetValue(Integer_ref, …)` returns a `System.Int32` containing the underling value, *not* an `IJavaPeerable` value for the `java.lang.Integer` instance. Consider: var i_class = new JniType ("java/lang/Integer"); var i_ctor = i_class.GetConstructor ("(I)V"); JniArgumentValue* i_args = stackalloc JniArgumentValue [1]; i_args [0] = new JniArgumentValue (42); var i_value = i_class.NewObject (i_ctor, i_args); var v = JniEnvironment.Runtime.ValueManager.GetValue (ref i_value, JniObjectReferenceOptions.CopyAndDispose, null); Console.WriteLine ($"v? {v} {v?.GetType ()}"); which prints `v? 42 System.Int32`. This was expected and desirable, until we try to use `GetValue()` for `Object.GetObject<T>()`; the semantics don't match. Add a new `JniRuntime.JniValueManager.GetPeer()` method, which better matches the semantics that `Object.GetObject<T>()` requires, allowing: partial class Object { internal static IJavaPeerable? GetObject (IntPtr handle, JniHandleOwnership transfer, Type? type = null) { var r = JNIEnvInit.ValueManager.GetPeer (new JniObjectReference (handle)); JNIEnv.DeleteRef (handle, transfer); return r; } } Finally, add a new `JniRuntimeJniValueManagerContract` unit test, so that we have "more formalized" semantic requirements on `JniRuntime.JniValueManager` implementations. [0]: https://github.com/dotnet/android/blob/cc35a263e046444c2123e7a7dba106b18e2bbebb/src/Mono.Android/Java.Lang/Object.cs#L133-L166
1 parent bbb15b7 commit e288589

File tree

5 files changed

+325
-1
lines changed

5 files changed

+325
-1
lines changed

Diff for: src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs

+23-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ partial class CreationOptions {
3232
public JniValueManager? ValueManager {get; set;}
3333
}
3434

35-
JniValueManager? valueManager;
35+
internal JniValueManager? valueManager;
3636
public JniValueManager ValueManager {
3737
get => valueManager ?? throw new NotSupportedException ();
3838
}
@@ -271,6 +271,28 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type)
271271
return type;
272272
}
273273

274+
public IJavaPeerable? GetPeer (
275+
JniObjectReference reference,
276+
[DynamicallyAccessedMembers (Constructors)]
277+
Type? targetType = null)
278+
{
279+
if (disposed) {
280+
throw new ObjectDisposedException (GetType ().Name);
281+
}
282+
283+
if (!reference.IsValid) {
284+
return null;
285+
}
286+
287+
var peeked = PeekPeer (reference);
288+
if (peeked != null &&
289+
(targetType == null ||
290+
targetType.IsAssignableFrom (peeked.GetType ()))) {
291+
return peeked;
292+
}
293+
return CreatePeer (ref reference, JniObjectReferenceOptions.Copy, targetType);
294+
}
295+
274296
public virtual IJavaPeerable? CreatePeer (
275297
ref JniObjectReference reference,
276298
JniObjectReferenceOptions transfer,

Diff for: src/Java.Interop/PublicAPI.Unshipped.txt

+1
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,6 @@ virtual Java.Interop.JniRuntime.OnEnterMarshalMethod() -> void
55
virtual Java.Interop.JniRuntime.OnUserUnhandledException(ref Java.Interop.JniTransition transition, System.Exception! e) -> void
66
Java.Interop.JavaException.JavaException(ref Java.Interop.JniObjectReference reference, Java.Interop.JniObjectReferenceOptions transfer, Java.Interop.JniObjectReference throwableOverride) -> void
77
Java.Interop.JavaException.SetJavaStackTrace(Java.Interop.JniObjectReference peerReferenceOverride = default(Java.Interop.JniObjectReference)) -> void
8+
Java.Interop.JniRuntime.JniValueManager.GetPeer(Java.Interop.JniObjectReference reference, System.Type? targetType = null) -> Java.Interop.IJavaPeerable?
89
Java.Interop.JniTypeSignatureAttribute.InvokerType.get -> System.Type?
910
Java.Interop.JniTypeSignatureAttribute.InvokerType.set -> void

Diff for: tests/Java.Interop-Tests/Java.Interop-Tests.csproj

+2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
<PropertyGroup>
44
<TargetFramework>$(DotNetTargetFramework)</TargetFramework>
55
<IsPackable>false</IsPackable>
6+
<SignAssembly>true</SignAssembly>
7+
<AssemblyOriginatorKeyFile>..\..\product.snk</AssemblyOriginatorKeyFile>
68
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
79
<DefineConstants>$(DefineConstants);NO_MARSHAL_MEMBER_BUILDER_SUPPORT;NO_GC_BRIDGE_SUPPORT</DefineConstants>
810
</PropertyGroup>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,297 @@
1+
#nullable enable
2+
3+
using System;
4+
using System.Collections.Generic;
5+
using System.Linq;
6+
using System.Reflection;
7+
using System.Threading;
8+
9+
using Java.Interop;
10+
11+
using NUnit.Framework;
12+
13+
namespace Java.InteropTests {
14+
15+
// Android doesn't support `[NonParallelizable]`, but runs tests sequentially by default.
16+
#if !__ANDROID__
17+
// Modifies JniRuntime.valueManager instance field; can't be done in parallel
18+
[NonParallelizable]
19+
#endif // !__ANDROID__
20+
public abstract class JniRuntimeJniValueManagerContract : JavaVMFixture {
21+
22+
protected abstract Type ValueManagerType {
23+
get;
24+
}
25+
26+
protected virtual JniRuntime.JniValueManager CreateValueManager ()
27+
{
28+
var manager = Activator.CreateInstance (ValueManagerType) as JniRuntime.JniValueManager;
29+
return manager ?? throw new InvalidOperationException ($"Could not create instance of `{ValueManagerType}`!");
30+
}
31+
32+
#pragma warning disable CS8618
33+
JniRuntime.JniValueManager systemManager;
34+
JniRuntime.JniValueManager valueManager;
35+
#pragma warning restore CS8618
36+
37+
[SetUp]
38+
public void CreateVM ()
39+
{
40+
systemManager = JniRuntime.CurrentRuntime.valueManager!;
41+
valueManager = CreateValueManager ();
42+
valueManager.OnSetRuntime (JniRuntime.CurrentRuntime);
43+
JniRuntime.CurrentRuntime.valueManager = valueManager;
44+
}
45+
46+
[TearDown]
47+
public void DestroyVM ()
48+
{
49+
JniRuntime.CurrentRuntime.valueManager = systemManager;
50+
systemManager = null!;
51+
valueManager?.Dispose ();
52+
valueManager = null!;
53+
}
54+
55+
[Test]
56+
public void AddPeer ()
57+
{
58+
}
59+
60+
int GetSurfacedPeersCount ()
61+
{
62+
return valueManager.GetSurfacedPeers ().Count;
63+
}
64+
65+
[Test]
66+
public void AddPeer_NoDuplicates ()
67+
{
68+
int startPeerCount = GetSurfacedPeersCount ();
69+
using (var v = new MyDisposableObject ()) {
70+
// MyDisposableObject ctor implicitly calls AddPeer();
71+
Assert.AreEqual (startPeerCount + 1, GetSurfacedPeersCount (), DumpPeers ());
72+
valueManager.AddPeer (v);
73+
Assert.AreEqual (startPeerCount + 1, GetSurfacedPeersCount (), DumpPeers ());
74+
}
75+
}
76+
77+
[Test]
78+
public void ConstructPeer_ImplicitViaBindingConstructor_PeerIsInSurfacedPeers ()
79+
{
80+
int startPeerCount = GetSurfacedPeersCount ();
81+
82+
var g = new GetThis ();
83+
var surfaced = valueManager.GetSurfacedPeers ();
84+
Assert.AreEqual (startPeerCount + 1, surfaced.Count);
85+
86+
var found = false;
87+
foreach (var pr in surfaced) {
88+
if (!pr.SurfacedPeer.TryGetTarget (out var p))
89+
continue;
90+
if (object.ReferenceEquals (g, p)) {
91+
found = true;
92+
}
93+
}
94+
Assert.IsTrue (found);
95+
96+
var localRef = g.PeerReference.NewLocalRef ();
97+
g.Dispose ();
98+
Assert.AreEqual (startPeerCount, GetSurfacedPeersCount ());
99+
Assert.IsNull (valueManager.PeekPeer (localRef));
100+
JniObjectReference.Dispose (ref localRef);
101+
}
102+
103+
[Test]
104+
public void ConstructPeer_ImplicitViaBindingMethod_PeerIsInSurfacedPeers ()
105+
{
106+
int startPeerCount = GetSurfacedPeersCount ();
107+
108+
var g = new GetThis ();
109+
var surfaced = valueManager.GetSurfacedPeers ();
110+
Assert.AreEqual (startPeerCount + 1, surfaced.Count);
111+
112+
var found = false;
113+
foreach (var pr in surfaced) {
114+
if (!pr.SurfacedPeer.TryGetTarget (out var p))
115+
continue;
116+
if (object.ReferenceEquals (g, p)) {
117+
found = true;
118+
}
119+
}
120+
Assert.IsTrue (found);
121+
122+
var localRef = g.PeerReference.NewLocalRef ();
123+
g.Dispose ();
124+
Assert.AreEqual (startPeerCount, GetSurfacedPeersCount ());
125+
Assert.IsNull (valueManager.PeekPeer (localRef));
126+
JniObjectReference.Dispose (ref localRef);
127+
}
128+
129+
130+
[Test]
131+
public void CollectPeers ()
132+
{
133+
// TODO
134+
}
135+
136+
[Test]
137+
public void CreateValue ()
138+
{
139+
using (var o = new JavaObject ()) {
140+
var r = o.PeerReference;
141+
var x = (IJavaPeerable) valueManager.CreateValue (ref r, JniObjectReferenceOptions.Copy)!;
142+
Assert.AreNotSame (o, x);
143+
x.Dispose ();
144+
145+
x = valueManager.CreateValue<IJavaPeerable> (ref r, JniObjectReferenceOptions.Copy);
146+
Assert.AreNotSame (o, x);
147+
x!.Dispose ();
148+
}
149+
}
150+
151+
[Test]
152+
public void GetValue_ReturnsAlias ()
153+
{
154+
var local = new JavaObject ();
155+
local.UnregisterFromRuntime ();
156+
Assert.IsNull (valueManager.PeekValue (local.PeerReference));
157+
// GetObject must always return a value (unless handle is null, etc.).
158+
// However, since we called local.UnregisterFromRuntime(),
159+
// JniRuntime.PeekObject() is null (asserted above), but GetObject() must
160+
// **still** return _something_.
161+
// In this case, it returns an _alias_.
162+
// TODO: "most derived type" alias generation. (Not relevant here, but...)
163+
var p = local.PeerReference;
164+
var alias = JniRuntime.CurrentRuntime.ValueManager.GetValue<IJavaPeerable> (ref p, JniObjectReferenceOptions.Copy);
165+
Assert.AreNotSame (local, alias);
166+
alias!.Dispose ();
167+
local.Dispose ();
168+
}
169+
170+
[Test]
171+
public void GetValue_ReturnsNullWithNullHandle ()
172+
{
173+
var r = new JniObjectReference ();
174+
var o = valueManager.GetValue (ref r, JniObjectReferenceOptions.Copy);
175+
Assert.IsNull (o);
176+
}
177+
178+
[Test]
179+
public void GetValue_ReturnsNullWithInvalidSafeHandle ()
180+
{
181+
var invalid = new JniObjectReference ();
182+
Assert.IsNull (valueManager.GetValue (ref invalid, JniObjectReferenceOptions.CopyAndDispose));
183+
}
184+
185+
[Test]
186+
public unsafe void GetValue_FindBestMatchType ()
187+
{
188+
#if !NO_MARSHAL_MEMBER_BUILDER_SUPPORT
189+
using (var t = new JniType (TestType.JniTypeName)) {
190+
var c = t.GetConstructor ("()V");
191+
var o = t.NewObject (c, null);
192+
using (var w = valueManager.GetValue<IJavaPeerable> (ref o, JniObjectReferenceOptions.CopyAndDispose)) {
193+
Assert.AreEqual (typeof (TestType), w!.GetType ());
194+
Assert.IsTrue (((TestType) w).ExecutedActivationConstructor);
195+
}
196+
}
197+
#endif // !NO_MARSHAL_MEMBER_BUILDER_SUPPORT
198+
}
199+
200+
[Test]
201+
public void PeekPeer ()
202+
{
203+
Assert.IsNull (valueManager.PeekPeer (new JniObjectReference ()));
204+
205+
using (var v = new MyDisposableObject ()) {
206+
Assert.IsNotNull (valueManager.PeekPeer (v.PeerReference));
207+
Assert.AreSame (v, valueManager.PeekPeer (v.PeerReference));
208+
}
209+
}
210+
211+
[Test]
212+
public void PeekValue ()
213+
{
214+
JniObjectReference lref;
215+
using (var o = new JavaObject ()) {
216+
lref = o.PeerReference.NewLocalRef ();
217+
Assert.AreSame (o, valueManager.PeekValue (lref));
218+
}
219+
// At this point, the Java-side object is kept alive by `lref`,
220+
// but the wrapper instance has been disposed, and thus should
221+
// be unregistered, and thus unfindable.
222+
Assert.IsNull (valueManager.PeekValue (lref));
223+
JniObjectReference.Dispose (ref lref);
224+
}
225+
226+
[Test]
227+
public void PeekValue_BoxedObjects ()
228+
{
229+
var marshaler = valueManager.GetValueMarshaler<object> ();
230+
var ad = AppDomain.CurrentDomain;
231+
232+
var proxy = marshaler.CreateGenericArgumentState (ad);
233+
Assert.AreSame (ad, valueManager.PeekValue (proxy.ReferenceValue));
234+
marshaler.DestroyGenericArgumentState (ad, ref proxy);
235+
236+
var ex = new InvalidOperationException ("boo!");
237+
proxy = marshaler.CreateGenericArgumentState (ex);
238+
Assert.AreSame (ex, valueManager.PeekValue (proxy.ReferenceValue));
239+
marshaler.DestroyGenericArgumentState (ex, ref proxy);
240+
}
241+
242+
void AllNestedRegistrationScopeTests ()
243+
{
244+
AddPeer ();
245+
AddPeer_NoDuplicates ();
246+
ConstructPeer_ImplicitViaBindingConstructor_PeerIsInSurfacedPeers ();
247+
CreateValue ();
248+
GetValue_FindBestMatchType ();
249+
GetValue_ReturnsAlias ();
250+
GetValue_ReturnsNullWithInvalidSafeHandle ();
251+
GetValue_ReturnsNullWithNullHandle ();
252+
PeekPeer ();
253+
PeekValue ();
254+
PeekValue_BoxedObjects ();
255+
}
256+
257+
string DumpPeers ()
258+
{
259+
return DumpPeers (valueManager.GetSurfacedPeers ());
260+
}
261+
262+
static string DumpPeers (IEnumerable<JniSurfacedPeerInfo> peers)
263+
{
264+
return string.Join ("," + Environment.NewLine, peers);
265+
}
266+
267+
268+
// also test:
269+
// Singleton scenario
270+
// Types w/o "activation" constructors -- need to support checking parent scopes
271+
// nesting of scopes
272+
// Adding an instance already added in a previous scope?
273+
}
274+
275+
public abstract class JniRuntimeJniValueManagerContract<T> : JniRuntimeJniValueManagerContract {
276+
277+
protected override Type ValueManagerType => typeof (T);
278+
}
279+
280+
#if !__ANDROID__
281+
#if !NETCOREAPP
282+
[TestFixture]
283+
public class JniRuntimeJniValueManagerContract_Mono : JniRuntimeJniValueManagerContract {
284+
static Type MonoRuntimeValueManagerType = Type.GetType ("Java.Interop.MonoRuntimeValueManager, Java.Runtime.Environment", throwOnError:true)!;
285+
286+
protected override Type ValueManagerType => MonoRuntimeValueManagerType;
287+
}
288+
#endif // !NETCOREAPP
289+
290+
[TestFixture]
291+
public class JniRuntimeJniValueManagerContract_NoGCIntegration : JniRuntimeJniValueManagerContract {
292+
static Type ManagedValueManagerType = Type.GetType ("Java.Interop.ManagedValueManager, Java.Runtime.Environment", throwOnError:true)!;
293+
294+
protected override Type ValueManagerType => ManagedValueManagerType;
295+
}
296+
#endif // !__ANDROID__
297+
}

Diff for: tests/TestJVM/TestJVM.csproj

+2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
<TargetFramework>$(DotNetTargetFramework)</TargetFramework>
55
<Nullable>enable</Nullable>
66
<IsPackable>false</IsPackable>
7+
<SignAssembly>true</SignAssembly>
8+
<AssemblyOriginatorKeyFile>..\..\product.snk</AssemblyOriginatorKeyFile>
79
</PropertyGroup>
810

911
<Import Project="..\..\TargetFrameworkDependentValues.props" />

0 commit comments

Comments
 (0)