From bc0d5feef3683fcaa54a2bff5531fb191fccc319 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 14 Jan 2025 18:43:40 -0500 Subject: [PATCH 1/5] [Java.Interop] JavaException supports param override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/dotnet/android/pull/9640 Context: 25de1f38bb6b3ef2d4c98d2d95923a4bd50d2ea0 dotnet/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 25de1f38 -- 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. Next, 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 exception: 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. [0]: https://github.com/dotnet/android/blob/main/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs#L285-L305 --- .../Java.Interop/JavaException.cs | 40 +++++++++++++++++-- src/Java.Interop/PublicAPI.Unshipped.txt | 2 + 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/Java.Interop/Java.Interop/JavaException.cs b/src/Java.Interop/Java.Interop/JavaException.cs index 0bfa3fd58..b07735b5a 100644 --- a/src/Java.Interop/Java.Interop/JavaException.cs +++ b/src/Java.Interop/Java.Interop/JavaException.cs @@ -38,7 +38,7 @@ public unsafe JavaException () var peer = JniPeerMembers.InstanceMethods.StartCreateInstance ("()V", GetType (), null); Construct (ref peer, JniObjectReferenceOptions.CopyAndDispose); JniPeerMembers.InstanceMethods.FinishCreateInstance ("()V", this, null); - JavaStackTrace = GetJavaStack (PeerReference); + SetJavaStackTrace (); } public unsafe JavaException (string message) @@ -55,7 +55,7 @@ public unsafe JavaException (string message) } finally { JniObjectReference.Dispose (ref native_message, JniObjectReferenceOptions.CopyAndDispose); } - JavaStackTrace = GetJavaStack (PeerReference); + SetJavaStackTrace (); } public unsafe JavaException (string message, Exception innerException) @@ -72,7 +72,14 @@ public unsafe JavaException (string message, Exception innerException) } finally { JniObjectReference.Dispose (ref native_message, JniObjectReferenceOptions.CopyAndDispose); } - JavaStackTrace = GetJavaStack (PeerReference); + SetJavaStackTrace (); + } + + protected JavaException (ref JniObjectReference reference, JniObjectReferenceOptions transfer, JniObjectReference throwableOverride) + : base (GetMessage (throwableOverride), GetCause (throwableOverride)) + { + Construct (ref reference, transfer); + SetJavaStackTrace (throwableOverride); } public JavaException (ref JniObjectReference reference, JniObjectReferenceOptions transfer) @@ -80,7 +87,7 @@ public JavaException (ref JniObjectReference reference, JniObjectReferenceOption { Construct (ref reference, transfer); if (PeerReference.IsValid) - JavaStackTrace = GetJavaStack (PeerReference); + SetJavaStackTrace (); } protected void Construct (ref JniObjectReference reference, JniObjectReferenceOptions options) @@ -183,6 +190,13 @@ public override unsafe int GetHashCode () { if (transfer == JniObjectReferenceOptions.None) return null; + return GetMessage (reference); + } + + static string? GetMessage (JniObjectReference reference) + { + if (!reference.IsValid) + return null; var m = _members.InstanceMethods.GetMethodInfo ("getMessage.()Ljava/lang/String;"); var s = JniEnvironment.InstanceMethods.CallObjectMethod (reference, m); @@ -193,12 +207,30 @@ public override unsafe int GetHashCode () { if (transfer == JniObjectReferenceOptions.None) return null; + return GetCause (reference); + } + + static Exception? GetCause (JniObjectReference reference) + { + if (!reference.IsValid) + return null; var m = _members.InstanceMethods.GetMethodInfo ("getCause.()Ljava/lang/Throwable;"); var e = JniEnvironment.InstanceMethods.CallObjectMethod (reference, m); return JniEnvironment.Runtime.GetExceptionForThrowable (ref e, JniObjectReferenceOptions.CopyAndDispose); } + protected void SetJavaStackTrace (JniObjectReference peerReferenceOverride = default) + { + if (!peerReferenceOverride.IsValid) { + peerReferenceOverride = PeerReference; + } + if (!peerReferenceOverride.IsValid) { + return; + } + JavaStackTrace = GetJavaStack (peerReferenceOverride); + } + unsafe string? GetJavaStack (JniObjectReference handle) { using (var StringWriter_class = new JniType ("java/io/StringWriter")) diff --git a/src/Java.Interop/PublicAPI.Unshipped.txt b/src/Java.Interop/PublicAPI.Unshipped.txt index e0f851bb0..6a17b2a53 100644 --- a/src/Java.Interop/PublicAPI.Unshipped.txt +++ b/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.JniObjectReferenceOptions throwableOverride) -> void +Java.Interop.JavaException.SetJavaStackTrace(Java.Interop.JniObjectReference peerReferenceOverride) -> void Java.Interop.JniTypeSignatureAttribute.InvokerType.get -> System.Type? Java.Interop.JniTypeSignatureAttribute.InvokerType.set -> void From 585cd421262e70816a404524c5f3ba7c9c929ab1 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 14 Jan 2025 20:05:34 -0500 Subject: [PATCH 2/5] Fix PublicAPI*.txt --- src/Java.Interop/PublicAPI.Unshipped.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Java.Interop/PublicAPI.Unshipped.txt b/src/Java.Interop/PublicAPI.Unshipped.txt index 6a17b2a53..d4be03231 100644 --- a/src/Java.Interop/PublicAPI.Unshipped.txt +++ b/src/Java.Interop/PublicAPI.Unshipped.txt @@ -3,7 +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.JniObjectReferenceOptions throwableOverride) -> void -Java.Interop.JavaException.SetJavaStackTrace(Java.Interop.JniObjectReference peerReferenceOverride) -> 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 From 1f7f1257335b0c392675bc6570b23a8e117a18e1 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 14 Jan 2025 22:03:02 -0500 Subject: [PATCH 3/5] Add [Serializable] to JavaObject Needed to fix `JsonDeserializationCreatesJavaHandle()` test: E AndroidRuntime: android.runtime.JavaProxyThrowable: [System.Runtime.Serialization.InvalidDataContractException]: AttributedTypesCannotInheritFromNonAttributedSerializableTypes, Java.Lang.Object, Java.Interop.JavaObject --- src/Java.Interop/Java.Interop/JavaObject.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Java.Interop/Java.Interop/JavaObject.cs b/src/Java.Interop/Java.Interop/JavaObject.cs index 22e382fba..5d49343f6 100644 --- a/src/Java.Interop/Java.Interop/JavaObject.cs +++ b/src/Java.Interop/Java.Interop/JavaObject.cs @@ -6,6 +6,7 @@ namespace Java.Interop { [JniTypeSignature ("java/lang/Object", GenerateJavaPeer=false)] + [Serializable] unsafe public class JavaObject : IJavaPeerable { internal const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors; From 9d1cae1ae74a9a9d36b0f3f528fe50bc3c2ee818 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 15 Jan 2025 11:37:22 -0500 Subject: [PATCH 4/5] [Java.Interop] JavaObject fields shouldn't be serialized Context: https://github.com/dotnet/android/pull/9640#issuecomment-2593392666 --- src/Java.Interop/Java.Interop/JavaObject.cs | 23 +++++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/Java.Interop/Java.Interop/JavaObject.cs b/src/Java.Interop/Java.Interop/JavaObject.cs index 5d49343f6..02a69e061 100644 --- a/src/Java.Interop/Java.Interop/JavaObject.cs +++ b/src/Java.Interop/Java.Interop/JavaObject.cs @@ -2,6 +2,7 @@ using System; using System.Diagnostics.CodeAnalysis; +using System.Runtime.Serialization; namespace Java.Interop { @@ -13,19 +14,23 @@ unsafe public class JavaObject : IJavaPeerable readonly static JniPeerMembers _members = new JniPeerMembers ("java/lang/Object", typeof (JavaObject)); - public int JniIdentityHashCode { get; private set; } - public JniManagedPeerStates JniManagedPeerState { get; private set; } + [NonSerialized] int identityHashCode; + [NonSerialized] JniManagedPeerStates managedPeerState; + + public int JniIdentityHashCode => identityHashCode; + + public JniManagedPeerStates JniManagedPeerState => managedPeerState; #if FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES - JniObjectReference reference; + [NonSerialized] JniObjectReference reference; #endif // FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES #if FEATURE_JNIOBJECTREFERENCE_INTPTRS - IntPtr handle; - JniObjectReferenceType handle_type; + [NonSerialized] IntPtr handle; + [NonSerialized] JniObjectReferenceType handle_type; #pragma warning disable 0169 // Used by JavaInteropGCBridge - IntPtr weak_handle; - int refs_added; + [NonSerialized] IntPtr weak_handle; + [NonSerialized] int refs_added; #pragma warning restore 0169 #endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS @@ -152,12 +157,12 @@ void IJavaPeerable.Finalized () void IJavaPeerable.SetJniIdentityHashCode (int value) { - JniIdentityHashCode = value; + identityHashCode = value; } void IJavaPeerable.SetJniManagedPeerState (JniManagedPeerStates value) { - JniManagedPeerState = value; + managedPeerState = value; } void IJavaPeerable.SetPeerReference (JniObjectReference reference) From 52e154a9e1a34bd8f17c5e8ab5284d7f6c96c09b Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Thu, 16 Jan 2025 10:54:46 -0500 Subject: [PATCH 5/5] Flushity FLush. TODO: why?! --- .../Java.Interop/JniRuntime.JniValueManager.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs b/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs index 207359e61..55af4f7b4 100644 --- a/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs +++ b/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs @@ -97,8 +97,13 @@ public void ConstructPeer (IJavaPeerable peer, ref JniObjectReference reference, var newRef = peer.PeerReference; if (newRef.IsValid) { - // Activation! See ManagedPeer.RunConstructor - peer.SetJniManagedPeerState (peer.JniManagedPeerState | JniManagedPeerStates.Activatable); + JniObjectReference.Dispose (ref reference, options); + + // Activation? See ManagedPeer.Construct, CreatePeer + // Instance was already added, don't add again + if (peer.JniManagedPeerState.HasFlag (JniManagedPeerStates.Activatable)) { + return; + } JniObjectReference.Dispose (ref reference, options); newRef = newRef.NewGlobalRef (); } else if (options == JniObjectReferenceOptions.None) {