From 93fbdd5e784acc785957b7d0a5cd39433aaa4e09 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 14 Apr 2026 14:04:29 -0500 Subject: [PATCH 1/3] [Mono.Android] fix global ref leak in TypeManager.Activate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: https://github.com/dotnet/android/issues/11101 Fixes: https://github.com/dotnet/android/issues/10989 Context: https://github.com/dotnet/android/commit/5c23bcda8 (PR #9640) The Java.Interop Unification (5c23bcda8) changed `Object` to extend `JavaObject`, introducing an additional `ConstructPeer` call in the constructor chain. `TypeManager.Activate` was updated to use `SetPeerReference` but never set the `Activatable` state flag. Without it, each `ConstructPeer` call in the constructor chain (`JavaObject()` and `Object.SetHandle()`) creates a new JNI global ref, overwriting the previous one without deleting it — leaking 3 global refs per `LayoutInflater.Inflate` call. Before the fix, the new test fails with: Global reference leak detected: 30 extra global refs after inflating/GC'ing 10 custom views. Before=207, After=237 This went unnoticed because the `Activate` path is only triggered when Java creates .NET objects (not the other way around). The two main scenarios are Activity recreation and custom C# views in Android XML layouts. Most developers use .NET MAUI, which has a single Activity and does not use custom C# views in Android layout XML files, so neither scenario was commonly hit. Changes: - Promote `GetUninitializedObject` from a local function in `CreateProxy` to a shared static method. It sets `Activatable | Replaceable`, which tells `ConstructPeer` to return early and not create duplicate global refs. - Have `Activate` call `GetUninitializedObject` then `ConstructPeer` to create one global ref while `jobject` is still a valid JNI local ref. The `Activatable` flag then prevents duplicates during the constructor chain. - Add regression test that inflates custom views and asserts JNI global reference count does not grow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Mono.Android/Java.Interop/TypeManager.cs | 28 ++++++------- .../Android.Widget/CustomWidgetTests.cs | 40 ++++++++++++++++++- 2 files changed, 53 insertions(+), 15 deletions(-) diff --git a/src/Mono.Android/Java.Interop/TypeManager.cs b/src/Mono.Android/Java.Interop/TypeManager.cs index cc0b22936bd..528534a4b13 100644 --- a/src/Mono.Android/Java.Interop/TypeManager.cs +++ b/src/Mono.Android/Java.Interop/TypeManager.cs @@ -181,12 +181,12 @@ static void n_Activate (IntPtr jnienv, IntPtr jclass, IntPtr typename_ptr, IntPt internal static void Activate (IntPtr jobject, ConstructorInfo cinfo, object? []? parms) { try { - var newobj = RuntimeHelpers.GetUninitializedObject (cinfo.DeclaringType!); - if (newobj is IJavaPeerable peer) { - peer.SetPeerReference (new JniObjectReference (jobject)); - } else { - throw new InvalidOperationException ($"Unsupported type: '{newobj}'"); - } + var newobj = GetUninitializedObject (cinfo.DeclaringType!); + var reference = new JniObjectReference (jobject); + JniEnvironment.Runtime.ValueManager.ConstructPeer ( + newobj, + ref reference, + JniObjectReferenceOptions.Copy); cinfo.Invoke (newobj, parms); } catch (Exception e) { var m = FormattableString.Invariant ( @@ -425,15 +425,15 @@ internal static object CreateProxy ( throw new MissingMethodException ( "No constructor found for " + type.FullName + "::.ctor(System.IntPtr, Android.Runtime.JniHandleOwnership)", CreateJavaLocationException ()); + } - static IJavaPeerable GetUninitializedObject ( - [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] - Type type) - { - var v = (IJavaPeerable) System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject (type); - v.SetJniManagedPeerState (JniManagedPeerStates.Replaceable | JniManagedPeerStates.Activatable); - return v; - } + static IJavaPeerable GetUninitializedObject ( + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] + Type type) + { + var v = (IJavaPeerable) System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject (type); + v.SetJniManagedPeerState (JniManagedPeerStates.Replaceable | JniManagedPeerStates.Activatable); + return v; } public static void RegisterType (string java_class, Type t) diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs b/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs index 78d62576a0e..98893bc9f20 100644 --- a/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs @@ -1,4 +1,5 @@ -using Android.App; +using System; +using Android.App; using Android.Content; using Android.Util; using Android.Views; @@ -44,6 +45,43 @@ public void UpperAndLowerCaseCustomWidget_FromLibrary_ShouldNotThrowInflateExcep inflater.Inflate (Resource.Layout.upper_lower_custom, null); }, "Regression test for widgets with uppercase and lowercase namespace (bug #23880) failed."); } + + // https://github.com/dotnet/android/issues/11101 + [Test] + public void InflateCustomView_ShouldNotLeakGlobalRefs () + { + var inflater = (LayoutInflater) Application.Context.GetSystemService (Context.LayoutInflaterService)!; + + // Warm up: inflate once to ensure all caches and type mappings are populated + inflater.Inflate (Resource.Layout.lowercase_custom, null); + + CollectGarbage (times: 3); + + int grefBefore = Java.Interop.Runtime.GlobalReferenceCount; + + for (int i = 0; i < 10; i++) { + inflater.Inflate (Resource.Layout.lowercase_custom, null); + } + + CollectGarbage (times: 3); + + int grefAfter = Java.Interop.Runtime.GlobalReferenceCount; + int delta = grefAfter - grefBefore; + + // Each inflate creates a LinearLayout + CustomButton via TypeManager.Activate. + // If global refs are leaking during activation, delta will be >= 10. + // Allow a small delta for noise (cached objects, etc.) + Assert.IsTrue (delta <= 5, + $"Global reference leak detected: {delta} extra global refs after inflating/GC'ing 10 custom views. Before={grefBefore}, After={grefAfter}"); + + static void CollectGarbage (int times) + { + for (int i = 0; i < times; i++) { + GC.Collect (); + GC.WaitForPendingFinalizers (); + } + } + } } public class CustomButton : Button From c64fdb936831e5cc47d1b859011955a2b6d29355 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 14 Apr 2026 17:02:40 -0500 Subject: [PATCH 2/3] Address PR feedback: use Assert.IsNotNull instead of ! Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs b/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs index 98893bc9f20..bcefa10a8c7 100644 --- a/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs @@ -50,7 +50,8 @@ public void UpperAndLowerCaseCustomWidget_FromLibrary_ShouldNotThrowInflateExcep [Test] public void InflateCustomView_ShouldNotLeakGlobalRefs () { - var inflater = (LayoutInflater) Application.Context.GetSystemService (Context.LayoutInflaterService)!; + var inflater = (LayoutInflater) Application.Context.GetSystemService (Context.LayoutInflaterService); + Assert.IsNotNull (inflater); // Warm up: inflate once to ensure all caches and type mappings are populated inflater.Inflate (Resource.Layout.lowercase_custom, null); From d49eaab39679fc3eac1415f8a76c502c108a117c Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 15 Apr 2026 11:02:12 +0200 Subject: [PATCH 3/3] Fix reflection activation leak in value managers\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../JavaMarshalValueManager.cs | 20 +++++++++++++++---- .../SimpleValueManager.cs | 20 +++++++++++++++---- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalValueManager.cs index 2278e272461..371160af317 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalValueManager.cs @@ -268,14 +268,26 @@ public override void ActivatePeer (IJavaPeerable? self, JniObjectReference refer void ActivateViaReflection (JniObjectReference reference, ConstructorInfo cinfo, object?[]? argumentValues) { var declType = GetDeclaringType (cinfo); + var self = GetUninitializedObject (declType); -#pragma warning disable IL2072 - var self = (IJavaPeerable) System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject (declType); -#pragma warning restore IL2072 - self.SetPeerReference (reference); + // ConstructPeer BEFORE the constructor to create a proper + // global ref and eliminate the race window where bridge + // processing could see a raw local ref. + // See: https://github.com/dotnet/android/issues/11101 + JniEnvironment.Runtime.ValueManager.ConstructPeer ( + self, ref reference, JniObjectReferenceOptions.Copy); cinfo.Invoke (self, argumentValues); + [UnconditionalSuppressMessage ("Trimming", "IL2072", Justification = "Activation constructors are preserved by the runtime typemap.")] + static IJavaPeerable GetUninitializedObject ( + [DynamicallyAccessedMembers (Constructors)] Type type) + { + var value = (IJavaPeerable) System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject (type); + value.SetJniManagedPeerState (JniManagedPeerStates.Replaceable | JniManagedPeerStates.Activatable); + return value; + } + [UnconditionalSuppressMessage ("Trimming", "IL2073", Justification = "🤷‍♂️")] [return: DynamicallyAccessedMembers (Constructors)] Type GetDeclaringType (ConstructorInfo cinfo) => diff --git a/src/Mono.Android/Microsoft.Android.Runtime/SimpleValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/SimpleValueManager.cs index 57d8ef5f84d..df01d621414 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/SimpleValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/SimpleValueManager.cs @@ -228,14 +228,26 @@ public override void ActivatePeer (IJavaPeerable? self, JniObjectReference refer void ActivateViaReflection (JniObjectReference reference, ConstructorInfo cinfo, object?[]? argumentValues) { var declType = GetDeclaringType (cinfo); + var self = GetUninitializedObject (declType); -#pragma warning disable IL2072 - var self = (IJavaPeerable) System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject (declType); -#pragma warning restore IL2072 - self.SetPeerReference (reference); + // ConstructPeer BEFORE the constructor to create a proper + // global ref and eliminate the race window where bridge + // processing could see a raw local ref. + // See: https://github.com/dotnet/android/issues/11101 + JniEnvironment.Runtime.ValueManager.ConstructPeer ( + self, ref reference, JniObjectReferenceOptions.Copy); cinfo.Invoke (self, argumentValues); + [UnconditionalSuppressMessage ("Trimming", "IL2072", Justification = "Activation constructors are preserved by the runtime typemap.")] + static IJavaPeerable GetUninitializedObject ( + [DynamicallyAccessedMembers (Constructors)] Type type) + { + var value = (IJavaPeerable) System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject (type); + value.SetJniManagedPeerState (JniManagedPeerStates.Replaceable | JniManagedPeerStates.Activatable); + return value; + } + [UnconditionalSuppressMessage ("Trimming", "IL2073", Justification = "🤷‍♂️")] [return: DynamicallyAccessedMembers (Constructors)] Type GetDeclaringType (ConstructorInfo cinfo) =>