From 7ab216d83c9066416e3f3f2ef84fd50833b740c1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 09:41:56 +0000 Subject: [PATCH 1/6] Initial plan From e7153723a51c9cbc641925545bdfb0b4cac33313 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 10:25:50 +0000 Subject: [PATCH 2/6] Make WaitForGCBridgeProcessing synchronously drain pending contexts Fixes test failure in Dispose_Finalized by ensuring WaitForGCBridgeProcessing calls CollectPeers() to drain any contexts enqueued during bridge processing. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --- .../Microsoft.Android.Runtime/ManagedValueManager.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index a7c32941e51..b4bb7e72e31 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -53,6 +53,9 @@ void ThrowIfDisposed () public override void WaitForGCBridgeProcessing () { + // Drain any pending contexts that were enqueued during bridge processing. + // This ensures synchronous cleanup when explicitly requested (e.g., in tests). + CollectPeers (); } public unsafe override void CollectPeers () From 7d24c369cd6a6347f3606b3fb29a113f32f7926a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 10:30:01 +0000 Subject: [PATCH 3/6] Use TryGetInstance in bridge processing to avoid creating instance Add TryGetInstance() helper that checks IsValueCreated before accessing Value. Use it in BridgeProcessingFinished and ProcessCollectedContexts to avoid creating ManagedValueManager instance unnecessarily during bridge processing. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --- .../ManagedValueManager.cs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index b4bb7e72e31..7b5b7d92f73 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -30,7 +30,10 @@ class ManagedValueManager : JniRuntime.JniValueManager bool _disposed; static Lazy s_instance = new (() => new ManagedValueManager ()); + public static ManagedValueManager GetOrCreateInstance () => s_instance.Value; + + static ManagedValueManager? TryGetInstance () => s_instance.IsValueCreated ? s_instance.Value : null; unsafe ManagedValueManager () { @@ -444,13 +447,24 @@ static unsafe void BridgeProcessingFinished (MarkCrossReferencesArgs* mcr) // Schedule cleanup of _registeredInstances on a thread pool thread. // The bridge thread must not take lock(_registeredInstances) — see deadlock notes. - Task.Run (GetOrCreateInstance ().CollectPeers); + // Only schedule if instance exists (bridge processing implies registered objects exist). + var instance = TryGetInstance (); + if (instance != null) { + Task.Run (instance.CollectPeers); + } } static unsafe ReadOnlySpan ProcessCollectedContexts (MarkCrossReferencesArgs* mcr) { List handlesToFree = []; - ManagedValueManager instance = GetOrCreateInstance (); + + // Bridge processing should only happen if instance exists (there are registered objects). + // Use TryGetInstance to avoid creating instance unnecessarily. + ManagedValueManager? instance = TryGetInstance (); + if (instance == null) { + // No registered objects, nothing to process + return ReadOnlySpan.Empty; + } for (int i = 0; (nuint)i < mcr->ComponentCount; i++) { StronglyConnectedComponent component = mcr->Components [i]; From 1bfccb19d994661a08eae1c44add402992713260 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 10:31:03 +0000 Subject: [PATCH 4/6] Rename TryGetInstance to GetInstanceIfCreated and clarify comments Follow conventional naming: GetInstanceIfCreated (returns null) vs Try* (returns bool with out param). Clarify comments to reflect defensive checking rather than invariants. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --- .../Microsoft.Android.Runtime/ManagedValueManager.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index 7b5b7d92f73..b08a3505051 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -33,7 +33,7 @@ class ManagedValueManager : JniRuntime.JniValueManager public static ManagedValueManager GetOrCreateInstance () => s_instance.Value; - static ManagedValueManager? TryGetInstance () => s_instance.IsValueCreated ? s_instance.Value : null; + static ManagedValueManager? GetInstanceIfCreated () => s_instance.IsValueCreated ? s_instance.Value : null; unsafe ManagedValueManager () { @@ -447,8 +447,8 @@ static unsafe void BridgeProcessingFinished (MarkCrossReferencesArgs* mcr) // Schedule cleanup of _registeredInstances on a thread pool thread. // The bridge thread must not take lock(_registeredInstances) — see deadlock notes. - // Only schedule if instance exists (bridge processing implies registered objects exist). - var instance = TryGetInstance (); + // Only schedule if instance exists (bridge processing typically implies registered objects exist). + var instance = GetInstanceIfCreated (); if (instance != null) { Task.Run (instance.CollectPeers); } @@ -458,9 +458,9 @@ static unsafe ReadOnlySpan ProcessCollectedContexts (MarkCrossReferenc { List handlesToFree = []; - // Bridge processing should only happen if instance exists (there are registered objects). - // Use TryGetInstance to avoid creating instance unnecessarily. - ManagedValueManager? instance = TryGetInstance (); + // Bridge processing typically implies registered objects exist, but check defensively + // to avoid creating the ManagedValueManager instance unnecessarily. + ManagedValueManager? instance = GetInstanceIfCreated (); if (instance == null) { // No registered objects, nothing to process return ReadOnlySpan.Empty; From fabcb091dfcd791115dd82d7162648c160bee8b8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 12:03:35 +0000 Subject: [PATCH 5/6] Rename GetOrCreateInstance to Instance property and make WaitForGCBridgeProcessing a no-op - Replace GetOrCreateInstance() method with Instance property getter for cleaner singleton pattern - Update JNIEnvInit.cs to use the new Instance property - Make WaitForGCBridgeProcessing() a no-op (remove CollectPeers call) - Update JavaObjectTest.WaitForGC() to explicitly call CollectPeers() after WaitForGCBridgeProcessing() Note: JavaObjectTest.cs change is in external/Java.Interop submodule and will need to be committed separately in the Java.Interop repository. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --- src/Mono.Android/Android.Runtime/JNIEnvInit.cs | 2 +- .../Microsoft.Android.Runtime/ManagedValueManager.cs | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Mono.Android/Android.Runtime/JNIEnvInit.cs b/src/Mono.Android/Android.Runtime/JNIEnvInit.cs index 71b370a3cee..65741304b35 100644 --- a/src/Mono.Android/Android.Runtime/JNIEnvInit.cs +++ b/src/Mono.Android/Android.Runtime/JNIEnvInit.cs @@ -139,7 +139,7 @@ internal static unsafe void Initialize (JnienvInitializeArgs* args) if (RuntimeFeature.IsMonoRuntime) { valueManager = new AndroidValueManager (); } else if (RuntimeFeature.IsCoreClrRuntime) { - valueManager = ManagedValueManager.GetOrCreateInstance (); + valueManager = ManagedValueManager.Instance; } else { throw new NotSupportedException ("Internal error: unknown runtime not supported"); } diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index b08a3505051..afaa3bbed2a 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -31,7 +31,7 @@ class ManagedValueManager : JniRuntime.JniValueManager static Lazy s_instance = new (() => new ManagedValueManager ()); - public static ManagedValueManager GetOrCreateInstance () => s_instance.Value; + public static ManagedValueManager Instance => s_instance.Value; static ManagedValueManager? GetInstanceIfCreated () => s_instance.IsValueCreated ? s_instance.Value : null; @@ -56,9 +56,6 @@ void ThrowIfDisposed () public override void WaitForGCBridgeProcessing () { - // Drain any pending contexts that were enqueued during bridge processing. - // This ensures synchronous cleanup when explicitly requested (e.g., in tests). - CollectPeers (); } public unsafe override void CollectPeers () From a961e4ab7236b3204deb1102d31e4f5c6028e320 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 16:46:52 +0000 Subject: [PATCH 6/6] Remove GetInstanceIfCreated and use Instance everywhere Remove the defensive null checks in bridge processing. If bridge processing is being called, the instance must already exist since objects need to be registered for bridge processing to happen. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --- .../ManagedValueManager.cs | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index afaa3bbed2a..27ceb09d019 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -32,8 +32,6 @@ class ManagedValueManager : JniRuntime.JniValueManager static Lazy s_instance = new (() => new ManagedValueManager ()); public static ManagedValueManager Instance => s_instance.Value; - - static ManagedValueManager? GetInstanceIfCreated () => s_instance.IsValueCreated ? s_instance.Value : null; unsafe ManagedValueManager () { @@ -444,24 +442,13 @@ static unsafe void BridgeProcessingFinished (MarkCrossReferencesArgs* mcr) // Schedule cleanup of _registeredInstances on a thread pool thread. // The bridge thread must not take lock(_registeredInstances) — see deadlock notes. - // Only schedule if instance exists (bridge processing typically implies registered objects exist). - var instance = GetInstanceIfCreated (); - if (instance != null) { - Task.Run (instance.CollectPeers); - } + Task.Run (Instance.CollectPeers); } static unsafe ReadOnlySpan ProcessCollectedContexts (MarkCrossReferencesArgs* mcr) { List handlesToFree = []; - - // Bridge processing typically implies registered objects exist, but check defensively - // to avoid creating the ManagedValueManager instance unnecessarily. - ManagedValueManager? instance = GetInstanceIfCreated (); - if (instance == null) { - // No registered objects, nothing to process - return ReadOnlySpan.Empty; - } + ManagedValueManager instance = Instance; for (int i = 0; (nuint)i < mcr->ComponentCount; i++) { StronglyConnectedComponent component = mcr->Components [i];