From bb2fe5e5071484094ab77461660d197c67191ddb Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 15 Apr 2026 08:31:00 -0500 Subject: [PATCH 1/2] [Java.Interop] fix global ref leak in ConstructPeer Fixes: https://github.com/dotnet/android/issues/11101 Fixes: https://github.com/dotnet/android/issues/10989 When ConstructPeer is called on a peer that already has a valid PeerReference (but without Activatable set), lines 107-108 had a bug: the second JniObjectReference.Dispose call was a duplicate of line 100, and NewGlobalRef created a new global ref without disposing the old one. This caused a global ref leak every time ConstructPeer was called multiple times during constructor chains. Changes: - Fix ConstructPeer lines 107-108 to properly dispose the old PeerReference before replacing it with a new global ref. - Make JniObjectReference.Dispose a no-op for Invalid type refs, since activation code stores raw jobject handles with Invalid type that cannot be deleted via JNI. - Add regression test that calls ConstructPeer twice on the same peer and asserts the global ref count does not grow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/JniObjectReference.cs | 2 + .../JniRuntime.JniValueManager.cs | 5 ++- .../JniRuntimeJniValueManagerContract.cs | 38 +++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/Java.Interop/Java.Interop/JniObjectReference.cs b/src/Java.Interop/Java.Interop/JniObjectReference.cs index cb92a8364..9048b2370 100644 --- a/src/Java.Interop/Java.Interop/JniObjectReference.cs +++ b/src/Java.Interop/Java.Interop/JniObjectReference.cs @@ -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 () + "."); } diff --git a/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs b/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs index 1ce3e6543..869243854 100644 --- a/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs +++ b/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs @@ -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); } else if (options == JniObjectReferenceOptions.None) { // `reference` is likely *InvalidJniObjectReference, and can't be touched return; diff --git a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs index 475403875..96505f43d 100644 --- a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs +++ b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs @@ -129,6 +129,44 @@ 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)); + + // 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"); + + peer.Dispose (); + } + } + [Test] public void CollectPeers () From 4eb955687e2710be80b4c71f324ffcbdc606271f Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 16 Apr 2026 08:39:25 -0500 Subject: [PATCH 2/2] Wrap test assertions in try/finally to ensure peer cleanup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../JniRuntimeJniValueManagerContract.cs | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs index 96505f43d..385bef168 100644 --- a/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs +++ b/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs @@ -148,22 +148,24 @@ public void ConstructPeer_CalledMultipleTimes_ShouldNotLeakGlobalRefs () var peer = (IJavaPeerable) RuntimeHelpers.GetUninitializedObject (typeof (MyDisposableObject)); peer.SetPeerReference (new JniObjectReference (handle.Handle)); - // Now simulate the constructor chain calling ConstructPeer multiple times - var ref1 = new JniObjectReference (handle.Handle); - valueManager.ConstructPeer (peer, ref ref1, JniObjectReferenceOptions.Copy); + 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; + int grefAfterFirst = JniEnvironment.Runtime.GlobalReferenceCount; - var ref2 = new JniObjectReference (handle.Handle); - valueManager.ConstructPeer (peer, ref ref2, JniObjectReferenceOptions.Copy); + var ref2 = new JniObjectReference (handle.Handle); + valueManager.ConstructPeer (peer, ref ref2, JniObjectReferenceOptions.Copy); - int grefAfterSecond = JniEnvironment.Runtime.GlobalReferenceCount; + 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"); - - peer.Dispose (); + // 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 (); + } } }