[Mono.Android] Investigation: restoring WaitForGCBridgeProcessing() on CoreCLR (informational, do not merge)

src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalRegisteredPeers.cs

@@ -6,6 +6,7 @@
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Runtime.InteropServices.Java;
+using System.Threading;
 using Android.Runtime;
 using Java.Interop;
 
@@ -37,6 +38,75 @@ static class JavaMarshalRegisteredPeers
 	static readonly object initializeLock = new ();
 	static bool initialized;
 
+	/// <summary>
+	/// Signaled whenever GC bridge processing is *not* in progress.
+	/// </summary>
+	/// <remarks>
+	/// <para>
+	/// This is the CoreCLR/NativeAOT equivalent of Mono's
+	/// <c>mono_gc_wait_for_bridge_processing()</c>, which blocks on the SGen GC lock held
+	/// for the duration of a bridge round. A bridge round flips every registered peer's
+	/// global reference strong-&gt;weak, asks ART to collect
+	/// (<c>java.lang.Runtime.gc()</c>), and then flips the survivors back. The wait exists
+	/// to match Mono's semantics: threads entering managed code from Java do not observe
+	/// peers mid-flip.
+	/// </para>
+	/// <para>
+	/// It is *not* a performance optimization. It was measured on a MAUI/SkiaSharp
+	/// benchmark (Pixel 5, arm64, 60 Hz, two runs per arm) with the barrier enabled and
+	/// disabled, and it made no useful difference: ART "Explicit concurrent copying GC"
+	/// wall time was 24.7/27.1 ms enabled vs 24.6/21.6 ms disabled, worst frame gap
+	/// 53.6/53.6 ms vs 54.3/57.4 ms. Both arms were far from the Mono baseline on the same
+	/// SDK (10.8-11.5 ms GC, 33.5 ms worst gap) even though the JNI global reference count
+	/// was identical (~840) on all arms. Whatever makes an ART bridge collection ~2x more
+	/// expensive under CoreCLR, it is not mutator activity during the round.
+	/// </para>
+	/// <para>
+	/// <see cref="ManualResetEventSlim"/> is constructed with <c>spinCount: 0</c> on
+	/// purpose: the goal is to park the waiting thread rather than burn CPU that ART's
+	/// collector could use.
+	/// </para>
+	/// </remarks>
+	static readonly ManualResetEventSlim bridgeProcessingFinished = new (initialState: true, spinCount: 0);
+
+	/// <summary>
+	/// <see cref="Environment.CurrentManagedThreadId"/> of the bridge processing thread
+	/// while a round is in progress, otherwise 0.
+	/// </summary>
+	static int bridgeProcessingThreadId;
+
+	/// <summary>
+	/// Blocks until any in-progress GC bridge round completes.
+	/// </summary>
+	/// <remarks>
+	/// <para>
+	/// Like Mono's implementation, this is inherently racy: a thread that observes "no
+	/// bridge processing" can still be preempted by a round that begins immediately
+	/// afterwards. The wait narrows the window, it does not close it.
+	/// </para>
+	/// <para>
+	/// In practice exactly one thread parks per round in a typical UI app (measured:
+	/// 15-36 ms per round), so this trades one unbounded stall for one bounded one
+	/// without reducing total GC cost.
+	/// </para>
+	/// </remarks>
+	internal static void WaitForBridgeProcessing ()
+	{
+		if (!RuntimeFeature.WaitForGCBridgeProcessing)
+			return;
+
+		var finished = bridgeProcessingFinished;
+		if (finished.IsSet)
+			return;
+
+		// The bridge thread runs BridgeProcessingStarted/Finished itself, so it must
+		// never wait on its own completion signal.
+		if (Environment.CurrentManagedThreadId == Volatile.Read (ref bridgeProcessingThreadId))
+			return;
+
+		finished.Wait ();
+	}
+
 	/// <summary>
 	/// Performs the one-shot, process-global GC-bridge initialization the first time it is
 	/// called; subsequent calls return immediately. See <see cref="JavaMarshalRegisteredPeers"/>
@@ -404,6 +474,11 @@ static unsafe void BridgeProcessingStarted (MarkCrossReferencesArgs* mcr)
 			throw new ArgumentNullException (nameof (mcr), "MarkCrossReferencesArgs should never be null.");
 		}
 
+		// Publish the thread id before clearing the signal so that a reentrant call on
+		// this thread can never observe an unsignaled event with a stale id.
+		Volatile.Write (ref bridgeProcessingThreadId, Environment.CurrentManagedThreadId);
+		bridgeProcessingFinished.Reset ();
+
 		HandleContext.EnsureAllContextsAreOurs (mcr);
 	}
 
@@ -414,12 +489,19 @@ static unsafe void BridgeProcessingFinished (MarkCrossReferencesArgs* mcr)
 			throw new ArgumentNullException (nameof (mcr), "MarkCrossReferencesArgs should never be null.");
 		}
 
-		ReadOnlySpan<GCHandle> handlesToFree = ProcessCollectedContexts (mcr);
+		try {
+			ReadOnlySpan<GCHandle> handlesToFree = ProcessCollectedContexts (mcr);
 
 // This call site is reachable on all platforms. 'JavaMarshal.FinishCrossReferenceProcessing(MarkCrossReferencesArgs*, ReadOnlySpan<GCHandle>)' is only supported on: 'android'.
 #pragma warning disable CA1416
-		JavaMarshal.FinishCrossReferenceProcessing (mcr, handlesToFree);
+			JavaMarshal.FinishCrossReferenceProcessing (mcr, handlesToFree);
 #pragma warning restore CA1416
+		} finally {
+			// Must run even if processing throws, otherwise every thread that entered
+			// managed code from Java would block forever.
+			Volatile.Write (ref bridgeProcessingThreadId, 0);
+			bridgeProcessingFinished.Set ();
+		}
 	}
 
 	static unsafe ReadOnlySpan<GCHandle> ProcessCollectedContexts (MarkCrossReferencesArgs* mcr)

src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalValueManager.cs

@@ -23,12 +23,13 @@ public JavaMarshalValueManager ()
 
 	public override void WaitForGCBridgeProcessing ()
 	{
-		// Intentionally empty. The Mono runtime's own implementation acknowledges this
-		// pattern is fundamentally flawed (see FIXME in sgen-bridge.c): a thread that
-		// passes the check can still race with bridge processing that starts immediately
-		// after. The wait cannot prevent the race, only reduce its window. On CoreCLR,
-		// JNI wrapper threads hold their own handle copies via JniObjectReference, so
-		// they are not affected by the bridge swapping control_block handles.
+		// Note the caveat from https://github.com/dotnet/android/pull/11119, which removed
+		// this wait: it cannot actually prevent the race it appears to guard, only shrink
+		// the window, and CoreCLR's JNI wrapper threads hold their own handle copies via
+		// JniObjectReference so they do not observe the bridge swapping control_block
+		// handles. It is restored purely to match Mono's observable blocking semantics --
+		// it was measured to be performance-neutral. See JavaMarshalRegisteredPeers.
+		JavaMarshalRegisteredPeers.WaitForBridgeProcessing ();
 	}
 
 	public override void CollectPeers ()

src/Mono.Android/Microsoft.Android.Runtime/RuntimeFeature.cs

@@ -13,6 +13,7 @@ static class RuntimeFeature
 	const bool TrimmableTypeMapEnabledByDefault = false;
 	const bool ObjectReferenceLoggingEnabledByDefault = false;
 	const bool ManagedToJavaUsesAssemblyFullNameEnabledByDefault = false;
+	const bool WaitForGCBridgeProcessingEnabledByDefault = true;
 
 	const string FeatureSwitchPrefix = "Microsoft.Android.Runtime.RuntimeFeature.";
 	const string StartupHookProviderSwitch = "System.StartupHookProvider.IsSupported";
@@ -50,4 +51,10 @@ static class RuntimeFeature
 	[FeatureSwitchDefinition ($"{FeatureSwitchPrefix}{nameof (ManagedToJavaUsesAssemblyFullName)}")]
 	internal static bool ManagedToJavaUsesAssemblyFullName { get; } =
 		AppContext.TryGetSwitch ($"{FeatureSwitchPrefix}{nameof (ManagedToJavaUsesAssemblyFullName)}", out bool isEnabled) ? isEnabled : ManagedToJavaUsesAssemblyFullNameEnabledByDefault;
+
+	// When disabled, the trimmer can remove the GC bridge barrier and every
+	// WaitForGCBridgeProcessing() call site becomes an empty method body.
+	[FeatureSwitchDefinition ($"{FeatureSwitchPrefix}{nameof (WaitForGCBridgeProcessing)}")]
+	internal static bool WaitForGCBridgeProcessing { get; } =
+		AppContext.TryGetSwitch ($"{FeatureSwitchPrefix}{nameof (WaitForGCBridgeProcessing)}", out bool isEnabled) ? isEnabled : WaitForGCBridgeProcessingEnabledByDefault;
 }

src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMapValueManager.cs

@@ -22,12 +22,13 @@ public TrimmableTypeMapValueManager ()
 
 	public override void WaitForGCBridgeProcessing ()
 	{
-		// Intentionally empty. The Mono runtime's own implementation acknowledges this
-		// pattern is fundamentally flawed (see FIXME in sgen-bridge.c): a thread that
-		// passes the check can still race with bridge processing that starts immediately
-		// after. The wait cannot prevent the race, only reduce its window. On CoreCLR,
-		// JNI wrapper threads hold their own handle copies via JniObjectReference, so
-		// they are not affected by the bridge swapping control_block handles.
+		// Note the caveat from https://github.com/dotnet/android/pull/11119, which removed
+		// this wait: it cannot actually prevent the race it appears to guard, only shrink
+		// the window, and CoreCLR's JNI wrapper threads hold their own handle copies via
+		// JniObjectReference so they do not observe the bridge swapping control_block
+		// handles. It is restored purely to match Mono's observable blocking semantics --
+		// it was measured to be performance-neutral. See JavaMarshalRegisteredPeers.
+		JavaMarshalRegisteredPeers.WaitForBridgeProcessing ();
 	}
 
 	public override void CollectPeers ()

src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.RuntimeConfig.targets

@@ -19,6 +19,11 @@ See: https://github.com/dotnet/runtime/blob/b13715b6984889a709ba29ea8a1961db469f
     <!-- Enable .NET runtime crash reporting before Android's signal chaining takes place.
          See: https://github.com/dotnet/runtime/pull/123735 -->
     <_AndroidEnableDiagnosticCrashReporting Condition=" '$(_AndroidEnableDiagnosticCrashReporting)' == '' ">true</_AndroidEnableDiagnosticCrashReporting>
+    <!-- Internal switch. Blocks Java->managed transitions for the duration of a GC bridge round,
+         matching Mono's mono_gc_wait_for_bridge_processing(). This is a semantics knob, not a
+         performance one: enabling vs disabling it measured as neutral on a MAUI/SkiaSharp
+         benchmark. Set to false to opt out; the trimmer then removes the barrier entirely. -->
+    <_AndroidWaitForGCBridgeProcessing Condition=" '$(_AndroidWaitForGCBridgeProcessing)' == '' ">true</_AndroidWaitForGCBridgeProcessing>
   </PropertyGroup>
 
   <ItemGroup>
@@ -63,6 +68,11 @@ See: https://github.com/dotnet/runtime/blob/b13715b6984889a709ba29ea8a1961db469f
         Value="$(_AndroidEnableObjectReferenceLogging)"
         Trim="true"
     />
+    <RuntimeHostConfigurationOption Include="Microsoft.Android.Runtime.RuntimeFeature.WaitForGCBridgeProcessing"
+        Condition="'$(_AndroidWaitForGCBridgeProcessing)' != ''"
+        Value="$(_AndroidWaitForGCBridgeProcessing)"
+        Trim="true"
+    />
     <!-- https://github.com/dotnet/runtime/pull/123735 -->
     <RuntimeHostConfigurationOption Include="System.Runtime.CrashReportBeforeSignalChaining"
         Value="$(_AndroidEnableDiagnosticCrashReporting)"