Skip to content

Commit

Permalink
[Java.Interop] Fix JNI reference count assertions
Browse files Browse the repository at this point in the history
The JNI reference count assertions in
JniRuntime.JniObjectReferenceManager were "off," in that the values
they were asserting didn't actually make sense.

Previous logic for JNI local reference create + destroy:

	// Assume localReferenceCount starts at 0, because of course it does
	Assert (localReferenceCount >= 0);  // True; 0 >= 0
	localReferenceCount++;              // localReferenceCount == 1
	...
	Assert (localReferenceCount >= 0);  // True; 1 >= 0
	localReferenceCount--;              // localReferenceCount == 0

The problem with this logic is that it doesn't actually make sense;
when localReferenceCount is incremented to 1, there is one reference
in existence; conceptually, then, the created reference *is* #1.

Meanwhile, at *cleanup*, we first check that localReferenceCount is
valid, *before* we decrement it. We're not validating that e.g.
reference "#1" has been destroyed, or that the number of outstanding
references *after* cleanup is identical to what existed *before* it
was created.

In short, the "dispose" check is in the wrong place. It should be done
*after* decrementing the count, not before:

	Assert (localReferenceCount >= 0);  // True; 0 >= 0
	localReferenceCount++;              // localReferenceCount == 1
	...
	localReferenceCount--;              // localReferenceCount == 0
	Assert (localReferenceCount >= 0);  // True; 0 >= 0

This dovetails nicely with LoggingJniObjectReferenceManagerDecorator
behavior, in that the logging should follow the same pattern as the
count updating: log after create, before delete. If/when reference
lifetimes are entirely nested and not overlapping, this allows for
lrefc value "1" on create to have the same lrefc value "1" on destroy.
  • Loading branch information
jonpryor committed Nov 10, 2015
1 parent 812590a commit e6468d2
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
4 changes: 4 additions & 0 deletions src/Java.Interop/Java.Interop/JavaException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ public void DisposeUnlessRegistered ()

protected virtual void Dispose (bool disposing)
{
var inner = InnerException as JavaException;
if (inner != null) {
inner.Dispose ();
}
}

public override bool Equals (object obj)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ public virtual void DeleteLocalReference (ref JniObjectReference reference, ref
if (!reference.IsValid)
return;
AssertReferenceType (ref reference, JniObjectReferenceType.Local);
AssertCount (localReferenceCount, "LREF", reference.ToString ());
localReferenceCount--;
AssertCount (localReferenceCount, "LREF", reference.ToString ());
JniEnvironment.References.DeleteLocalRef (reference.Handle);
reference.Invalidate ();
}
Expand Down Expand Up @@ -96,8 +96,8 @@ public virtual IntPtr ReleaseLocalReference (ref JniObjectReference reference, r
{
if (!reference.IsValid)
return IntPtr.Zero;
AssertCount (localReferenceCount, "LREF", reference.ToString ());
localReferenceCount--;
AssertCount (localReferenceCount, "LREF", reference.ToString ());
var h = reference.Handle;
reference.Invalidate ();
return h;
Expand All @@ -121,8 +121,8 @@ public virtual void DeleteGlobalReference (ref JniObjectReference reference)
if (!reference.IsValid)
return;
AssertReferenceType (ref reference, JniObjectReferenceType.Global);
AssertCount (grefc, "GREF", reference.ToString ());
Interlocked.Decrement (ref grefc);
AssertCount (grefc, "GREF", reference.ToString ());
JniEnvironment.References.DeleteGlobalRef (reference.Handle);
reference.Invalidate ();
}
Expand All @@ -141,8 +141,8 @@ public virtual void DeleteWeakGlobalReference (ref JniObjectReference reference)
if (!reference.IsValid)
return;
AssertReferenceType (ref reference, JniObjectReferenceType.WeakGlobal);
AssertCount (wgrefc, "WGREF", reference.ToString ());
Interlocked.Decrement (ref wgrefc);
AssertCount (wgrefc, "WGREF", reference.ToString ());
JniEnvironment.References.DeleteWeakGlobalRef (reference.Handle);
reference.Invalidate ();
}
Expand Down

0 comments on commit e6468d2

Please sign in to comment.