diff --git a/src/Mono.Android/Android.Runtime/AndroidRuntime.cs b/src/Mono.Android/Android.Runtime/AndroidRuntime.cs index 883e3c6efe1..0714fe93476 100644 --- a/src/Mono.Android/Android.Runtime/AndroidRuntime.cs +++ b/src/Mono.Android/Android.Runtime/AndroidRuntime.cs @@ -685,23 +685,33 @@ public override void AddPeer (IJavaPeerable value) internal void AddPeer (IJavaPeerable value, JniObjectReference reference, IntPtr hash) { + if (Logger.LogGlobalRef) Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"AddPeer: hash=0x{hash:x} PeerRef={reference} PeerRef.Type={reference.Type} State={value.JniManagedPeerState} type={value.GetType ().FullName}")); lock (instances) { if (!instances.TryGetValue (hash, out var targets)) { targets = new IdentityHashTargets (value); instances.Add (hash, targets); + if (Logger.LogGlobalRef) Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"AddPeer: new entry, instances.Count={instances.Count}")); return; } bool found = false; + if (Logger.LogGlobalRef) Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"AddPeer: existing hash with {targets.Count} entries")); for (int i = 0; i < targets.Count; ++i) { IJavaPeerable? target; var wref = targets [i]; if (ShouldReplaceMapping (wref!, reference, value, out target)) { found = true; + if (Logger.LogGlobalRef) Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"AddPeer: REPLACING entry[{i}] (target.State={target?.JniManagedPeerState} target.Type={target?.GetType ().FullName})")); targets [i] = IdentityHashTargets.CreateWeakReference (value); break; } if (JniEnvironment.Types.IsSameObject (value.PeerReference, target!.PeerReference)) { found = true; + if (Logger.LogGlobalRef) Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"AddPeer: SAME object already registered at [{i}] (target.State={target.JniManagedPeerState} target.Type={target.GetType ().FullName})")); if (Logger.LogGlobalRef) { Logger.Log (LogLevel.Info, "monodroid-gref", FormattableString.Invariant ( $"warning: not replacing previous registered handle {target.PeerReference} with handle {reference} for key_handle 0x{hash:x}")); @@ -710,6 +720,8 @@ internal void AddPeer (IJavaPeerable value, JniObjectReference reference, IntPtr } if (!found) { targets.Add (value); + if (Logger.LogGlobalRef) Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"AddPeer: APPENDED as entry[{targets.Count - 1}]")); } } } @@ -809,8 +821,12 @@ public override void RemovePeer (IJavaPeerable value) internal void RemovePeer (IJavaPeerable value, IntPtr hash) { + if (Logger.LogGlobalRef) Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"RemovePeer: hash=0x{hash:x} type={value.GetType ().FullName}")); lock (instances) { if (!instances.TryGetValue (hash, out var targets)) { + if (Logger.LogGlobalRef) Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"RemovePeer: no entry for hash=0x{hash:x}")); return; } for (int i = targets.Count - 1; i >= 0; i--) { @@ -828,6 +844,8 @@ internal void RemovePeer (IJavaPeerable value, IntPtr hash) if (targets.Count == 0) { instances.Remove (hash); } + if (Logger.LogGlobalRef) Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"RemovePeer: done, instances.Count={instances.Count}")); } } @@ -879,6 +897,9 @@ public override void FinalizePeer (IJavaPeerable value) if (value == null) throw new ArgumentNullException (nameof (value)); + if (Logger.LogGlobalRef) Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"FinalizePeer: PeerRef={value.PeerReference} PeerRef.Type={value.PeerReference.Type} PeerRef.IsValid={value.PeerReference.IsValid} State={value.JniManagedPeerState} IdentityHashCode=0x{value.JniIdentityHashCode:x} type={value.GetType ().FullName}")); + if (Logger.LogGlobalRef) { RuntimeNativeMethods._monodroid_gref_log ( string.Format (CultureInfo.InvariantCulture, @@ -894,8 +915,12 @@ public override void FinalizePeer (IJavaPeerable value) // handle still contains a java reference, we can't finalize the // object and should "resurrect" it. if (value.PeerReference.IsValid) { + if (Logger.LogGlobalRef) Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"FinalizePeer: RESURRECTING (PeerReference still valid) type={value.GetType ().FullName}")); GC.ReRegisterForFinalize (value); } else { + if (Logger.LogGlobalRef) Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"FinalizePeer: DISPOSING type={value.GetType ().FullName}")); RemovePeer (value, (IntPtr) value.JniIdentityHashCode); value.SetPeerReference (new JniObjectReference ()); value.Finalized (); diff --git a/src/Mono.Android/Java.Interop/TypeManager.cs b/src/Mono.Android/Java.Interop/TypeManager.cs index cc0b22936bd..7b4c63189ae 100644 --- a/src/Mono.Android/Java.Interop/TypeManager.cs +++ b/src/Mono.Android/Java.Interop/TypeManager.cs @@ -183,11 +183,38 @@ internal static void Activate (IntPtr jobject, ConstructorInfo cinfo, object? [] try { var newobj = RuntimeHelpers.GetUninitializedObject (cinfo.DeclaringType!); if (newobj is IJavaPeerable peer) { - peer.SetPeerReference (new JniObjectReference (jobject)); + // Set Activatable BEFORE ConstructPeer so that the + // constructor chain's ConstructPeer (via SetHandle) will + // see the existing PeerReference and return early, avoiding + // a duplicate global ref. + peer.SetJniManagedPeerState (JniManagedPeerStates.Activatable); + // Create a proper JNI global ref and register the peer + // BEFORE invoking the constructor. This eliminates a race + // window: if SetPeerReference stored a raw local ref and a + // GC triggered bridge processing before ConstructPeer ran, + // the bridge would call DeleteGlobalRef on a local ref + // (JNI error) and create an orphaned global ref that keeps + // the Java object alive forever. + // See: https://github.com/dotnet/android/issues/11101 + var reference = new JniObjectReference (jobject); + if (Logger.LogGlobalRef) { + Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"Activate: ConstructPeer handle=0x{jobject:x} type={cinfo.DeclaringType?.FullName}")); + } + JniEnvironment.Runtime.ValueManager.ConstructPeer ( + peer, ref reference, JniObjectReferenceOptions.Copy); + if (Logger.LogGlobalRef) { + Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"Activate: after ConstructPeer PeerRef={peer.PeerReference} PeerRef.Type={peer.PeerReference.Type} State={peer.JniManagedPeerState}")); + } } else { throw new InvalidOperationException ($"Unsupported type: '{newobj}'"); } cinfo.Invoke (newobj, parms); + if (Logger.LogGlobalRef) { + Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"Activate: after ctor PeerRef={peer.PeerReference} PeerRef.Type={peer.PeerReference.Type} State={peer.JniManagedPeerState} IdentityHashCode=0x{peer.JniIdentityHashCode:x}")); + } } catch (Exception e) { var m = FormattableString.Invariant ( $"Could not activate JNI Handle 0x{jobject:x} (key_handle 0x{JNIEnv.IdentityHash (jobject):x}) of Java type '{JNIEnv.GetClassNameFromInstance (jobject)}' as managed type '{cinfo?.DeclaringType?.FullName}'."); diff --git a/src/Mono.Android/Java.Lang/Object.cs b/src/Mono.Android/Java.Lang/Object.cs index 814d4c3b277..012aa76ec7d 100644 --- a/src/Mono.Android/Java.Lang/Object.cs +++ b/src/Mono.Android/Java.Lang/Object.cs @@ -109,12 +109,21 @@ protected override void Dispose (bool disposing) [EditorBrowsable (EditorBrowsableState.Never)] protected void SetHandle (IntPtr value, JniHandleOwnership transfer) { + var effectiveOptions = value == IntPtr.Zero ? JniObjectReferenceOptions.None : FromJniHandleOwnership (transfer); + if (Logger.LogGlobalRef) { + var existingRef = PeerReference; + Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"SetHandle: value=0x{value:x} transfer={transfer} existingPeerRef={existingRef} existingPeerRef.Type={existingRef.Type} effectiveOptions={effectiveOptions} State={((IJavaPeerable)this).JniManagedPeerState} type={GetType ().FullName}")); + } var reference = new JniObjectReference (value); - var options = FromJniHandleOwnership (transfer); JniEnvironment.Runtime.ValueManager.ConstructPeer ( this, ref reference, - value == IntPtr.Zero ? JniObjectReferenceOptions.None : options); + effectiveOptions); + if (Logger.LogGlobalRef) { + Logger.Log (LogLevel.Info, "monodroid-peer", + FormattableString.Invariant ($"SetHandle: after ConstructPeer PeerRef={PeerReference} PeerRef.Type={PeerReference.Type} State={((IJavaPeerable)this).JniManagedPeerState} IdentityHashCode=0x{JniIdentityHashCode:x}")); + } JNIEnv.DeleteRef (value, transfer); } diff --git a/src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalValueManager.cs index 2278e272461..865a6247efc 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalValueManager.cs @@ -272,7 +272,13 @@ void ActivateViaReflection (JniObjectReference reference, ConstructorInfo cinfo, #pragma warning disable IL2072 var self = (IJavaPeerable) System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject (declType); #pragma warning restore IL2072 - self.SetPeerReference (reference); + // Set Activatable + 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 + self.SetJniManagedPeerState (JniManagedPeerStates.Activatable); + JniEnvironment.Runtime.ValueManager.ConstructPeer ( + self, ref reference, JniObjectReferenceOptions.Copy); cinfo.Invoke (self, argumentValues); diff --git a/src/Mono.Android/Microsoft.Android.Runtime/SimpleValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/SimpleValueManager.cs index 57d8ef5f84d..6c98adade73 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/SimpleValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/SimpleValueManager.cs @@ -232,7 +232,13 @@ void ActivateViaReflection (JniObjectReference reference, ConstructorInfo cinfo, #pragma warning disable IL2072 var self = (IJavaPeerable) System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject (declType); #pragma warning restore IL2072 - self.SetPeerReference (reference); + // Set Activatable + 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 + self.SetJniManagedPeerState (JniManagedPeerStates.Activatable); + JniEnvironment.Runtime.ValueManager.ConstructPeer ( + self, ref reference, JniObjectReferenceOptions.Copy); cinfo.Invoke (self, argumentValues); diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Views/InflatedCustomViewTests.cs b/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Views/InflatedCustomViewTests.cs new file mode 100644 index 00000000000..6ebb4a4cb7a --- /dev/null +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Views/InflatedCustomViewTests.cs @@ -0,0 +1,129 @@ +using System; +using Android.App; +using Android.Content; +using Android.Runtime; +using Android.Util; +using Android.Views; +using Java.Interop; +using NUnit.Framework; + +namespace Xamarin.Android.RuntimeTests; + +// https://github.com/dotnet/android/issues/11101 +[TestFixture] +public class InflatedCustomViewTests +{ + [Test] + public void InflatedCustomView_HasValidPeerReference () + { + var inflater = LayoutInflater.From (Application.Context)!; + var layout = inflater.Inflate (Resource.Layout.inflated_custom_view, null, false)!; + + // Find our custom view in the inflated layout + var customView = FindCustomView (layout); + + Assert.IsNotNull (customView, "Custom view should be found in inflated layout"); + + // After inflation via Java-initiated activation, the peer should have a + // properly managed global JNI reference, not a raw local ref with Invalid type. + var peerRef = customView!.PeerReference; + Assert.IsTrue (peerRef.IsValid, "PeerReference should be valid"); + Assert.AreNotEqual ( + JniObjectReferenceType.Invalid, + peerRef.Type, + "PeerReference.Type should not be Invalid — it should be a Global ref"); + + // The peer should be registered so PeekObject can find it + var peeked = Java.Lang.Object.PeekObject (customView.Handle); + Assert.IsNotNull (peeked, "PeekObject should find the registered peer"); + Assert.AreSame (customView, peeked, "PeekObject should return the same instance"); + } + + [Test] + public void InflatedCustomView_CanBeCollected () + { + WeakReference? weakRef = null; + + // Create and discard the inflated view on a separate thread + // to avoid any local variable keeping it alive + var t = new System.Threading.Thread (() => { + var inflater = LayoutInflater.From (Application.Context)!; + var layout = inflater.Inflate (Resource.Layout.inflated_custom_view, null, false)!; + var customView = FindCustomView (layout); + Assert.IsNotNull (customView, "Custom view should be found in inflated layout"); + weakRef = new WeakReference (customView); + }); + t.Start (); + t.Join (); + + // Force GC + bridge processing + GC.Collect (); + GC.WaitForPendingFinalizers (); + GC.Collect (); + GC.WaitForPendingFinalizers (); + + Assert.IsNotNull (weakRef, "WeakReference should have been created"); + Assert.IsFalse (weakRef!.IsAlive, + "Custom view should be collected after GC — if it's still alive, there is a memory leak (https://github.com/dotnet/android/issues/11101)"); + } + + // Stress test: repeated inflation + GC to trigger the race condition + // between Activate.SetPeerReference and ConstructPeer. Under the bug, + // each race hit leaks a JNI global ref, so gref count grows unboundedly. + [Test] + public void InflatedCustomView_RepeatedInflation_DoesNotLeakGlobalRefs () + { + int initialGrefCount = Java.Interop.JniEnvironment.Runtime.GlobalReferenceCount; + + for (int i = 0; i < 50; i++) { + var t = new System.Threading.Thread (() => { + var inflater = LayoutInflater.From (Application.Context)!; + inflater.Inflate (Resource.Layout.inflated_custom_view, null, false); + }); + t.Start (); + t.Join (); + } + + GC.Collect (); + GC.WaitForPendingFinalizers (); + GC.Collect (); + GC.WaitForPendingFinalizers (); + + int finalGrefCount = Java.Interop.JniEnvironment.Runtime.GlobalReferenceCount; + + // Allow some tolerance — other code may allocate/release grefs. + // The key assertion is that gref count doesn't grow proportionally + // to the number of inflations (under the bug, each inflation + // leaks ~1 gref, so 50 inflations would leak ~50 grefs). + int leaked = finalGrefCount - initialGrefCount; + Assert.Less (leaked, 10, + $"Global reference count grew by {leaked} after 50 inflations — " + + $"expected near-zero growth after GC (initial={initialGrefCount}, final={finalGrefCount})"); + } + + static InflatedCustomView? FindCustomView (View root) + { + if (root is InflatedCustomView customView) + return customView; + + if (root is ViewGroup viewGroup) { + for (int i = 0; i < viewGroup.ChildCount; i++) { + var child = viewGroup.GetChildAt (i); + if (child is InflatedCustomView found) + return found; + } + } + + return null; + } +} + +// A simple custom view that can be inflated from XML +public sealed class InflatedCustomView : View +{ + public InflatedCustomView (Context? context) : base (context) { } + public InflatedCustomView (nint javaReference, JniHandleOwnership transfer) : base (javaReference, transfer) { } + public InflatedCustomView (Context? context, IAttributeSet? attrs) : base (context, attrs) { } + public InflatedCustomView (Context? context, IAttributeSet? attrs, int defStyleAttr) : base (context, attrs, defStyleAttr) { } + public InflatedCustomView (Context? context, IAttributeSet? attrs, int defStyleAttr, int defStyleRes) : base (context, attrs, defStyleAttr, defStyleRes) { } +} diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Resources/layout/inflated_custom_view.axml b/tests/Mono.Android-Tests/Mono.Android-Tests/Resources/layout/inflated_custom_view.axml new file mode 100644 index 00000000000..987673103ae --- /dev/null +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Resources/layout/inflated_custom_view.axml @@ -0,0 +1,11 @@ + + + + + +