[Java.Interop] Java.Lang.Object, Mono.Android Unification Changes (#1293)

Context: dotnet/android#9640
Context: 25de1f3
Context: 3043d89dotnet/android#9640 completes "unification" with dotnet/java-interop,
updating `Java.Lang.Object` to inherit `Java.Interop.JavaObject`,
just like `src/Java.Base`.
The biggest impediment with this unification step is that the
semantics for referring to a Java instance are quite different:
* .NET for Android née Xamarin.Android née Mono for Android uses
an `(IntPtr, JniHandleOwnership)` pair to specify the JNI object
reference pointer value and what should be done with it.
The *type* of the JNI object reference is *inferred* by the
`JniHandleOwnership` value, e.g.
`JniHandleOwnership.TransferLocalRef` means that the `IntPtr`
value is a JNI local reference.
* dotnet/java-interop uses a
`(ref JniObjectReference, JniObjectReferenceOptions)` pair to
specify the JNI object reference and what can be done with it.
The `JniObjectReference` value contains the type, but -- due
to "pointer trickery"; see 25de1f3 -- it might be *so* invalid
that it cannot be touched at all, and `JniObjectReferenceOptions`
will let us know.
Which brings us to the conundrum: how do we implement the
`Java.Lang.Throwable(IntPtr, JniHandleOwnership)` constructor?
namespace Java.Lang {
public class Throwable {
public Throwable(IntPtr handle, JniHandleOwnership transfer);
}
}
The "obvious and wrong" solution would be to use `ref` with a
temporary…
public Throwable(IntPtr handle, JniHandleOwnership transfer)
: base (ref new JniObjectReference(handle), …)
{
}
…but that won't compile.
Additionally, we want to be able to provide `message` and
`innerException` parameters to `System.Exception`, but the
`JavaException(ref JniObjectReference, JniObjectReferenceOptions)`
constructor requires a valid JNI Object Reference to do that.
If `Java.Lang.Throwable` tries to "work around" this by using the
`JavaException(string, Exception)` constructor:
public Throwable(IntPtr handle, JniHandleOwnership transfer)
: base (_GetMessage (handle), _GetInnerException (handle))
{
…
}
then the `JavaException(string, Exception)` constructor will try to
invoke the `E(String)` constructor on the Java side, which:
1. Is semantically wrong, because we may already have a Java
instance in `handle`, so why are we creating a new instance? But-
2. This fails for constructor subclasses which don't provide a
`E(String)` constructor! Specifically, the
[`JnienvTest.ActivatedDirectThrowableSubclassesShouldBeRegistered`][0]
unit test starts crashing for "bizarre" reasons.
Fix these issues by adding the new `JavaException` constructor:
namespace Java.Interop {
partial class JavaException {
protected JavaException (
ref JniObjectReference reference,
JniObjectReferenceOptions transfer,
JniObjectReference throwableOverride);
}
}
The new `throwableOverride` parameter is used to obtain the `message`
and `innerException` values to provide to the
`System.Exception(string, Exception)` constructor, allowing
`reference` to be a "do not touch" value.
Additionally, add a `protected SetJavaStackTrace()` method which
is used to set the `JavaException.JavaStackTrace` property based on
a `JniObjectReference` value.
Update `Java.Interop.JavaObject` to be `[Serializable]`, and to mark
its fields as *`[NonSerialized]`*. This is needed to fix the `InstallAndRunTests.JsonDeserializationCreatesJavaHandle()` tests:
E AndroidRuntime: android.runtime.JavaProxyThrowable: [System.Runtime.Serialization.InvalidDataContractException]: AttributedTypesCannotInheritFromNonAttributedSerializableTypes, Java.Lang.Object, Java.Interop.JavaObject
Finally, update
`Java.Interop.JniRuntime.JniValueManager.ConstructPeer()` to *not*
attempt to call `AddPeer()`/etc. when `peer.JniManagedPeerState`
has `JniManagedPeerStates.Activatable` set. Failure to do so was
causing `BindingTests.JavaSideActivation()` to fail, as it was
"losing" a GREF. `JavaSideActivation()` is the scenario described
in 3043d89, in which:
1. The static Java method `CallMethodFromCtor.NewInstance()` creates
an instance of `ConstructorTest`, from Java.
2. The default constructor in the Java Callable Wrapper (JCW) for
`ConstructorTest` chains to the Java `CallMethodFromCtor()` default
constructor.
3. The Java `CallMethodFromCtor()` default constructor invokes the
method `calledFromCtor()`, which is overridden in the C#
`ConstructorTest.CalledFromCtor()` method.
4. This causes the `CallMethodFromCtor.n_CalledFromCtor()` marshal
method to be invoked, which calls
`Java.Lang.Object.GetObject<CallMethodFromCtor>()`, which is akin
to `runtime.ValueManager.GetValue<CallMethodFromCtor>(…)`.
5. `Object.GetObject<CallMethodFromCtor>()` doesn't find an already
existing object mapping for the instance we're still trying to
construct, so it creates a "proxy" object via
[`TypeManager.CreateInstance()`][2], which winds up invoking the
`ConstructorTest(IntPtr, JniHandleOwnership)` "activation"
constructor to create the "proxy". The activation constructor
eventually invokes `JavaObject.Construct()` /
`JniRuntime.JniValueManager.ConstructPeer()`.
This is the first "important" GREF created.
Once the proxy is created, it's state is set to
`JniManagedPeerStates.Activatable`.
6. The `ConstructorTest.CalledFromCtor()` C# override is invoked,
control returns to the Java `CallMethodFromCtor` constructor,
which completes, allowing the JCW `ConstructorTest` constructor
to execute, which calls [`TypeManager.n_Activate(…)`][3], which
is equivalent to `ManagedPeer.Construct()`.
7. `TypeManager.n_Activate(…)` sees that the instance
"needs activation", and invokes the
C# `ConstructorTest()` default constructor. This eventually hits
(again) `JniRuntime.JniValueManager.ConstructPeer()`.
The problem was that on the second
`JniRuntime.JniValueManager.ConstructPeer()` invocation in (7),
another GREF was created, replacing the GREF created in (5), but
without *disposing* of the GREF created in (5). This caused a GREF
leak, which the `JavaSideActivation()` test was looking for.
Updating `JniRuntime.JniValueManager.ConstructPeer()` to bail when
it encounters a Peer with `JniManagedPeerStates.Activatable` avoids
the GREF creation in (7), thus avoiding the GREF leak.
[0]: https://github.com/dotnet/android/blob/f9f421b109b2a0f8f805ed14ab1c8dbe651f563e/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs#L285-L305
[1]: https://github.com/dotnet/android/blob/main/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs#L97-L115
[2]: https://github.com/dotnet/android/blob/f9f421b109b2a0f8f805ed14ab1c8dbe651f563e/src/Mono.Android/Java.Interop/TypeManager.cs#L240-L332
[3]: https://github.com/dotnet/android/blob/f9f421b109b2a0f8f805ed14ab1c8dbe651f563e/src/Mono.Android/Java.Interop/TypeManager.cs#L132-L167

Author: jonpryor

src/Java.Interop/Java.Interop/JavaException.cs

@@ -38,7 +38,7 @@ public unsafe JavaException ()
 varpeer=JniPeerMembers.InstanceMethods.StartCreateInstance("()V",GetType(),null);
 Construct(refpeer,JniObjectReferenceOptions.CopyAndDispose);
 JniPeerMembers.InstanceMethods.FinishCreateInstance("()V",this,null);
-JavaStackTrace=GetJavaStack(PeerReference);
+SetJavaStackTrace();
 }
 
 publicunsafeJavaException(stringmessage)
@@ -55,7 +55,7 @@ public unsafe JavaException (string message)
 }finally{
 JniObjectReference.Dispose(refnative_message,JniObjectReferenceOptions.CopyAndDispose);
 }
-JavaStackTrace=GetJavaStack(PeerReference);
+SetJavaStackTrace();
 }
 
 publicunsafeJavaException(stringmessage,ExceptioninnerException)
@@ -72,15 +72,22 @@ public unsafe JavaException (string message, Exception innerException)
 }finally{
 JniObjectReference.Dispose(refnative_message,JniObjectReferenceOptions.CopyAndDispose);
 }
-JavaStackTrace=GetJavaStack(PeerReference);
+SetJavaStackTrace();
+}
+
+protectedJavaException(refJniObjectReferencereference,JniObjectReferenceOptionstransfer,JniObjectReferencethrowableOverride)
+:base(GetMessage(throwableOverride),GetCause(throwableOverride))
+{
+Construct(refreference,transfer);
+SetJavaStackTrace(throwableOverride);
 }
 
 publicJavaException(refJniObjectReferencereference,JniObjectReferenceOptionstransfer)
 :base(GetMessage(refreference,transfer),GetCause(refreference,transfer))
 {
 Construct(refreference,transfer);
 if(PeerReference.IsValid)
-JavaStackTrace=GetJavaStack(PeerReference);
+SetJavaStackTrace();
 }
 
 protectedvoidConstruct(refJniObjectReferencereference,JniObjectReferenceOptionsoptions)
@@ -183,6 +190,13 @@ public override unsafe int GetHashCode ()
 {
 if(transfer==JniObjectReferenceOptions.None)
 returnnull;
+returnGetMessage(reference);
+}
+
+staticstring?GetMessage(JniObjectReferencereference)
+{
+if(!reference.IsValid)
+returnnull;
 
 varm=_members.InstanceMethods.GetMethodInfo("getMessage.()Ljava/lang/String;");
 vars=JniEnvironment.InstanceMethods.CallObjectMethod(reference,m);
@@ -193,12 +207,30 @@ public override unsafe int GetHashCode ()
 {
 if(transfer==JniObjectReferenceOptions.None)
 returnnull;
+returnGetCause(reference);
+}
+
+staticException?GetCause(JniObjectReferencereference)
+{
+if(!reference.IsValid)
+returnnull;
 
 varm=_members.InstanceMethods.GetMethodInfo("getCause.()Ljava/lang/Throwable;");
 vare=JniEnvironment.InstanceMethods.CallObjectMethod(reference,m);
 returnJniEnvironment.Runtime.GetExceptionForThrowable(refe,JniObjectReferenceOptions.CopyAndDispose);
 }
 
+protectedvoidSetJavaStackTrace(JniObjectReferencepeerReferenceOverride=default)
+{
+if(!peerReferenceOverride.IsValid){
+peerReferenceOverride=PeerReference;
+}
+if(!peerReferenceOverride.IsValid){
+return;
+}
+JavaStackTrace=GetJavaStack(peerReferenceOverride);
+}
+
 unsafestring?GetJavaStack(JniObjectReferencehandle)
 {
 using(varStringWriter_class=newJniType("java/io/StringWriter"))

src/Java.Interop/Java.Interop/JavaObject.cs

@@ -2,29 +2,35 @@
 
 usingSystem;
 usingSystem.Diagnostics.CodeAnalysis;
+usingSystem.Runtime.Serialization;
 
 namespaceJava.Interop
 {
 [JniTypeSignature("java/lang/Object",GenerateJavaPeer=false)]
+[Serializable]
 unsafepublicclassJavaObject:IJavaPeerable
 {
 internalconstDynamicallyAccessedMemberTypesConstructors=DynamicallyAccessedMemberTypes.PublicConstructors|DynamicallyAccessedMemberTypes.NonPublicConstructors;
 
 readonlystaticJniPeerMembers_members=newJniPeerMembers("java/lang/Object",typeof(JavaObject));
 
-publicintJniIdentityHashCode{get;privateset;}
-publicJniManagedPeerStatesJniManagedPeerState{get;privateset;}
+[NonSerialized]intidentityHashCode;
+[NonSerialized]JniManagedPeerStatesmanagedPeerState;
+
+publicintJniIdentityHashCode=>identityHashCode;
+
+publicJniManagedPeerStatesJniManagedPeerState=>managedPeerState;
 
 #if FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES
-JniObjectReferencereference;
+[NonSerialized]JniObjectReferencereference;
 #endif // FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES
 #if FEATURE_JNIOBJECTREFERENCE_INTPTRS
-IntPtrhandle;
-JniObjectReferenceTypehandle_type;
+[NonSerialized]IntPtrhandle;
+[NonSerialized]JniObjectReferenceTypehandle_type;
 	#pragma warning disable 0169
 // Used by JavaInteropGCBridge
-IntPtrweak_handle;
-intrefs_added;
+[NonSerialized]IntPtrweak_handle;
+[NonSerialized]intrefs_added;
 	#pragma warning restore 0169
 #endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS
 
@@ -151,12 +157,12 @@ void IJavaPeerable.Finalized ()
 
 voidIJavaPeerable.SetJniIdentityHashCode(intvalue)
 {
-JniIdentityHashCode=value;
+identityHashCode=value;
 }
 
 voidIJavaPeerable.SetJniManagedPeerState(JniManagedPeerStatesvalue)
 {
-JniManagedPeerState=value;
+managedPeerState=value;
 }
 
 voidIJavaPeerable.SetPeerReference(JniObjectReferencereference)

src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs

@@ -97,8 +97,13 @@ public void ConstructPeer (IJavaPeerable peer, ref JniObjectReference reference,
 
 varnewRef=peer.PeerReference;
 if(newRef.IsValid){
-// Activation! See ManagedPeer.RunConstructor
-peer.SetJniManagedPeerState(peer.JniManagedPeerState|JniManagedPeerStates.Activatable);
+JniObjectReference.Dispose(refreference,options);
+
+// Activation? See ManagedPeer.Construct, CreatePeer
+// Instance was already added, don't add again
+if(peer.JniManagedPeerState.HasFlag(JniManagedPeerStates.Activatable)){
+return;
+}
 JniObjectReference.Dispose(refreference,options);
 newRef=newRef.NewGlobalRef();
 }elseif(options==JniObjectReferenceOptions.None){

src/Java.Interop/PublicAPI.Unshipped.txt

@@ -3,5 +3,7 @@ static Java.Interop.JniEnvironment.BeginMarshalMethod(nint jnienv, out Java.Inte
 static Java.Interop.JniEnvironment.EndMarshalMethod(ref Java.Interop.JniTransition transition) -> void
 virtual Java.Interop.JniRuntime.OnEnterMarshalMethod() -> void
 virtual Java.Interop.JniRuntime.OnUserUnhandledException(ref Java.Interop.JniTransition transition, System.Exception! e) -> void
+Java.Interop.JavaException.JavaException(ref Java.Interop.JniObjectReference reference, Java.Interop.JniObjectReferenceOptions transfer, Java.Interop.JniObjectReference throwableOverride) -> void
+Java.Interop.JavaException.SetJavaStackTrace(Java.Interop.JniObjectReference peerReferenceOverride = default(Java.Interop.JniObjectReference)) -> void
 Java.Interop.JniTypeSignatureAttribute.InvokerType.get -> System.Type?
 Java.Interop.JniTypeSignatureAttribute.InvokerType.set -> void