From 6f4ae42869c2e9e9a82eb20441af537dacf84e4f Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Fri, 8 Nov 2024 08:24:53 -0500 Subject: [PATCH] [Java.Interop] Add JniIdentityHashCode to ObjectDisposedException MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/dotnet/android/issues/9039 Context: https://github.com/dotnet/android/commit/32495f36905d0e5f203c0ed581824038286c58e2 @jonpryor suspects that the `ObjectDisposedException` being thrown within dotnet/android#9039 *may* be due to a GC-related bug. A problem with diagnosing this is tracking object lifetimes: yes, an `Android.Runtime.InputStreamInvoker` is throwing `ObjectDisposedException`, but in local reproductions, there are *multiple* `InputStreamInvoker` instances created! Which one is throwing? A local answer to that was "Update `InputStreamInvoker.Read()` to log `BaseInputStream.JniIdentityHashCode`", which *was* useful, but is not a "scalable" solution. Review all `throw new ObjectDisposedException()` calls within `Java.Interop.dll`, and update all sites which use `IJavaPeerable` to include the `JniIdentityHashCode` value in the exception message. This would result in a message like: System.ObjectDisposedException: Cannot access disposed object with JniIdentityHashCode=0x12345678. Object name: 'Android.Runtime.InputStreamInvoker'. at Java.Interop.JniPeerMembers.AssertSelf(IJavaPeerable ) … --- src/Java.Interop/Java.Interop/JavaException.cs | 2 +- src/Java.Interop/Java.Interop/JavaObject.cs | 2 +- .../Java.Interop/JavaPrimitiveArrays.cs | 16 ++++++++-------- .../Java.Interop/JavaPrimitiveArrays.tt | 3 ++- src/Java.Interop/Java.Interop/JniEnvironment.cs | 6 ++++++ src/Java.Interop/Java.Interop/JniPeerMembers.cs | 2 +- 6 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/Java.Interop/Java.Interop/JavaException.cs b/src/Java.Interop/Java.Interop/JavaException.cs index f62135f1f..0bfa3fd58 100644 --- a/src/Java.Interop/Java.Interop/JavaException.cs +++ b/src/Java.Interop/Java.Interop/JavaException.cs @@ -140,7 +140,7 @@ protected void SetPeerReference (ref JniObjectReference reference, JniObjectRefe public void UnregisterFromRuntime () { if (!PeerReference.IsValid) - throw new ObjectDisposedException (GetType ().FullName); + throw JniEnvironment.CreateObjectDisposedException (this); JniEnvironment.Runtime.ValueManager.RemovePeer (this); } diff --git a/src/Java.Interop/Java.Interop/JavaObject.cs b/src/Java.Interop/Java.Interop/JavaObject.cs index 111ec735a..3b84bd0ef 100644 --- a/src/Java.Interop/Java.Interop/JavaObject.cs +++ b/src/Java.Interop/Java.Interop/JavaObject.cs @@ -96,7 +96,7 @@ protected void SetPeerReference (ref JniObjectReference reference, JniObjectRefe public void UnregisterFromRuntime () { if (!PeerReference.IsValid) - throw new ObjectDisposedException (GetType ().FullName); + throw JniEnvironment.CreateObjectDisposedException (this); JniEnvironment.Runtime.ValueManager.RemovePeer (this); } diff --git a/src/Java.Interop/Java.Interop/JavaPrimitiveArrays.cs b/src/Java.Interop/Java.Interop/JavaPrimitiveArrays.cs index 85795c14a..bf9d60f04 100644 --- a/src/Java.Interop/Java.Interop/JavaPrimitiveArrays.cs +++ b/src/Java.Interop/Java.Interop/JavaPrimitiveArrays.cs @@ -178,7 +178,7 @@ protected override JniArrayElements CreateElements () public new unsafe JniBooleanArrayElements GetElements () { if (!PeerReference.IsValid) - throw new ObjectDisposedException (this.GetType ().FullName); + throw JniEnvironment.CreateObjectDisposedException (this); var elements = JniEnvironment.Arrays.GetBooleanArrayElements (PeerReference, null); if (elements == null) throw new InvalidOperationException ("`JniEnvironment.Arrays.GetBooleanArrayElements()` returned NULL!"); @@ -382,7 +382,7 @@ protected override JniArrayElements CreateElements () public new unsafe JniSByteArrayElements GetElements () { if (!PeerReference.IsValid) - throw new ObjectDisposedException (this.GetType ().FullName); + throw JniEnvironment.CreateObjectDisposedException (this); var elements = JniEnvironment.Arrays.GetByteArrayElements (PeerReference, null); if (elements == null) throw new InvalidOperationException ("`JniEnvironment.Arrays.GetByteArrayElements()` returned NULL!"); @@ -586,7 +586,7 @@ protected override JniArrayElements CreateElements () public new unsafe JniCharArrayElements GetElements () { if (!PeerReference.IsValid) - throw new ObjectDisposedException (this.GetType ().FullName); + throw JniEnvironment.CreateObjectDisposedException (this); var elements = JniEnvironment.Arrays.GetCharArrayElements (PeerReference, null); if (elements == null) throw new InvalidOperationException ("`JniEnvironment.Arrays.GetCharArrayElements()` returned NULL!"); @@ -790,7 +790,7 @@ protected override JniArrayElements CreateElements () public new unsafe JniInt16ArrayElements GetElements () { if (!PeerReference.IsValid) - throw new ObjectDisposedException (this.GetType ().FullName); + throw JniEnvironment.CreateObjectDisposedException (this); var elements = JniEnvironment.Arrays.GetShortArrayElements (PeerReference, null); if (elements == null) throw new InvalidOperationException ("`JniEnvironment.Arrays.GetShortArrayElements()` returned NULL!"); @@ -994,7 +994,7 @@ protected override JniArrayElements CreateElements () public new unsafe JniInt32ArrayElements GetElements () { if (!PeerReference.IsValid) - throw new ObjectDisposedException (this.GetType ().FullName); + throw JniEnvironment.CreateObjectDisposedException (this); var elements = JniEnvironment.Arrays.GetIntArrayElements (PeerReference, null); if (elements == null) throw new InvalidOperationException ("`JniEnvironment.Arrays.GetIntArrayElements()` returned NULL!"); @@ -1198,7 +1198,7 @@ protected override JniArrayElements CreateElements () public new unsafe JniInt64ArrayElements GetElements () { if (!PeerReference.IsValid) - throw new ObjectDisposedException (this.GetType ().FullName); + throw JniEnvironment.CreateObjectDisposedException (this); var elements = JniEnvironment.Arrays.GetLongArrayElements (PeerReference, null); if (elements == null) throw new InvalidOperationException ("`JniEnvironment.Arrays.GetLongArrayElements()` returned NULL!"); @@ -1402,7 +1402,7 @@ protected override JniArrayElements CreateElements () public new unsafe JniSingleArrayElements GetElements () { if (!PeerReference.IsValid) - throw new ObjectDisposedException (this.GetType ().FullName); + throw JniEnvironment.CreateObjectDisposedException (this); var elements = JniEnvironment.Arrays.GetFloatArrayElements (PeerReference, null); if (elements == null) throw new InvalidOperationException ("`JniEnvironment.Arrays.GetFloatArrayElements()` returned NULL!"); @@ -1606,7 +1606,7 @@ protected override JniArrayElements CreateElements () public new unsafe JniDoubleArrayElements GetElements () { if (!PeerReference.IsValid) - throw new ObjectDisposedException (this.GetType ().FullName); + throw JniEnvironment.CreateObjectDisposedException (this); var elements = JniEnvironment.Arrays.GetDoubleArrayElements (PeerReference, null); if (elements == null) throw new InvalidOperationException ("`JniEnvironment.Arrays.GetDoubleArrayElements()` returned NULL!"); diff --git a/src/Java.Interop/Java.Interop/JavaPrimitiveArrays.tt b/src/Java.Interop/Java.Interop/JavaPrimitiveArrays.tt index be362d091..086f6bf07 100644 --- a/src/Java.Interop/Java.Interop/JavaPrimitiveArrays.tt +++ b/src/Java.Interop/Java.Interop/JavaPrimitiveArrays.tt @@ -175,7 +175,7 @@ namespace Java.Interop { public new unsafe Jni<#= info.TypeModifier #>ArrayElements GetElements () { if (!PeerReference.IsValid) - throw new ObjectDisposedException (this.GetType ().FullName); + throw JniEnvironment.CreateObjectDisposedException (this); var elements = JniEnvironment.Arrays.Get<#= info.JniMarshalType #>ArrayElements (PeerReference, null); if (elements == null) throw new InvalidOperationException ("`JniEnvironment.Arrays.Get<#= info.JniMarshalType #>ArrayElements()` returned NULL!"); @@ -280,6 +280,7 @@ namespace Java.Interop { JavaArray<<#= info.ManagedType #>>.DestroyArgumentStateArray> (value, ref state, synchronize); } + [RequiresDynamicCode (ExpressionRequiresUnreferencedCode)] [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)] public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize = 0, Type? targetType = null) { diff --git a/src/Java.Interop/Java.Interop/JniEnvironment.cs b/src/Java.Interop/Java.Interop/JniEnvironment.cs index 2b1bd5cdd..2978b5c70 100644 --- a/src/Java.Interop/Java.Interop/JniEnvironment.cs +++ b/src/Java.Interop/Java.Interop/JniEnvironment.cs @@ -91,6 +91,12 @@ internal static void SetEnvironmentInfo (JniEnvironmentInfo info) return Runtime.GetExceptionForThrowable (ref e, JniObjectReferenceOptions.CopyAndDispose); } + internal static Exception CreateObjectDisposedException (IJavaPeerable value) + { + return new ObjectDisposedException (value.GetType ().FullName, + $"Cannot access disposed object with JniIdentityHashCode={value.JniIdentityHashCode}."); + } + internal static void LogCreateLocalRef (JniObjectReference value) { if (!value.IsValid) diff --git a/src/Java.Interop/Java.Interop/JniPeerMembers.cs b/src/Java.Interop/Java.Interop/JniPeerMembers.cs index d2ffd424b..261f7c32a 100644 --- a/src/Java.Interop/Java.Interop/JniPeerMembers.cs +++ b/src/Java.Interop/Java.Interop/JniPeerMembers.cs @@ -150,7 +150,7 @@ internal static void AssertSelf (IJavaPeerable self) var peer = self.PeerReference; if (!peer.IsValid) - throw new ObjectDisposedException (self.GetType ().FullName); + throw JniEnvironment.CreateObjectDisposedException (self); #if FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES var lref = peer.SafeHandle as JniLocalReference;