Skip to content

Commit e6468d2

Browse files
committed
[Java.Interop] Fix JNI reference count assertions
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.
1 parent 812590a commit e6468d2

File tree

2 files changed

+8
-4
lines changed

2 files changed

+8
-4
lines changed

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

+4
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ public void DisposeUnlessRegistered ()
157157

158158
protected virtual void Dispose (bool disposing)
159159
{
160+
var inner = InnerException as JavaException;
161+
if (inner != null) {
162+
inner.Dispose ();
163+
}
160164
}
161165

162166
public override bool Equals (object obj)

src/Java.Interop/Java.Interop/JniRuntime.JniObjectReferenceManager.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ public virtual void DeleteLocalReference (ref JniObjectReference reference, ref
6363
if (!reference.IsValid)
6464
return;
6565
AssertReferenceType (ref reference, JniObjectReferenceType.Local);
66-
AssertCount (localReferenceCount, "LREF", reference.ToString ());
6766
localReferenceCount--;
67+
AssertCount (localReferenceCount, "LREF", reference.ToString ());
6868
JniEnvironment.References.DeleteLocalRef (reference.Handle);
6969
reference.Invalidate ();
7070
}
@@ -96,8 +96,8 @@ public virtual IntPtr ReleaseLocalReference (ref JniObjectReference reference, r
9696
{
9797
if (!reference.IsValid)
9898
return IntPtr.Zero;
99-
AssertCount (localReferenceCount, "LREF", reference.ToString ());
10099
localReferenceCount--;
100+
AssertCount (localReferenceCount, "LREF", reference.ToString ());
101101
var h = reference.Handle;
102102
reference.Invalidate ();
103103
return h;
@@ -121,8 +121,8 @@ public virtual void DeleteGlobalReference (ref JniObjectReference reference)
121121
if (!reference.IsValid)
122122
return;
123123
AssertReferenceType (ref reference, JniObjectReferenceType.Global);
124-
AssertCount (grefc, "GREF", reference.ToString ());
125124
Interlocked.Decrement (ref grefc);
125+
AssertCount (grefc, "GREF", reference.ToString ());
126126
JniEnvironment.References.DeleteGlobalRef (reference.Handle);
127127
reference.Invalidate ();
128128
}
@@ -141,8 +141,8 @@ public virtual void DeleteWeakGlobalReference (ref JniObjectReference reference)
141141
if (!reference.IsValid)
142142
return;
143143
AssertReferenceType (ref reference, JniObjectReferenceType.WeakGlobal);
144-
AssertCount (wgrefc, "WGREF", reference.ToString ());
145144
Interlocked.Decrement (ref wgrefc);
145+
AssertCount (wgrefc, "WGREF", reference.ToString ());
146146
JniEnvironment.References.DeleteWeakGlobalRef (reference.Handle);
147147
reference.Invalidate ();
148148
}

0 commit comments

Comments
 (0)