Skip to content

[Java.Interop] fix global ref leak in ConstructPeer #1403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jonathanpeppers merged 3 commits into from
Apr 16, 2026
+45 −2
Merged

[Java.Interop] fix global ref leak in ConstructPeer #1403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bb2fe5e
[Java.Interop] fix global ref leak in ConstructPeer
jonathanpeppers Apr 15, 2026
2ffc22a
Merge branch 'main' into dev/peppers/gref-leak
jonathanpeppers Apr 15, 2026
4eb9556
Wrap test assertions in try/finally to ensure peer cleanup
jonathanpeppers Apr 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Java.Interop/Java.Interop/JniObjectReference.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ public static void Dispose (ref JniObjectReference reference)
case JniObjectReferenceType.WeakGlobal:
JniEnvironment.Runtime.ObjectReferenceManager.DeleteWeakGlobalReference (ref reference);
break;
case JniObjectReferenceType.Invalid:
break;
default:
throw new NotImplementedException ("Do not know how to dispose: " + reference.Type.ToString () + ".");
}
Expand Down
5 changes: 3 additions & 2 deletions src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,9 @@ public void ConstructPeer (IJavaPeerable peer, ref JniObjectReference reference,
if (peer.JniManagedPeerState.HasFlag (JniManagedPeerStates.Activatable)) {
return;
}
JniObjectReference.Dispose (ref reference, options);
newRef = newRef.NewGlobalRef ();
var orig = newRef;
newRef = orig.NewGlobalRef ();
JniObjectReference.Dispose (ref orig);
Comment on lines +107 to +109
Copy link
Copy Markdown
Member

@simonrozsival simonrozsival Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing something, but I wonder why we even create a new global ref to the same object if we then release the old handle? This means that we own the gref and we could maybe just keep using it without the need to create a new one? I must be missing something 🤔

Copy link
Copy Markdown
Member Author

@jonathanpeppers jonathanpeppers Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The peer's current PeerReference (orig) might not be a global ref — it could be Invalid type (from SetPeerReference with a raw jobject), a local ref, or a weak global ref. NewGlobalRef() ensures we get a proper global ref regardless of the input type. We can't just keep using the existing ref because:

  1. If it's Invalid type, JNI operations on it may fail or behave unpredictably
  2. If it's a local ref, it becomes stale when the JNI frame ends
  3. Only global refs provide the stable, long-lived reference the peer table needs

So NewGlobalRef + Dispose(old) normalizes any ref type into a proper global ref.

} else if (options == JniObjectReferenceOptions.None) {
// `reference` is likely *InvalidJniObjectReference, and can't be touched
return;
Expand Down
40 changes: 40 additions & 0 deletions tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,46 @@ public void ConstructPeer_ImplicitViaBindingMethod_PeerIsInSurfacedPeers ()
JniObjectReference.Dispose (ref localRef);
}

// https://github.com/dotnet/android/issues/11101
[Test]
public void ConstructPeer_CalledMultipleTimes_ShouldNotLeakGlobalRefs ()
{
// Simulate what TypeManager.Activate does in dotnet/android:
// 1. Create an uninitialized JavaObject
// 2. Set a peer reference (storing the raw handle)
// 3. The constructor chain then calls ConstructPeer multiple times
//
// This should not leak JNI global references.

using (var original = new MyDisposableObject ()) {
// Get the jobject handle, simulating the IntPtr jobject param in Activate
var handle = original.PeerReference;

// Create an uninitialized peer and set its reference (simulating Activate)
var peer = (IJavaPeerable) RuntimeHelpers.GetUninitializedObject (typeof (MyDisposableObject));
peer.SetPeerReference (new JniObjectReference (handle.Handle));

try {
// Now simulate the constructor chain calling ConstructPeer multiple times
var ref1 = new JniObjectReference (handle.Handle);
valueManager.ConstructPeer (peer, ref ref1, JniObjectReferenceOptions.Copy);

int grefAfterFirst = JniEnvironment.Runtime.GlobalReferenceCount;

var ref2 = new JniObjectReference (handle.Handle);
valueManager.ConstructPeer (peer, ref ref2, JniObjectReferenceOptions.Copy);

int grefAfterSecond = JniEnvironment.Runtime.GlobalReferenceCount;

// The second ConstructPeer should NOT create an additional global ref
Assert.AreEqual (grefAfterFirst, grefAfterSecond,
"Second ConstructPeer call should not create an additional global ref");
} finally {
peer.Dispose ();
}
}
}


[Test]
public void CollectPeers ()
Expand Down