Skip to content

Commit

Permalink
[Java.Interop] Add JniIdentityHashCode to ObjectDisposedException (#1276
Browse files Browse the repository at this point in the history
)

Context: dotnet/android#9039
Context: dotnet/android@32495f3

@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 )
	   …
  • Loading branch information
jonpryor authored Nov 8, 2024
1 parent 2bdf2bc commit 2440416
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/Java.Interop/Java.Interop/JavaException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion src/Java.Interop/Java.Interop/JavaObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
16 changes: 8 additions & 8 deletions src/Java.Interop/Java.Interop/JavaPrimitiveArrays.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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!");
Expand Down Expand Up @@ -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!");
Expand Down Expand Up @@ -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!");
Expand Down Expand Up @@ -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!");
Expand Down Expand Up @@ -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!");
Expand Down Expand Up @@ -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!");
Expand Down Expand Up @@ -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!");
Expand Down Expand Up @@ -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!");
Expand Down
3 changes: 2 additions & 1 deletion src/Java.Interop/Java.Interop/JavaPrimitiveArrays.tt
Original file line number Diff line number Diff line change
Expand Up @@ -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!");
Expand Down Expand Up @@ -280,6 +280,7 @@ namespace Java.Interop {
JavaArray<<#= info.ManagedType #>>.DestroyArgumentState<Java<#= info.TypeModifier #>Array> (value, ref state, synchronize);
}

[RequiresDynamicCode (ExpressionRequiresUnreferencedCode)]
[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize = 0, Type? targetType = null)
{
Expand Down
6 changes: 6 additions & 0 deletions src/Java.Interop/Java.Interop/JniEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/Java.Interop/Java.Interop/JniPeerMembers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 2440416

Please sign in to comment.