Skip to content

Commit 5c23bcd

Browse files
authored
[Mono.Android] Java.Interop Unification! (#9640)
Context: #9636 Context: dotnet/java-interop@d5dfa0a Context: https://github.com/xamarin/monodroid/commit/e318861ed8eb20a71852378ddd558409d6b1c234 Context: 130905e Context: de04316 Changes: dotnet/java-interop@4f06201...d5dfa0a * dotnet/java-interop@d5dfa0aa: [Java.Interop] `Java.Lang.Object, Mono.Android` Unification Changes (dotnet/java-interop#1293) * dotnet/java-interop@c86ae26c: [ci] Fail build if any git tracked files were modified. (dotnet/java-interop#1288) In the beginning there was Mono for Android, which had a set of `Mono.Android.dll` assemblies (one per supported API level), each of which contained "duplicated" binding logic: each API level had its own `Java.Lang.Object`, `Android.Runtime.JNIEnv`, etc. dotnet/java-interop started, in part, as a way to "split out" the core integration logic, so that it *wouldn't* need to be duplicated across every assembly. As part of this, it introduced its own core abstractions, notably `Java.Interop.IJavaPeerable` and `Java.Interop.JavaObject`. When dotnet/java-interop was first introduced into Xamarin.Android, with xamarin/monodroid@e318861e, the integration was incomplete. Integration continued with 130905e, allowing unit tests within `Java.Interop-Tests.dll` to run within Xamarin.Android and construction of instances of e.g. `JavaInt32Array`, but one large piece of integration remained: Move GC bridge code *out* of `Java.Lang.Object`, and instead rely on `Java.Interop.JavaObject`, turning this: namespace Java.Lang { public partial class Object : System.Object, IJavaPeerable /* … */ { } } into this: namespace Java.Lang { public partial class Object : Java.Interop.JavaObject, IJavaPeerable /* … */ { } } *Why*? In part because @jonpryor has wanted to do this for literal years at this point, but also in part because of #9636 and related efforts to use Native AOT, which involves avoiding / bypassing `DllImportAttribute` invocations (for now, everything touched by Native AOT becomes a single `.so` binary, which we don't know the name of). Avoiding P/Invoke means *embracing* and extending existing Java.Interop constructs (e.g. de04316). In addition to altering the base types of `Java.Lang.Object` and `Java.Lang.Throwable`: * Remove `handle` and related fields from `Java.Lang.Object` and `Java.Lang.Throwable`. * Update `PreserveLists/Mono.Android.xml` et al. so that the removed fields are not preserved. * Rename `JNIenvInit.AndroidValueManager` to `JNIEnvInit.ValueManager`, and change its type to `JniRuntime.JniValueManager`. This is to help "force" usage of `JnIRuntime.JniValueManager` in more places, as we can't currently use `AndroidValueManager` in Native AOT (P/Invokes!). * Cleanup: Remove `JNIEnv.Exit()` and related code. These were used by the Android Designer, which is no longer supported. * Update (`internal`) interface `IJavaObjectEx` to remove constructs present on `IJavaPeerable.` * Update the `Java.Lang.Object(IntPtr, JniHandleOwnership)` and `Java.Lang.Throwable(IntPtr, JniHandleOwnership)` constructors to follow dotnet/java-interop convention, and patch over the differences that exist between the two paradigms. * Update `ExceptionTest.CompareStackTraces()` to use `System.Diagnostics.StackTrace(ex, fNeedFileInfo:true)` so that when the `debug.mono.debug` system property is set, the `ExceptionTest.InnerExceptionIsSet()` unit test doesn't fail. Also, increase assertion message utility. * Update `AndroidObjectReferenceManager` so that dotnet/java-interop -initiated JNI object reference log messages are appropriately captured. * Update `JNIEnv.IsGCUserPeer()` to also consider types which implement `net.dot.jni.GCUserPeerable` to be "GC User Peers".
1 parent f9f421b commit 5c23bcd

File tree

19 files changed

+123
-625
lines changed

19 files changed

+123
-625
lines changed

src/Microsoft.Android.Sdk.ILLink/PreserveLists/Mono.Android.xml

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
<type fullname="Android.Runtime.InputStreamAdapter" preserve="methods" />
1010
<type fullname="Android.Runtime.InputStreamInvoker" preserve="methods" />
1111
<type fullname="Android.Runtime.JNIEnv">
12-
<method name="Exit" />
1312
<method name="PropagateUncaughtException" />
1413
</type>
1514
<type fullname="Android.Runtime.JNIEnvInit">
@@ -36,15 +35,7 @@
3635
-->
3736
<type fullname="Java.Interop.TypeManager*JavaTypeManager" />
3837
<type fullname="Java.Lang.Object">
39-
<field name="handle" />
40-
<field name="handle_type" />
41-
<field name="refs_added" />
4238
<method name="SetHandleOnDeserialized" />
4339
</type>
44-
<type fullname="Java.Lang.Throwable">
45-
<field name="handle" />
46-
<field name="handle_type" />
47-
<field name="refs_added" />
48-
</type>
4940
</assembly>
5041
</linker>

src/Mono.Android/Android.Runtime/AndroidRuntime.cs

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,29 +56,36 @@ public override string GetCurrentManagedThreadStackTrace (int skipFrames, bool f
5656
{
5757
if (!reference.IsValid)
5858
return null;
59-
var peeked = JNIEnvInit.AndroidValueManager?.PeekPeer (reference);
59+
var peeked = JNIEnvInit.ValueManager?.PeekPeer (reference);
6060
var peekedExc = peeked as Exception;
6161
if (peekedExc == null) {
6262
var throwable = Java.Lang.Object.GetObject<Java.Lang.Throwable> (reference.Handle, JniHandleOwnership.DoNotTransfer);
6363
JniObjectReference.Dispose (ref reference, options);
6464
return throwable;
6565
}
6666
JniObjectReference.Dispose (ref reference, options);
67-
var unwrapped = JNIEnvInit.AndroidValueManager?.UnboxException (peeked!);
67+
var unwrapped = UnboxException (peeked!);
6868
if (unwrapped != null) {
6969
return unwrapped;
7070
}
7171
return peekedExc;
7272
}
7373

74+
Exception? UnboxException (IJavaPeerable value)
75+
{
76+
if (JNIEnvInit.ValueManager is AndroidValueManager vm) {
77+
return vm.UnboxException (value);
78+
}
79+
return null;
80+
}
81+
7482
public override void RaisePendingException (Exception pendingException)
7583
{
76-
var je = pendingException as JavaProxyThrowable;
84+
var je = pendingException as JavaException;
7785
if (je == null) {
7886
je = JavaProxyThrowable.Create (pendingException);
7987
}
80-
var r = new JniObjectReference (je.Handle);
81-
JniEnvironment.Exceptions.Throw (r);
88+
JniEnvironment.Exceptions.Throw (je.PeerReference);
8289
}
8390
}
8491

@@ -158,6 +165,14 @@ public override IntPtr ReleaseLocalReference (ref JniObjectReference value, ref
158165
return r;
159166
}
160167

168+
public override bool LogGlobalReferenceMessages => Logger.LogGlobalRef;
169+
public override bool LogLocalReferenceMessages => Logger.LogLocalRef;
170+
171+
public override void WriteLocalReferenceLine (string format, params object?[] args)
172+
{
173+
RuntimeNativeMethods._monodroid_gref_log ("[LREF] " + string.Format (CultureInfo.InvariantCulture, format, args));
174+
}
175+
161176
public override void WriteGlobalReferenceLine (string format, params object?[] args)
162177
{
163178
RuntimeNativeMethods._monodroid_gref_log (string.Format (CultureInfo.InvariantCulture, format, args));
@@ -470,6 +485,7 @@ static bool CallRegisterMethodByIndex (JniNativeMethodRegistrationArguments argu
470485
}
471486
}
472487

488+
[Obsolete ("Use RegisterNativeMembers(JniType, Type, ReadOnlySpan<char>) instead.")]
473489
public override void RegisterNativeMembers (
474490
JniType nativeClass,
475491
[DynamicallyAccessedMembers (MethodsAndPrivateNested)]

src/Mono.Android/Android.Runtime/JNIEnv.cs

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ internal static bool IsGCUserPeer (IntPtr value)
5252
if (value == IntPtr.Zero)
5353
return false;
5454

55-
return IsInstanceOf (value, JNIEnvInit.grefIGCUserPeer_class);
55+
return IsInstanceOf (value, JNIEnvInit.grefIGCUserPeer_class) ||
56+
IsInstanceOf (value, JNIEnvInit.grefGCUserPeerable_class);
5657
}
5758

5859
internal static bool ShouldWrapJavaException (Java.Lang.Throwable? t, [CallerMemberName] string? caller = null)
@@ -76,49 +77,6 @@ internal static bool ShouldWrapJavaException (Java.Lang.Throwable? t, [CallerMem
7677
return wrap;
7778
}
7879

79-
[DllImport ("libc")]
80-
static extern int gettid ();
81-
82-
internal static void Exit ()
83-
{
84-
/* Manually dispose surfaced objects and close the current JniEnvironment to
85-
* avoid ObjectDisposedException thrown on finalizer threads after shutdown
86-
*/
87-
foreach (var surfacedObject in Java.Interop.Runtime.GetSurfacedObjects ()) {
88-
try {
89-
var obj = surfacedObject.Target as IDisposable;
90-
if (obj != null)
91-
obj.Dispose ();
92-
continue;
93-
} catch (Exception e) {
94-
RuntimeNativeMethods.monodroid_log (LogLevel.Warn, LogCategories.Default, $"Couldn't dispose object: {e}");
95-
}
96-
/* If calling Dispose failed, the assumption is that user-code in
97-
* the Dispose(bool) overload is to blame for it. In that case we
98-
* fallback to manual deletion of the surfaced object.
99-
*/
100-
var jobj = surfacedObject.Target as Java.Lang.Object;
101-
if (jobj != null)
102-
ManualJavaObjectDispose (jobj);
103-
}
104-
JniEnvironment.Runtime.Dispose ();
105-
}
106-
107-
/* FIXME: This reproduces the minimal steps in Java.Lang.Object.Dispose
108-
* that needs to be executed so that we don't leak any GREF and prevent
109-
* code execution into an appdomain that we are disposing via a finalizer.
110-
* Ideally it should be done via another more generic mechanism, likely
111-
* from the Java.Interop.Runtime API.
112-
*/
113-
static void ManualJavaObjectDispose (Java.Lang.Object obj)
114-
{
115-
var peer = obj.PeerReference;
116-
var handle = peer.Handle;
117-
var keyHandle = ((IJavaObjectEx)obj).KeyHandle;
118-
Java.Lang.Object.Dispose (obj, ref handle, keyHandle, (JObjectRefType)peer.Type);
119-
GC.SuppressFinalize (obj);
120-
}
121-
12280
internal static void PropagateUncaughtException (IntPtr env, IntPtr javaThreadPtr, IntPtr javaExceptionPtr)
12381
{
12482
if (!JNIEnvInit.PropagateExceptions)

src/Mono.Android/Android.Runtime/JNIEnvInit.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,19 @@ internal struct JnienvInitializeArgs {
3232
public int jniAddNativeMethodRegistrationAttributePresent;
3333
public bool jniRemappingInUse;
3434
public bool marshalMethodsEnabled;
35+
public IntPtr grefGCUserPeerable;
3536
}
3637
#pragma warning restore 0649
3738

38-
internal static AndroidValueManager? AndroidValueManager;
39+
internal static JniRuntime.JniValueManager? ValueManager;
3940
internal static bool IsRunningOnDesktop;
4041
internal static bool jniRemappingInUse;
4142
internal static bool MarshalMethodsEnabled;
4243
internal static bool PropagateExceptions;
4344
internal static BoundExceptionType BoundExceptionType;
4445
internal static int gref_gc_threshold;
4546
internal static IntPtr grefIGCUserPeer_class;
47+
internal static IntPtr grefGCUserPeerable_class;
4648
internal static IntPtr java_class_loader;
4749
internal static JniMethodInfo? mid_Class_forName;
4850

@@ -94,11 +96,12 @@ internal static unsafe void Initialize (JnienvInitializeArgs* args)
9496

9597
BoundExceptionType = (BoundExceptionType)args->ioExceptionType;
9698
androidRuntime = new AndroidRuntime (args->env, args->javaVm, args->grefLoader, args->Loader_loadClass, args->jniAddNativeMethodRegistrationAttributePresent != 0);
97-
AndroidValueManager = (AndroidValueManager) androidRuntime.ValueManager;
99+
ValueManager = androidRuntime.ValueManager;
98100

99101
IsRunningOnDesktop = args->isRunningOnDesktop == 1;
100102

101103
grefIGCUserPeer_class = args->grefIGCUserPeer;
104+
grefGCUserPeerable_class = args->grefGCUserPeerable;
102105

103106
PropagateExceptions = args->brokenExceptionTransitions == 0;
104107

src/Mono.Android/Java.Interop/IJavaObjectEx.cs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,6 @@
33
namespace Java.Interop {
44

55
interface IJavaObjectEx {
6-
IntPtr KeyHandle {get; set;}
7-
bool IsProxy {get; set;}
8-
bool NeedsActivation {get; set;}
96
IntPtr ToLocalJniHandle ();
107
}
118
}

src/Mono.Android/Java.Interop/Runtime.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ public static class Runtime {
1111
[Obsolete ("Please use Java.Interop.JniEnvironment.Runtime.ValueManager.GetSurfacedPeers()")]
1212
public static List<WeakReference> GetSurfacedObjects ()
1313
{
14-
var peers = JNIEnvInit.AndroidValueManager!.GetSurfacedPeers ();
14+
var peers = JNIEnvInit.ValueManager!.GetSurfacedPeers ();
1515
var r = new List<WeakReference> (peers.Count);
1616
foreach (var p in peers) {
1717
if (p.SurfacedPeer.TryGetTarget (out var target))

src/Mono.Android/Java.Interop/TypeManager.cs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,10 @@ static Type[] GetParameterTypes (string? signature)
133133
static void n_Activate (IntPtr jnienv, IntPtr jclass, IntPtr typename_ptr, IntPtr signature_ptr, IntPtr jobject, IntPtr parameters_ptr)
134134
{
135135
var o = Java.Lang.Object.PeekObject (jobject);
136-
var ex = o as IJavaObjectEx;
136+
var ex = o as IJavaPeerable;
137137
if (ex != null) {
138-
if (!ex.NeedsActivation && !ex.IsProxy)
138+
var state = ex.JniManagedPeerState;
139+
if (!state.HasFlag (JniManagedPeerStates.Activatable) && !state.HasFlag (JniManagedPeerStates.Replaceable))
139140
return;
140141
}
141142
if (!ActivationEnabled) {
@@ -171,10 +172,8 @@ internal static void Activate (IntPtr jobject, ConstructorInfo cinfo, object? []
171172
{
172173
try {
173174
var newobj = RuntimeHelpers.GetUninitializedObject (cinfo.DeclaringType!);
174-
if (newobj is Java.Lang.Object o) {
175-
o.handle = jobject;
176-
} else if (newobj is Java.Lang.Throwable throwable) {
177-
throwable.handle = jobject;
175+
if (newobj is IJavaPeerable peer) {
176+
peer.SetPeerReference (new JniObjectReference (jobject));
178177
} else {
179178
throw new InvalidOperationException ($"Unsupported type: '{newobj}'");
180179
}
@@ -232,7 +231,7 @@ static Exception CreateJavaLocationException ()
232231
return null;
233232
}
234233

235-
internal static IJavaPeerable CreateInstance (IntPtr handle, JniHandleOwnership transfer)
234+
internal static IJavaPeerable? CreateInstance (IntPtr handle, JniHandleOwnership transfer)
236235
{
237236
return CreateInstance (handle, transfer, null);
238237
}
@@ -320,7 +319,7 @@ internal static IJavaPeerable CreateInstance (IntPtr handle, JniHandleOwnership
320319
try {
321320
result = (IJavaPeerable) CreateProxy (type, handle, transfer);
322321
if (Runtime.IsGCUserPeer (result.PeerReference.Handle)) {
323-
result.SetJniManagedPeerState (JniManagedPeerStates.Replaceable);
322+
result.SetJniManagedPeerState (JniManagedPeerStates.Replaceable | JniManagedPeerStates.Activatable);
324323
}
325324
} catch (MissingMethodException e) {
326325
var key_handle = JNIEnv.IdentityHash (handle);
@@ -353,7 +352,6 @@ internal static object CreateProxy (
353352
JniObjectReferenceOptions o = JniObjectReferenceOptions.Copy;
354353
var peer = (IJavaPeerable) c.Invoke (new object [] { r, o });
355354
JNIEnv.DeleteRef (handle, transfer);
356-
peer.SetJniManagedPeerState (peer.JniManagedPeerState | JniManagedPeerStates.Replaceable);
357355
return peer;
358356
}
359357
throw new MissingMethodException (

0 commit comments

Comments
 (0)