From 5e9207145f6737031b597a80e804eab1ec5b0560 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 13 Jun 2025 15:42:26 +0200 Subject: [PATCH 01/32] Implement GC bridge for CoreCLR --- .../JavaInteropRuntime.cs | 2 +- .../Java.Interop/JreRuntime.cs | 2 +- .../Android.Runtime/AndroidRuntime.cs | 4 +- src/Mono.Android/Android.Runtime/JNIEnv.cs | 2 +- .../Android.Runtime/JNIEnvInit.cs | 8 +- .../Android.Runtime/JNINativeWrapper.g.cs | 80 ++--- .../Android.Runtime/JNINativeWrapper.g.tt | 2 +- .../Android.Runtime/RuntimeNativeMethods.cs | 8 +- src/Mono.Android/Java.Interop/Runtime.cs | 2 +- src/Mono.Android/Java.Lang/Object.cs | 6 +- .../ManagedValueManager.cs | 204 ++++++++---- src/native/clr/host/CMakeLists.txt | 1 + src/native/clr/host/gc-bridge.cc | 315 ++++++++++++++++++ .../clr/host/generate-pinvoke-tables.cc | 1 + src/native/clr/host/host.cc | 2 + src/native/clr/host/internal-pinvokes.cc | 10 +- src/native/clr/host/os-bridge.cc | 15 + src/native/clr/host/pinvoke-tables.include | 8 +- src/native/clr/include/host/gc-bridge.hh | 106 ++++++ src/native/clr/include/host/os-bridge.hh | 8 +- .../include/runtime-base/internal-pinvokes.hh | 4 + src/native/clr/include/runtime-base/util.hh | 2 + src/native/clr/runtime-base/util.cc | 21 ++ 23 files changed, 687 insertions(+), 126 deletions(-) create mode 100644 src/native/clr/host/gc-bridge.cc create mode 100644 src/native/clr/include/host/gc-bridge.hh diff --git a/src/Microsoft.Android.Runtime.NativeAOT/Android.Runtime.NativeAOT/JavaInteropRuntime.cs b/src/Microsoft.Android.Runtime.NativeAOT/Android.Runtime.NativeAOT/JavaInteropRuntime.cs index 340078864e8..9f2274c091b 100644 --- a/src/Microsoft.Android.Runtime.NativeAOT/Android.Runtime.NativeAOT/JavaInteropRuntime.cs +++ b/src/Microsoft.Android.Runtime.NativeAOT/Android.Runtime.NativeAOT/JavaInteropRuntime.cs @@ -43,7 +43,7 @@ static void init (IntPtr jnienv, IntPtr klass, IntPtr classLoader) EnvironmentPointer = jnienv, ClassLoader = new JniObjectReference (classLoader), TypeManager = typeManager, - ValueManager = new ManagedValueManager (), + ValueManager = ManagedValueManager.GetOrCreateInstance (), UseMarshalMemberBuilder = false, JniGlobalReferenceLogWriter = settings.GrefLog, JniLocalReferenceLogWriter = settings.LrefLog, diff --git a/src/Microsoft.Android.Runtime.NativeAOT/Java.Interop/JreRuntime.cs b/src/Microsoft.Android.Runtime.NativeAOT/Java.Interop/JreRuntime.cs index c660cbc325b..fa244eb67b9 100644 --- a/src/Microsoft.Android.Runtime.NativeAOT/Java.Interop/JreRuntime.cs +++ b/src/Microsoft.Android.Runtime.NativeAOT/Java.Interop/JreRuntime.cs @@ -61,7 +61,7 @@ static NativeAotRuntimeOptions CreateJreVM (NativeAotRuntimeOptions builder) builder.TypeManager ??= new ManagedTypeManager (); #endif // NET - builder.ValueManager ??= new ManagedValueManager (); + builder.ValueManager ??= ManagedValueManager.GetOrCreateInstance (); builder.ObjectReferenceManager ??= new ManagedObjectReferenceManager (builder.JniGlobalReferenceLogWriter, builder.JniLocalReferenceLogWriter); if (builder.InvocationPointer != IntPtr.Zero || builder.EnvironmentPointer != IntPtr.Zero) diff --git a/src/Mono.Android/Android.Runtime/AndroidRuntime.cs b/src/Mono.Android/Android.Runtime/AndroidRuntime.cs index 3f6d8f86cf2..f6a1ee4e3c3 100644 --- a/src/Mono.Android/Android.Runtime/AndroidRuntime.cs +++ b/src/Mono.Android/Android.Runtime/AndroidRuntime.cs @@ -58,7 +58,7 @@ public override string GetCurrentManagedThreadStackTrace (int skipFrames, bool f { if (!reference.IsValid) return null; - var peeked = JNIEnvInit.ValueManager?.PeekPeer (reference); + var peeked = JniEnvironment.Runtime.ValueManager.PeekPeer (reference); var peekedExc = peeked as Exception; if (peekedExc == null) { var throwable = Java.Lang.Object.GetObject (reference.Handle, JniHandleOwnership.DoNotTransfer); @@ -66,7 +66,7 @@ public override string GetCurrentManagedThreadStackTrace (int skipFrames, bool f return throwable; } JniObjectReference.Dispose (ref reference, options); - var unwrapped = JNIEnvInit.ValueManager?.PeekValue (peeked!.PeerReference) as Exception; + var unwrapped = JniEnvironment.Runtime.ValueManager.PeekValue (peeked!.PeerReference) as Exception; if (unwrapped != null) { return unwrapped; } diff --git a/src/Mono.Android/Android.Runtime/JNIEnv.cs b/src/Mono.Android/Android.Runtime/JNIEnv.cs index f2aa7961b82..305d44081f6 100644 --- a/src/Mono.Android/Android.Runtime/JNIEnv.cs +++ b/src/Mono.Android/Android.Runtime/JNIEnv.cs @@ -118,7 +118,7 @@ internal static void PropagateUncaughtException (IntPtr env, IntPtr javaThreadPt public static void WaitForBridgeProcessing () { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); } public static IntPtr AllocObject (string jniClassName) diff --git a/src/Mono.Android/Android.Runtime/JNIEnvInit.cs b/src/Mono.Android/Android.Runtime/JNIEnvInit.cs index ad97a401a32..6ec510ff886 100644 --- a/src/Mono.Android/Android.Runtime/JNIEnvInit.cs +++ b/src/Mono.Android/Android.Runtime/JNIEnvInit.cs @@ -40,7 +40,6 @@ internal struct JnienvInitializeArgs { } #pragma warning restore 0649 - internal static JniRuntime.JniValueManager? ValueManager; internal static bool IsRunningOnDesktop; internal static bool jniRemappingInUse; internal static bool MarshalMethodsEnabled; @@ -88,7 +87,6 @@ static Type TypeGetType (string typeName) => internal static void InitializeJniRuntime (JniRuntime runtime) { androidRuntime = runtime; - ValueManager = runtime.ValueManager; SetSynchronizationContext (); } @@ -115,11 +113,12 @@ internal static unsafe void Initialize (JnienvInitializeArgs* args) JniRuntime.JniValueManager valueManager; if (RuntimeFeature.ManagedTypeMap) { typeManager = new ManagedTypeManager (); - valueManager = new ManagedValueManager (); } else { typeManager = new AndroidTypeManager (args->jniAddNativeMethodRegistrationAttributePresent != 0); - valueManager = RuntimeType == DotNetRuntimeType.MonoVM ? new AndroidValueManager () : new ManagedValueManager (); } + // TODO is there any reason why the ManagedTypeMap would need ManagedValueManager? + // ManagedValueManager is specifically tied to the CoreCLR JavaMarshal APIs and it does not work with MonoVM. + valueManager = RuntimeType == DotNetRuntimeType.MonoVM ? new AndroidValueManager () : ManagedValueManager.GetOrCreateInstance (); androidRuntime = new AndroidRuntime ( args->env, args->javaVm, @@ -128,7 +127,6 @@ internal static unsafe void Initialize (JnienvInitializeArgs* args) valueManager, args->jniAddNativeMethodRegistrationAttributePresent != 0 ); - ValueManager = androidRuntime.ValueManager; IsRunningOnDesktop = args->isRunningOnDesktop == 1; diff --git a/src/Mono.Android/Android.Runtime/JNINativeWrapper.g.cs b/src/Mono.Android/Android.Runtime/JNINativeWrapper.g.cs index c85e0feec16..9c7244b849a 100644 --- a/src/Mono.Android/Android.Runtime/JNINativeWrapper.g.cs +++ b/src/Mono.Android/Android.Runtime/JNINativeWrapper.g.cs @@ -17,7 +17,7 @@ static bool _unhandled_exception (Exception e) internal static void Wrap_JniMarshal_PP_V (this _JniMarshal_PP_V callback, IntPtr jnienv, IntPtr klazz) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz); } catch (Exception e) when (_unhandled_exception (e)) { @@ -28,7 +28,7 @@ internal static void Wrap_JniMarshal_PP_V (this _JniMarshal_PP_V callback, IntPt internal static int Wrap_JniMarshal_PP_I (this _JniMarshal_PP_I callback, IntPtr jnienv, IntPtr klazz) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz); } catch (Exception e) when (_unhandled_exception (e)) { @@ -39,7 +39,7 @@ internal static int Wrap_JniMarshal_PP_I (this _JniMarshal_PP_I callback, IntPtr internal static bool Wrap_JniMarshal_PP_Z (this _JniMarshal_PP_Z callback, IntPtr jnienv, IntPtr klazz) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz); } catch (Exception e) when (_unhandled_exception (e)) { @@ -50,7 +50,7 @@ internal static bool Wrap_JniMarshal_PP_Z (this _JniMarshal_PP_Z callback, IntPt internal static void Wrap_JniMarshal_PPI_V (this _JniMarshal_PPI_V callback, IntPtr jnienv, IntPtr klazz, int p0) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0); } catch (Exception e) when (_unhandled_exception (e)) { @@ -61,7 +61,7 @@ internal static void Wrap_JniMarshal_PPI_V (this _JniMarshal_PPI_V callback, Int internal static IntPtr Wrap_JniMarshal_PPI_L (this _JniMarshal_PPI_L callback, IntPtr jnienv, IntPtr klazz, int p0) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0); } catch (Exception e) when (_unhandled_exception (e)) { @@ -72,7 +72,7 @@ internal static IntPtr Wrap_JniMarshal_PPI_L (this _JniMarshal_PPI_L callback, I internal static int Wrap_JniMarshal_PPI_I (this _JniMarshal_PPI_I callback, IntPtr jnienv, IntPtr klazz, int p0) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0); } catch (Exception e) when (_unhandled_exception (e)) { @@ -83,7 +83,7 @@ internal static int Wrap_JniMarshal_PPI_I (this _JniMarshal_PPI_I callback, IntP internal static long Wrap_JniMarshal_PPI_J (this _JniMarshal_PPI_J callback, IntPtr jnienv, IntPtr klazz, int p0) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0); } catch (Exception e) when (_unhandled_exception (e)) { @@ -94,7 +94,7 @@ internal static long Wrap_JniMarshal_PPI_J (this _JniMarshal_PPI_J callback, Int internal static int Wrap_JniMarshal_PPL_I (this _JniMarshal_PPL_I callback, IntPtr jnienv, IntPtr klazz, IntPtr p0) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0); } catch (Exception e) when (_unhandled_exception (e)) { @@ -105,7 +105,7 @@ internal static int Wrap_JniMarshal_PPL_I (this _JniMarshal_PPL_I callback, IntP internal static IntPtr Wrap_JniMarshal_PPL_L (this _JniMarshal_PPL_L callback, IntPtr jnienv, IntPtr klazz, IntPtr p0) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0); } catch (Exception e) when (_unhandled_exception (e)) { @@ -116,7 +116,7 @@ internal static IntPtr Wrap_JniMarshal_PPL_L (this _JniMarshal_PPL_L callback, I internal static void Wrap_JniMarshal_PPL_V (this _JniMarshal_PPL_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0); } catch (Exception e) when (_unhandled_exception (e)) { @@ -127,7 +127,7 @@ internal static void Wrap_JniMarshal_PPL_V (this _JniMarshal_PPL_V callback, Int internal static bool Wrap_JniMarshal_PPL_Z (this _JniMarshal_PPL_Z callback, IntPtr jnienv, IntPtr klazz, IntPtr p0) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0); } catch (Exception e) when (_unhandled_exception (e)) { @@ -138,7 +138,7 @@ internal static bool Wrap_JniMarshal_PPL_Z (this _JniMarshal_PPL_Z callback, Int internal static bool Wrap_JniMarshal_PPJ_Z (this _JniMarshal_PPJ_Z callback, IntPtr jnienv, IntPtr klazz, long p0) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0); } catch (Exception e) when (_unhandled_exception (e)) { @@ -149,7 +149,7 @@ internal static bool Wrap_JniMarshal_PPJ_Z (this _JniMarshal_PPJ_Z callback, Int internal static void Wrap_JniMarshal_PPII_V (this _JniMarshal_PPII_V callback, IntPtr jnienv, IntPtr klazz, int p0, int p1) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1); } catch (Exception e) when (_unhandled_exception (e)) { @@ -160,7 +160,7 @@ internal static void Wrap_JniMarshal_PPII_V (this _JniMarshal_PPII_V callback, I internal static IntPtr Wrap_JniMarshal_PPII_L (this _JniMarshal_PPII_L callback, IntPtr jnienv, IntPtr klazz, int p0, int p1) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0, p1); } catch (Exception e) when (_unhandled_exception (e)) { @@ -171,7 +171,7 @@ internal static IntPtr Wrap_JniMarshal_PPII_L (this _JniMarshal_PPII_L callback, internal static void Wrap_JniMarshal_PPLI_V (this _JniMarshal_PPLI_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, int p1) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1); } catch (Exception e) when (_unhandled_exception (e)) { @@ -182,7 +182,7 @@ internal static void Wrap_JniMarshal_PPLI_V (this _JniMarshal_PPLI_V callback, I internal static void Wrap_JniMarshal_PPLZ_V (this _JniMarshal_PPLZ_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, bool p1) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1); } catch (Exception e) when (_unhandled_exception (e)) { @@ -193,7 +193,7 @@ internal static void Wrap_JniMarshal_PPLZ_V (this _JniMarshal_PPLZ_V callback, I internal static void Wrap_JniMarshal_PPLL_V (this _JniMarshal_PPLL_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, IntPtr p1) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1); } catch (Exception e) when (_unhandled_exception (e)) { @@ -204,7 +204,7 @@ internal static void Wrap_JniMarshal_PPLL_V (this _JniMarshal_PPLL_V callback, I internal static void Wrap_JniMarshal_PPLF_V (this _JniMarshal_PPLF_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, float p1) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1); } catch (Exception e) when (_unhandled_exception (e)) { @@ -215,7 +215,7 @@ internal static void Wrap_JniMarshal_PPLF_V (this _JniMarshal_PPLF_V callback, I internal static IntPtr Wrap_JniMarshal_PPLI_L (this _JniMarshal_PPLI_L callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, int p1) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0, p1); } catch (Exception e) when (_unhandled_exception (e)) { @@ -226,7 +226,7 @@ internal static IntPtr Wrap_JniMarshal_PPLI_L (this _JniMarshal_PPLI_L callback, internal static IntPtr Wrap_JniMarshal_PPLL_L (this _JniMarshal_PPLL_L callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, IntPtr p1) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0, p1); } catch (Exception e) when (_unhandled_exception (e)) { @@ -237,7 +237,7 @@ internal static IntPtr Wrap_JniMarshal_PPLL_L (this _JniMarshal_PPLL_L callback, internal static bool Wrap_JniMarshal_PPLL_Z (this _JniMarshal_PPLL_Z callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, IntPtr p1) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0, p1); } catch (Exception e) when (_unhandled_exception (e)) { @@ -248,7 +248,7 @@ internal static bool Wrap_JniMarshal_PPLL_Z (this _JniMarshal_PPLL_Z callback, I internal static bool Wrap_JniMarshal_PPIL_Z (this _JniMarshal_PPIL_Z callback, IntPtr jnienv, IntPtr klazz, int p0, IntPtr p1) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0, p1); } catch (Exception e) when (_unhandled_exception (e)) { @@ -259,7 +259,7 @@ internal static bool Wrap_JniMarshal_PPIL_Z (this _JniMarshal_PPIL_Z callback, I internal static void Wrap_JniMarshal_PPIIL_V (this _JniMarshal_PPIIL_V callback, IntPtr jnienv, IntPtr klazz, int p0, int p1, IntPtr p2) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1, p2); } catch (Exception e) when (_unhandled_exception (e)) { @@ -270,7 +270,7 @@ internal static void Wrap_JniMarshal_PPIIL_V (this _JniMarshal_PPIIL_V callback, internal static int Wrap_JniMarshal_PPLII_I (this _JniMarshal_PPLII_I callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, int p1, int p2) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0, p1, p2); } catch (Exception e) when (_unhandled_exception (e)) { @@ -281,7 +281,7 @@ internal static int Wrap_JniMarshal_PPLII_I (this _JniMarshal_PPLII_I callback, internal static bool Wrap_JniMarshal_PPLII_Z (this _JniMarshal_PPLII_Z callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, int p1, int p2) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0, p1, p2); } catch (Exception e) when (_unhandled_exception (e)) { @@ -292,7 +292,7 @@ internal static bool Wrap_JniMarshal_PPLII_Z (this _JniMarshal_PPLII_Z callback, internal static void Wrap_JniMarshal_PPLII_V (this _JniMarshal_PPLII_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, int p1, int p2) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1, p2); } catch (Exception e) when (_unhandled_exception (e)) { @@ -303,7 +303,7 @@ internal static void Wrap_JniMarshal_PPLII_V (this _JniMarshal_PPLII_V callback, internal static void Wrap_JniMarshal_PPIII_V (this _JniMarshal_PPIII_V callback, IntPtr jnienv, IntPtr klazz, int p0, int p1, int p2) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1, p2); } catch (Exception e) when (_unhandled_exception (e)) { @@ -314,7 +314,7 @@ internal static void Wrap_JniMarshal_PPIII_V (this _JniMarshal_PPIII_V callback, internal static bool Wrap_JniMarshal_PPLLJ_Z (this _JniMarshal_PPLLJ_Z callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, IntPtr p1, long p2) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0, p1, p2); } catch (Exception e) when (_unhandled_exception (e)) { @@ -325,7 +325,7 @@ internal static bool Wrap_JniMarshal_PPLLJ_Z (this _JniMarshal_PPLLJ_Z callback, internal static void Wrap_JniMarshal_PPILL_V (this _JniMarshal_PPILL_V callback, IntPtr jnienv, IntPtr klazz, int p0, IntPtr p1, IntPtr p2) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1, p2); } catch (Exception e) when (_unhandled_exception (e)) { @@ -336,7 +336,7 @@ internal static void Wrap_JniMarshal_PPILL_V (this _JniMarshal_PPILL_V callback, internal static bool Wrap_JniMarshal_PPLIL_Z (this _JniMarshal_PPLIL_Z callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, int p1, IntPtr p2) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0, p1, p2); } catch (Exception e) when (_unhandled_exception (e)) { @@ -347,7 +347,7 @@ internal static bool Wrap_JniMarshal_PPLIL_Z (this _JniMarshal_PPLIL_Z callback, internal static void Wrap_JniMarshal_PPLLL_V (this _JniMarshal_PPLLL_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, IntPtr p1, IntPtr p2) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1, p2); } catch (Exception e) when (_unhandled_exception (e)) { @@ -358,7 +358,7 @@ internal static void Wrap_JniMarshal_PPLLL_V (this _JniMarshal_PPLLL_V callback, internal static IntPtr Wrap_JniMarshal_PPLLL_L (this _JniMarshal_PPLLL_L callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, IntPtr p1, IntPtr p2) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0, p1, p2); } catch (Exception e) when (_unhandled_exception (e)) { @@ -369,7 +369,7 @@ internal static IntPtr Wrap_JniMarshal_PPLLL_L (this _JniMarshal_PPLLL_L callbac internal static bool Wrap_JniMarshal_PPLLL_Z (this _JniMarshal_PPLLL_Z callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, IntPtr p1, IntPtr p2) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0, p1, p2); } catch (Exception e) when (_unhandled_exception (e)) { @@ -380,7 +380,7 @@ internal static bool Wrap_JniMarshal_PPLLL_Z (this _JniMarshal_PPLLL_Z callback, internal static IntPtr Wrap_JniMarshal_PPIZI_L (this _JniMarshal_PPIZI_L callback, IntPtr jnienv, IntPtr klazz, int p0, bool p1, int p2) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0, p1, p2); } catch (Exception e) when (_unhandled_exception (e)) { @@ -391,7 +391,7 @@ internal static IntPtr Wrap_JniMarshal_PPIZI_L (this _JniMarshal_PPIZI_L callbac internal static void Wrap_JniMarshal_PPIIII_V (this _JniMarshal_PPIIII_V callback, IntPtr jnienv, IntPtr klazz, int p0, int p1, int p2, int p3) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1, p2, p3); } catch (Exception e) when (_unhandled_exception (e)) { @@ -402,7 +402,7 @@ internal static void Wrap_JniMarshal_PPIIII_V (this _JniMarshal_PPIIII_V callbac internal static void Wrap_JniMarshal_PPLLLL_V (this _JniMarshal_PPLLLL_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, IntPtr p1, IntPtr p2, IntPtr p3) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1, p2, p3); } catch (Exception e) when (_unhandled_exception (e)) { @@ -413,7 +413,7 @@ internal static void Wrap_JniMarshal_PPLLLL_V (this _JniMarshal_PPLLLL_V callbac internal static bool Wrap_JniMarshal_PPLZZL_Z (this _JniMarshal_PPLZZL_Z callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, bool p1, bool p2, IntPtr p3) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { return callback (jnienv, klazz, p0, p1, p2, p3); } catch (Exception e) when (_unhandled_exception (e)) { @@ -424,7 +424,7 @@ internal static bool Wrap_JniMarshal_PPLZZL_Z (this _JniMarshal_PPLZZL_Z callbac internal static void Wrap_JniMarshal_PPLIIII_V (this _JniMarshal_PPLIIII_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, int p1, int p2, int p3, int p4) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1, p2, p3, p4); } catch (Exception e) when (_unhandled_exception (e)) { @@ -435,7 +435,7 @@ internal static void Wrap_JniMarshal_PPLIIII_V (this _JniMarshal_PPLIIII_V callb internal static void Wrap_JniMarshal_PPZIIII_V (this _JniMarshal_PPZIIII_V callback, IntPtr jnienv, IntPtr klazz, bool p0, int p1, int p2, int p3, int p4) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1, p2, p3, p4); } catch (Exception e) when (_unhandled_exception (e)) { @@ -446,7 +446,7 @@ internal static void Wrap_JniMarshal_PPZIIII_V (this _JniMarshal_PPZIIII_V callb internal static void Wrap_JniMarshal_PPLIIIIIIII_V (this _JniMarshal_PPLIIIIIIII_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, int p1, int p2, int p3, int p4, int p5, int p6, int p7, int p8) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { callback (jnienv, klazz, p0, p1, p2, p3, p4, p5, p6, p7, p8); } catch (Exception e) when (_unhandled_exception (e)) { diff --git a/src/Mono.Android/Android.Runtime/JNINativeWrapper.g.tt b/src/Mono.Android/Android.Runtime/JNINativeWrapper.g.tt index 0dc1e4e9810..425dd99bcdb 100644 --- a/src/Mono.Android/Android.Runtime/JNINativeWrapper.g.tt +++ b/src/Mono.Android/Android.Runtime/JNINativeWrapper.g.tt @@ -268,7 +268,7 @@ foreach (var info in delegateTypes) { #> internal static <#= info.Return #> Wrap<#= info.Type #> (this <#= info.Type #> callback, <#= info.Signature #>) { - JNIEnvInit.ValueManager?.WaitForGCBridgeProcessing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); try { <#= info.Return != "void" ? "return " : "" #>callback (<#= info.Invoke #>); } catch (Exception e) when (_unhandled_exception (e)) { diff --git a/src/Mono.Android/Android.Runtime/RuntimeNativeMethods.cs b/src/Mono.Android/Android.Runtime/RuntimeNativeMethods.cs index e740dfdb4c9..ae7042b4efa 100644 --- a/src/Mono.Android/Android.Runtime/RuntimeNativeMethods.cs +++ b/src/Mono.Android/Android.Runtime/RuntimeNativeMethods.cs @@ -2,6 +2,7 @@ using System; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; +using System.Runtime.InteropServices.Java; using System.Text; namespace Android.Runtime @@ -18,7 +19,7 @@ enum TraceKind : uint All = Java | Managed | Native | Signals, } - internal static class RuntimeNativeMethods + internal unsafe static class RuntimeNativeMethods { [DllImport (RuntimeConstants.InternalDllName, CallingConvention = CallingConvention.Cdecl)] internal extern static void monodroid_log (LogLevel level, LogCategories category, string message); @@ -92,6 +93,11 @@ internal static class RuntimeNativeMethods [DllImport (RuntimeConstants.InternalDllName, CallingConvention = CallingConvention.Cdecl)] internal static extern bool clr_typemap_java_to_managed (string java_type_name, out IntPtr managed_assembly_name, out uint managed_type_token_id); + [DllImport (RuntimeConstants.InternalDllName, CallingConvention = CallingConvention.Cdecl)] + internal static extern delegate* unmanaged clr_initialize_gc_bridge ( + delegate* unmanaged bridge_processing_started_callback, + delegate* unmanaged bridge_processing_finished_callback); + [MethodImplAttribute(MethodImplOptions.InternalCall)] internal static extern void monodroid_unhandled_exception (Exception javaException); diff --git a/src/Mono.Android/Java.Interop/Runtime.cs b/src/Mono.Android/Java.Interop/Runtime.cs index 8d69aedffa4..314815a9edb 100644 --- a/src/Mono.Android/Java.Interop/Runtime.cs +++ b/src/Mono.Android/Java.Interop/Runtime.cs @@ -11,7 +11,7 @@ public static class Runtime { [Obsolete ("Please use Java.Interop.JniEnvironment.Runtime.ValueManager.GetSurfacedPeers()")] public static List GetSurfacedObjects () { - var peers = JNIEnvInit.ValueManager!.GetSurfacedPeers (); + var peers = JniEnvironment.Runtime.ValueManager.GetSurfacedPeers (); var r = new List (peers.Count); foreach (var p in peers) { if (p.SurfacedPeer.TryGetTarget (out var target)) diff --git a/src/Mono.Android/Java.Lang/Object.cs b/src/Mono.Android/Java.Lang/Object.cs index 58a7e54d88b..44c9519fb4d 100644 --- a/src/Mono.Android/Java.Lang/Object.cs +++ b/src/Mono.Android/Java.Lang/Object.cs @@ -111,7 +111,7 @@ protected void SetHandle (IntPtr value, JniHandleOwnership transfer) { var reference = new JniObjectReference (value); var options = FromJniHandleOwnership (transfer); - JNIEnvInit.ValueManager?.ConstructPeer ( + JniEnvironment.Runtime.ValueManager.ConstructPeer ( this, ref reference, value == IntPtr.Zero ? JniObjectReferenceOptions.None : options); @@ -128,7 +128,7 @@ static JniObjectReferenceOptions FromJniHandleOwnership (JniHandleOwnership tran internal static IJavaPeerable? PeekObject (IntPtr handle, Type? requiredType = null) { - var peeked = JNIEnvInit.ValueManager?.PeekPeer (new JniObjectReference (handle)); + var peeked = JniEnvironment.Runtime.ValueManager.PeekPeer (new JniObjectReference (handle)); if (peeked == null) return null; if (requiredType != null && !requiredType.IsAssignableFrom (peeked.GetType ())) @@ -180,7 +180,7 @@ internal static T? _GetObject< if (handle == IntPtr.Zero) return null; - var r = JNIEnvInit.ValueManager!.GetPeer (new JniObjectReference (handle), type); + var r = JniEnvironment.Runtime.ValueManager.GetPeer (new JniObjectReference (handle), type); JNIEnv.DeleteRef (handle, transfer); return r; } diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index c58cfbab431..112c47e9d1a 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -10,6 +10,7 @@ using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; +using System.Runtime.InteropServices.Java; using System.Threading; using Android.Runtime; using Java.Interop; @@ -20,43 +21,26 @@ class ManagedValueManager : JniRuntime.JniValueManager { const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors; - Dictionary>? RegisteredInstances = new Dictionary>(); + Dictionary>? RegisteredInstances = new (); - internal ManagedValueManager () + static Lazy s_instance = new (() => new ManagedValueManager ()); + public static ManagedValueManager GetOrCreateInstance () => s_instance.Value; + + unsafe ManagedValueManager () { + // There can only be one instance of ManagedValueManager because we can call JavaMarshal.Initialize only once. + var mark_cross_references_ftn = RuntimeNativeMethods.clr_initialize_gc_bridge (&BridgeProcessingStarted, &BridgeProcessingFinished); + JavaMarshal.Initialize (mark_cross_references_ftn); } public override void WaitForGCBridgeProcessing () { + AndroidRuntimeInternal.WaitForBridgeProcessing (); } public override void CollectPeers () { - if (RegisteredInstances == null) - throw new ObjectDisposedException (nameof (ManagedValueManager)); - - var peers = new List (); - - lock (RegisteredInstances) { - foreach (var ps in RegisteredInstances.Values) { - foreach (var p in ps) { - peers.Add (p); - } - } - RegisteredInstances.Clear (); - } - List? exceptions = null; - foreach (var peer in peers) { - try { - peer.Dispose (); - } - catch (Exception e) { - exceptions = exceptions ?? new List (); - exceptions.Add (e); - } - } - if (exceptions != null) - throw new AggregateException ("Exceptions while collecting peers.", exceptions); + GC.Collect (); } public override void AddPeer (IJavaPeerable value) @@ -74,35 +58,31 @@ public override void AddPeer (IJavaPeerable value) } int key = value.JniIdentityHashCode; lock (RegisteredInstances) { - List? peers; + List? peers; if (!RegisteredInstances.TryGetValue (key, out peers)) { - peers = new List () { - value, - }; + peers = [new WeakPeerReference (value)]; RegisteredInstances.Add (key, peers); return; } for (int i = peers.Count - 1; i >= 0; i--) { var p = peers [i]; - if (!JniEnvironment.Types.IsSameObject (p.PeerReference, value.PeerReference)) + if (p.Target is not IJavaPeerable peer) continue; - if (Replaceable (p)) { - peers [i] = value; + if (!JniEnvironment.Types.IsSameObject (peer.PeerReference, value.PeerReference)) + continue; + if (peer.JniManagedPeerState.HasFlag (JniManagedPeerStates.Replaceable)) { + p.Dispose (); + peers [i] = new WeakPeerReference (value); + GC.KeepAlive (peer); } else { - WarnNotReplacing (key, value, p); + WarnNotReplacing (key, value, peer); } return; } - peers.Add (value); - } - } - static bool Replaceable (IJavaPeerable peer) - { - if (peer == null) - return true; - return peer.JniManagedPeerState.HasFlag (JniManagedPeerStates.Replaceable); + peers.Add (new WeakPeerReference (value)); + } } void WarnNotReplacing (int key, IJavaPeerable ignoreValue, IJavaPeerable keepValue) @@ -132,15 +112,17 @@ void WarnNotReplacing (int key, IJavaPeerable ignoreValue, IJavaPeerable keepVal int key = GetJniIdentityHashCode (reference); lock (RegisteredInstances) { - List? peers; - if (!RegisteredInstances.TryGetValue (key, out peers)) + if (!RegisteredInstances.TryGetValue (key, out List? peers)) return null; for (int i = peers.Count - 1; i >= 0; i--) { - var p = peers [i]; - if (JniEnvironment.Types.IsSameObject (reference, p.PeerReference)) - return p; + if (peers [i].Target is IJavaPeerable peer + && JniEnvironment.Types.IsSameObject (reference, peer.PeerReference)) + { + return peer; + } } + if (peers.Count == 0) RegisteredInstances.Remove (key); } @@ -155,21 +137,30 @@ public override void RemovePeer (IJavaPeerable value) if (value == null) throw new ArgumentNullException (nameof (value)); - int key = value.JniIdentityHashCode; lock (RegisteredInstances) { - List? peers; - if (!RegisteredInstances.TryGetValue (key, out peers)) - return; + RemoveRegisteredInstance (value, out WeakPeerReference? removedPeer); + removedPeer?.Dispose (); + GC.KeepAlive (value); + } + } - for (int i = peers.Count - 1; i >= 0; i--) { - var p = peers [i]; - if (object.ReferenceEquals (value, p)) { - peers.RemoveAt (i); - } + private void RemoveRegisteredInstance (IJavaPeerable target, out WeakPeerReference? removedPeer) + { + removedPeer = default; + + int key = target.JniIdentityHashCode; + if (!RegisteredInstances.TryGetValue (key, out List? peers)) + return; + + for (int i = peers.Count - 1; i >= 0; i--) { + var peer = peers [i]; + if (object.ReferenceEquals (target, peer.Target)) { + peers.RemoveAt (i); + removedPeer = peer; } - if (peers.Count == 0) - RegisteredInstances.Remove (key); } + if (peers.Count == 0) + RegisteredInstances.Remove (key); } public override void FinalizePeer (IJavaPeerable value) @@ -251,16 +242,102 @@ public override List GetSurfacedPeers () var peers = new List (RegisteredInstances.Count); foreach (var e in RegisteredInstances) { foreach (var p in e.Value) { - peers.Add (new JniSurfacedPeerInfo (e.Key, new WeakReference (p))); + if (p.Target is IJavaPeerable peer) { + peers.Add (new JniSurfacedPeerInfo (e.Key, new WeakReference (peer))); + } } } return peers; } } - const BindingFlags ActivationConstructorBindingFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; + unsafe struct WeakPeerReference : IDisposable + { + private WeakReference _weakReference; + private GCHandle _handle; + + public WeakPeerReference (IJavaPeerable peer) + { + _weakReference = new WeakReference (peer); + + var handleContext = (HandleContext*) NativeMemory.AllocZeroed (1, HandleContext.Size); + if (handleContext == null) { + throw new OutOfMemoryException ("Failed to allocate memory for HandleContext."); + } + + _handle = JavaMarshal.CreateReferenceTrackingHandle (peer, handleContext); + + handleContext->Handle = GCHandle.ToIntPtr (_handle); + handleContext->ControlBlock = peer.JniObjectReferenceControlBlock; + } + + public void FreeContext () + { + var context = (HandleContext*) JavaMarshal.GetContext (_handle); + if (context != null) { + NativeMemory.Free (context); + } + } + + /// + /// Calling this Dispose method is legal only if the caller holds a reference to the Target + /// to prevent data race with the GC. + /// + public void Dispose() + { + FreeContext (); + _handle.Free (); + } + + public IJavaPeerable? Target + => _weakReference.TryGetTarget (out var target) ? target : null; + } + + [StructLayout (LayoutKind.Sequential)] + struct HandleContext + { + public static readonly nuint Size = (nuint) Marshal.SizeOf (); + + public IntPtr Handle; + public IntPtr ControlBlock; + } + + [UnmanagedCallersOnly] + internal static void BridgeProcessingStarted () + { + AndroidRuntimeInternal.BridgeProcessing = true; + } + + [UnmanagedCallersOnly] + internal static unsafe void BridgeProcessingFinished (MarkCrossReferencesArgs* mcr) + { + List handlesToFree = []; + ManagedValueManager instance = GetOrCreateInstance (); + + lock (instance.RegisteredInstances) { + for (int i = 0; (nuint)i < mcr->ComponentCount; i++) { + for (int j = 0; (nuint)j < mcr->Components [i].Count; j++) { + var context = (HandleContext*) mcr->Components [i].Contexts [j]; + if (context->ControlBlock == IntPtr.Zero) { + var handle = GCHandle.FromIntPtr (context->Handle); + + // Clean up the RegisteredInstances dictionary + free the control block native memory associated with the handle + instance.RemoveRegisteredInstance ((IJavaPeerable)handle.Target, out WeakPeerReference? removedPeer); + removedPeer?.FreeContext (); + + handlesToFree.Add (handle); + } + } + } + } + + JavaMarshal.FinishCrossReferenceProcessing (mcr, CollectionsMarshal.AsSpan (handlesToFree)); + AndroidRuntimeInternal.BridgeProcessing = false; + } + + const BindingFlags ActivationConstructorBindingFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; - static readonly Type[] XAConstructorSignature = new Type [] { typeof (IntPtr), typeof (JniHandleOwnership) }; + static readonly Type[] XAConstructorSignature = new Type[] { typeof (IntPtr), typeof (JniHandleOwnership) }; protected override bool TryConstructPeer ( IJavaPeerable self, @@ -284,8 +361,7 @@ protected override bool TryConstructPeer ( protected override bool TryUnboxPeerObject (IJavaPeerable value, [NotNullWhen (true)]out object? result) { - var proxy = value as JavaProxyThrowable; - if (proxy != null) { + if (value is JavaProxyThrowable proxy) { result = proxy.InnerException; return true; } diff --git a/src/native/clr/host/CMakeLists.txt b/src/native/clr/host/CMakeLists.txt index d2338f4f072..9ddaa13234f 100644 --- a/src/native/clr/host/CMakeLists.txt +++ b/src/native/clr/host/CMakeLists.txt @@ -29,6 +29,7 @@ set(XAMARIN_NET_ANDROID_STATIC_LIB "${XAMARIN_NET_ANDROID_LIB}-static") set(XAMARIN_MONODROID_SOURCES assembly-store.cc + gc-bridge.cc host.cc host-jni.cc host-util.cc diff --git a/src/native/clr/host/gc-bridge.cc b/src/native/clr/host/gc-bridge.cc new file mode 100644 index 00000000000..714e52a03e5 --- /dev/null +++ b/src/native/clr/host/gc-bridge.cc @@ -0,0 +1,315 @@ +#include +#include +#include +#include +#include +#include +#include + +using namespace xamarin::android; + +void GCBridge::initialize_on_load (JNIEnv *jniEnv) noexcept +{ + abort_if_invalid_pointer_argument (jniEnv, "jniEnv"); + + jclass lref = jniEnv->FindClass ("java/lang/Runtime"); + abort_unless (lref != nullptr, "Failed to look up java/lang/Runtime class."); + + jmethodID Runtime_getRuntime = jniEnv->GetStaticMethodID (lref, "getRuntime", "()Ljava/lang/Runtime;"); + abort_unless (Runtime_getRuntime != nullptr, "Failed to look up the Runtime.getRuntime() method."); + + Runtime_gc = jniEnv->GetMethodID (lref, "gc", "()V"); + abort_unless (Runtime_gc != nullptr, "Failed to look up the Runtime.gc() method."); + + Runtime_instance = OSBridge::lref_to_gref (jniEnv, jniEnv->CallStaticObjectMethod (lref, Runtime_getRuntime)); + abort_unless (Runtime_instance != nullptr, "Failed to obtain Runtime instance."); + + jniEnv->DeleteLocalRef (lref); +} + +void GCBridge::trigger_java_gc () noexcept +{ + env->CallVoidMethod (Runtime_instance, Runtime_gc); + if (env->ExceptionCheck ()) [[unlikely]] { + env->ExceptionDescribe (); + env->ExceptionClear (); + log_error (LOG_DEFAULT, "Java GC failed"); + } +} + +void GCBridge::prepare_for_java_collection (MarkCrossReferencesArgs* cross_refs) noexcept +{ + // Some SCCs might have no IGCUserPeers associated with them, so we must create one + std::unordered_map temporary_peers; + + // Before looking at xrefs, scan the SCCs. During collection, an SCC has to behave like a + // single object. If the number of objects in the SCC is anything other than 1, the SCC + // must be doctored to mimic that one-object nature. + for (size_t i = 0; i < cross_refs->ComponentCount; i++) { + StronglyConnectedComponent *scc = &cross_refs->Components [i]; + + // Count > 1 case: The SCC contains many objects which must be collected as one. + // Solution: Make all objects within the SCC directly or indirectly reference each other + if (scc->Count > 1) { + add_references (scc); + } else if (scc->Count == 0) { + temporary_peers [scc] = env->NewObject (GCUserPeer_class, GCUserPeer_ctor); + } + } + + // Add the cross scc refs + for (size_t i = 0; i < cross_refs->CrossReferenceCount; i++) { + ComponentCrossReference *xref = &cross_refs->CrossReferences [i]; + + StronglyConnectedComponent *source = &cross_refs->Components [xref->SourceGroupIndex]; + jobject from = source->Count > 0 ? source->Contexts [0]->control_block->handle : temporary_peers [source]; + + StronglyConnectedComponent *destination = &cross_refs->Components [xref->DestinationGroupIndex]; + jobject to = destination->Count > 0 ? destination->Contexts [0]->control_block->handle : temporary_peers [destination]; + + add_reference (from, to); + } + + // With cross references processed, the temporary peer list can be released + for (const auto& [scc, temporary_peer] : temporary_peers) { + env->DeleteLocalRef (temporary_peer); + } + + // Switch global to weak references + for (size_t i = 0; i < cross_refs->ComponentCount; i++) { + StronglyConnectedComponent *scc = &cross_refs->Components [i]; + for (size_t j = 0; j < scc->Count; j++) { + take_weak_global_ref (scc->Contexts [j]); + } + } +} + +void GCBridge::add_references (StronglyConnectedComponent *scc) noexcept +{ + abort_if_invalid_pointer_argument (scc, "scc"); + abort_unless (scc->Count > 1, "SCC must have at least two items to add inner references"); + + JniObjectReferenceControlBlock *prev = scc->Contexts [scc->Count - 1]->control_block; + + for (size_t j = 1; j < scc->Count; j++) { + JniObjectReferenceControlBlock *current = scc->Contexts [j]->control_block; + if (add_reference (prev->handle, current->handle)) { + prev->refs_added++; + } + + prev = current; + } +} + +bool GCBridge::add_reference (jobject from, jobject to) noexcept +{ + jclass java_class = env->GetObjectClass (from); + jmethodID add_method_id = env->GetMethodID (java_class, "monodroidAddReference", "(Ljava/lang/Object;)V"); + env->DeleteLocalRef (java_class); + + if (add_method_id) { + env->CallVoidMethod (from, add_method_id, to); + return true; + } else { + env->ExceptionClear (); + return false; + } +} + +void GCBridge::clear_references (jobject handle) noexcept +{ + // Clear references from the object + jclass java_class = env->GetObjectClass (handle); + jmethodID clear_method_id = env->GetMethodID (java_class, "monodroidClearReferences", "()V"); + env->DeleteLocalRef (java_class); // Clean up the local reference to the class + + if (clear_method_id) { + env->CallVoidMethod (handle, clear_method_id); + } else { + log_error (LOG_DEFAULT, "Failed to find monodroidClearReferences method"); + env->ExceptionClear (); +#if DEBUG + if (Logger::gc_spew_enabled ()) { + char *class_name = Host::get_java_class_name_for_TypeManager (java_class); + log_error (LOG_GC, "Missing monodroidClearReferences method for object of class {}", class_name); + free (class_name); + } +#endif + } +} + +void GCBridge::take_global_ref (HandleContext *context) noexcept +{ + abort_unless (context->control_block->handle_type == JNIWeakGlobalRefType, "Expected weak global reference type for handle"); + + jobject weak = context->control_block->handle; + jobject handle = env->NewGlobalRef (weak); + + if (Logger::gref_log ()) [[unlikely]] { + char *message = Util::monodroid_strdup_printf ("take_global_ref gchandle=%p -> wref=%p handle=%p\n"sv, context->gc_handle, weak, handle); + OSBridge::_monodroid_gref_log (message); + free (message); + } + + if (handle != nullptr) { + context->control_block->handle = handle; + context->control_block->handle_type = JNIGlobalRefType; + + OSBridge::_monodroid_gref_log_new (weak, OSBridge::get_object_ref_type (env, weak), + handle, OSBridge::get_object_ref_type (env, handle), + "finalizer", gettid (), + " at [[clr-gc:take_global_ref]]", 0); + + OSBridge::_monodroid_weak_gref_delete (weak, OSBridge::get_object_ref_type (env, weak), + "finalizer", gettid (), " at [[clr-gc:take_global_ref]]", 0); + env->DeleteWeakGlobalRef (weak); + } else { + // The native memory of the control block will be freed in managed code as well as the weak global ref + context->control_block = nullptr; + + if (Logger::gc_spew_enabled ()) [[unlikely]] { + char *message = Util::monodroid_strdup_printf ("handle %p/W; was collected by a Java GC"sv, weak); + OSBridge::_monodroid_gref_log (message); + free (message); + } + } +} + +void GCBridge::take_weak_global_ref (HandleContext *context) noexcept +{ + jobject handle = context->control_block->handle; + if (Logger::gref_log ()) [[unlikely]] { + char *message = Util::monodroid_strdup_printf ("take_weak_global_ref gchandle=%p; handle=%p\n"sv, context->gc_handle, handle); + OSBridge::_monodroid_gref_log (message); + free (message); + } + + jobject weak = env->NewWeakGlobalRef (handle); + OSBridge::_monodroid_weak_gref_new (handle, OSBridge::get_object_ref_type (env, handle), + weak, OSBridge::get_object_ref_type (env, weak), + "finalizer", gettid (), " at [[clr-gc:take_weak_global_ref]]", 0); + + context->control_block->handle = weak; + context->control_block->handle_type = JNIWeakGlobalRefType; + + OSBridge::_monodroid_gref_log_delete (handle, OSBridge::get_object_ref_type (env, handle), + "finalizer", gettid (), " at [[clr-gc:take_weak_global_ref]]", 0); + env->DeleteGlobalRef (handle); +} + +void GCBridge::cleanup_after_java_collection (MarkCrossReferencesArgs* cross_refs) noexcept +{ +#if DEBUG + int total = 0; + int alive = 0; +#endif + + // try to switch back to global refs to analyze what stayed alive + for (size_t i = 0; i < cross_refs->ComponentCount; i++) { + StronglyConnectedComponent *scc = &cross_refs->Components [i]; + for (size_t j = 0; j < scc->Count; j++) { +#if DEBUG + total++; +#endif + take_global_ref (scc->Contexts [j]); + } + } + + // clear the cross references on any remaining items + for (size_t i = 0; i < cross_refs->ComponentCount; i++) { + StronglyConnectedComponent *scc = &cross_refs->Components [i]; + bool is_alive = false; + + for (size_t j = 0; j < scc->Count; j++) { + JniObjectReferenceControlBlock *control_block = scc->Contexts [j]->control_block; + + if (control_block != nullptr) { +#if DEBUG + alive++; +#endif + if (j > 0) { + abort_unless (is_alive, [&i] { return detail::_format_message ("Bridge SCC at index %d must be alive", i); }); + } + + is_alive = true; + if (control_block->refs_added > 0) { + clear_references (control_block->handle); + control_block->refs_added = 0; + } + } else { + abort_unless (!is_alive, [&i] { return detail::_format_message ("Bridge SCC at index %d must NOT be alive", i); }); + } + } + } + +#if DEBUG + log_info (LOG_GC, "GC cleanup summary: {} objects tested - resurrecting {}.", total, alive); +#endif +} + +void GCBridge::bridge_processing () noexcept +{ + abort_unless (bridge_processing_started_callback != nullptr, "GC bridge processing started callback is not set"); + abort_unless (bridge_processing_finished_callback != nullptr, "GC bridge processing finished callback is not set"); + + env = OSBridge::ensure_jnienv (); + + while (true) { + bridge_processing_semaphore.acquire (); + std::unique_lock lock (processing_mutex); + + MarkCrossReferencesArgs* args = &GCBridge::shared_cross_refs; + log_mark_cross_references_args_if_enabled (args); + + bridge_processing_started_callback (); + prepare_for_java_collection (args); + trigger_java_gc (); + cleanup_after_java_collection (args); + bridge_processing_finished_callback (args); + } +} + +void GCBridge::mark_cross_references (MarkCrossReferencesArgs* cross_refs) noexcept +{ + std::unique_lock lock (processing_mutex); + + GCBridge::shared_cross_refs.ComponentCount = cross_refs->ComponentCount; + GCBridge::shared_cross_refs.Components = cross_refs->Components; + GCBridge::shared_cross_refs.CrossReferenceCount = cross_refs->CrossReferenceCount; + GCBridge::shared_cross_refs.CrossReferences = cross_refs->CrossReferences; + + bridge_processing_semaphore.release (); +} + +void GCBridge::wait_for_bridge_processing () noexcept +{ + std::shared_lock lock (processing_mutex); +} + +[[gnu::always_inline]] +void GCBridge::log_mark_cross_references_args_if_enabled (MarkCrossReferencesArgs* args) noexcept +{ + if (Logger::gc_spew_enabled ()) [[unlikely]] { + log_info (LOG_GC, "cross references callback invoked with {} sccs and {} xrefs.", args->ComponentCount, args->CrossReferenceCount); + + for (size_t i = 0; i < args->ComponentCount; ++i) { + log_info (LOG_GC, "group {} with {} objects", i, args->Components [i].Count); + for (size_t j = 0; j < args->Components [i].Count; ++j) { + HandleContext *ctx = args->Components [i].Contexts [j]; + jobject handle = ctx->control_block->handle; + jclass java_class = env->GetObjectClass (handle); + char *class_name = Host::get_java_class_name_for_TypeManager (java_class); + log_info (LOG_GC, "\tgchandle {:p} gref {:p} [{}]", reinterpret_cast (ctx->gc_handle), reinterpret_cast (handle), class_name); + free (class_name); + } + } + + if (Util::should_log (LOG_GC)) { + for (size_t i = 0; i < args->CrossReferenceCount; ++i) { + size_t source_index = args->CrossReferences [i].SourceGroupIndex; + size_t dest_index = args->CrossReferences [i].DestinationGroupIndex; + log_info_nocheck_fmt (LOG_GC, "xref [{}] {} -> {}", i, source_index, dest_index); + } + } + } +} diff --git a/src/native/clr/host/generate-pinvoke-tables.cc b/src/native/clr/host/generate-pinvoke-tables.cc index 8f31936cdd7..0b354cae033 100644 --- a/src/native/clr/host/generate-pinvoke-tables.cc +++ b/src/native/clr/host/generate-pinvoke-tables.cc @@ -72,6 +72,7 @@ const std::vector internal_pinvoke_names = { "monodroid_TypeManager_get_java_class_name", "clr_typemap_managed_to_java", "clr_typemap_java_to_managed", + "clr_initialize_gc_bridge", "_monodroid_weak_gref_delete", "_monodroid_weak_gref_get", "_monodroid_weak_gref_new", diff --git a/src/native/clr/host/host.cc b/src/native/clr/host/host.cc index c48d1a7ca9c..d5b4463f63f 100644 --- a/src/native/clr/host/host.cc +++ b/src/native/clr/host/host.cc @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -574,6 +575,7 @@ auto Host::Java_JNI_OnLoad (JavaVM *vm, [[maybe_unused]] void *reserved) noexcep JNIEnv *env = nullptr; vm->GetEnv ((void**)&env, JNI_VERSION_1_6); OSBridge::initialize_on_onload (vm, env); + GCBridge::initialize_on_load (env); AndroidSystem::init_max_gref_count (); return JNI_VERSION_1_6; diff --git a/src/native/clr/host/internal-pinvokes.cc b/src/native/clr/host/internal-pinvokes.cc index a490a8b069e..0fb9dc341f0 100644 --- a/src/native/clr/host/internal-pinvokes.cc +++ b/src/native/clr/host/internal-pinvokes.cc @@ -1,3 +1,4 @@ +#include #include #include #include @@ -53,6 +54,13 @@ bool clr_typemap_java_to_managed (const char *java_type_name, char const** assem return TypeMapper::java_to_managed (java_type_name, assembly_name, managed_type_token_id); } +BridgeProcessingFtn clr_initialize_gc_bridge ( + BridgeProcessingStartedFtn bridge_processing_started_callback, + BridgeProcessingFinishedFtn bridge_processing_finished_callback) noexcept +{ + return GCBridge::initialize_callback (bridge_processing_started_callback, bridge_processing_finished_callback); +} + void monodroid_log (LogLevel level, LogCategories category, const char *message) noexcept { switch (level) { @@ -175,7 +183,7 @@ void _monodroid_lref_log_delete (int lrefc, jobject handle, char type, const cha void _monodroid_gc_wait_for_bridge_processing () { - // mono_gc_wait_for_bridge_processing (); - replace with the new GC bridge call, when we have it + GCBridge::wait_for_bridge_processing (); } void _monodroid_detect_cpu_and_architecture (uint16_t *built_for_cpu, uint16_t *running_on_cpu, unsigned char *is64bit) diff --git a/src/native/clr/host/os-bridge.cc b/src/native/clr/host/os-bridge.cc index 249f33432c1..904ad36c523 100644 --- a/src/native/clr/host/os-bridge.cc +++ b/src/native/clr/host/os-bridge.cc @@ -49,6 +49,21 @@ auto OSBridge::lref_to_gref (JNIEnv *env, jobject lref) noexcept -> jobject return g; } +auto OSBridge::get_object_ref_type (JNIEnv *env, void *handle) noexcept -> char +{ + jobjectRefType value; + if (handle == nullptr) + return 'I'; + value = env->GetObjectRefType (reinterpret_cast (handle)); + switch (value) { + case JNIInvalidRefType: return 'I'; + case JNILocalRefType: return 'L'; + case JNIGlobalRefType: return 'G'; + case JNIWeakGlobalRefType: return 'W'; + default: return '*'; + } +} + auto OSBridge::_monodroid_gref_inc () noexcept -> int { return __sync_add_and_fetch (&gc_gref_count, 1); diff --git a/src/native/clr/host/pinvoke-tables.include b/src/native/clr/host/pinvoke-tables.include index 000e4ce8afb..65afe1a26d5 100644 --- a/src/native/clr/host/pinvoke-tables.include +++ b/src/native/clr/host/pinvoke-tables.include @@ -11,7 +11,7 @@ namespace { #if INTPTR_MAX == INT64_MAX //64-bit internal p/invoke table - std::array internal_pinvokes {{ + std::array internal_pinvokes {{ {0xa50ce5de13bf8b5, "_monodroid_timezone_get_default_id", reinterpret_cast(&_monodroid_timezone_get_default_id)}, {0x3ade4348ac8ce0fa, "_monodroid_freeifaddrs", reinterpret_cast(&_monodroid_freeifaddrs)}, {0x3b2467e7eadd4a6a, "_monodroid_lref_log_new", reinterpret_cast(&_monodroid_lref_log_new)}, @@ -21,6 +21,7 @@ namespace { {0x5f0b4e426eff086b, "_monodroid_detect_cpu_and_architecture", reinterpret_cast(&_monodroid_detect_cpu_and_architecture)}, {0x9099a4b95e3c3a89, "_monodroid_lref_log_delete", reinterpret_cast(&_monodroid_lref_log_delete)}, {0x9187e6bc6294cacf, "clr_typemap_managed_to_java", reinterpret_cast(&clr_typemap_managed_to_java)}, + {0x920bf58357fb56f3, "clr_initialize_gc_bridge", reinterpret_cast(&clr_initialize_gc_bridge)}, {0x9a946dfe9916a942, "clr_typemap_java_to_managed", reinterpret_cast(&clr_typemap_java_to_managed)}, {0xa6ec846592d99536, "_monodroid_weak_gref_delete", reinterpret_cast(&_monodroid_weak_gref_delete)}, {0xa7f58f3ee428cc6b, "_monodroid_gref_log_delete", reinterpret_cast(&_monodroid_gref_log_delete)}, @@ -545,7 +546,7 @@ constexpr hash_t system_security_cryptography_native_android_library_hash = 0x18 constexpr hash_t system_globalization_native_library_hash = 0x28b5c8fca080abd5; #else //32-bit internal p/invoke table - std::array internal_pinvokes {{ + std::array internal_pinvokes {{ {0xb7a486a, "monodroid_TypeManager_get_java_class_name", reinterpret_cast(&monodroid_TypeManager_get_java_class_name)}, {0x2aea7c33, "_monodroid_max_gref_get", reinterpret_cast(&_monodroid_max_gref_get)}, {0x3227d81a, "monodroid_timing_start", reinterpret_cast(&monodroid_timing_start)}, @@ -558,6 +559,7 @@ constexpr hash_t system_globalization_native_library_hash = 0x28b5c8fca080abd5; {0x9a734f16, "_monodroid_weak_gref_get", reinterpret_cast(&_monodroid_weak_gref_get)}, {0x9c5b24a8, "_monodroid_weak_gref_new", reinterpret_cast(&_monodroid_weak_gref_new)}, {0xa04e5d1c, "monodroid_free", reinterpret_cast(&monodroid_free)}, + {0xa3c1e548, "clr_initialize_gc_bridge", reinterpret_cast(&clr_initialize_gc_bridge)}, {0xad511c82, "_monodroid_timezone_get_default_id", reinterpret_cast(&_monodroid_timezone_get_default_id)}, {0xb02468aa, "_monodroid_gref_get", reinterpret_cast(&_monodroid_gref_get)}, {0xb6431f9a, "clr_typemap_java_to_managed", reinterpret_cast(&clr_typemap_java_to_managed)}, @@ -1079,6 +1081,6 @@ constexpr hash_t system_security_cryptography_native_android_library_hash = 0x93 constexpr hash_t system_globalization_native_library_hash = 0xa66f1e5a; #endif -constexpr size_t internal_pinvokes_count = 26; +constexpr size_t internal_pinvokes_count = 27; constexpr size_t dotnet_pinvokes_count = 491; } // end of anonymous namespace diff --git a/src/native/clr/include/host/gc-bridge.hh b/src/native/clr/include/host/gc-bridge.hh new file mode 100644 index 00000000000..2960d41cd36 --- /dev/null +++ b/src/native/clr/include/host/gc-bridge.hh @@ -0,0 +1,106 @@ +#pragma once + +#include +#include +#include +#include + +#include + +struct JniObjectReferenceControlBlock +{ + jobject handle; + int handle_type; + jobject weak_handle; + int refs_added; +}; + +struct HandleContext +{ + intptr_t gc_handle; + JniObjectReferenceControlBlock* control_block; +}; + +struct StronglyConnectedComponent +{ + size_t Count; + HandleContext** Contexts; +}; + +struct ComponentCrossReference +{ + size_t SourceGroupIndex; + size_t DestinationGroupIndex; +}; + +struct MarkCrossReferencesArgs +{ + size_t ComponentCount; + StronglyConnectedComponent* Components; + size_t CrossReferenceCount; + ComponentCrossReference* CrossReferences; +}; + +using BridgeProcessingStartedFtn = void (*)(); +using BridgeProcessingFinishedFtn = void (*)(MarkCrossReferencesArgs*); +using BridgeProcessingFtn = void (*)(MarkCrossReferencesArgs*); + +namespace xamarin::android { + class GCBridge + { + public: + static void wait_for_bridge_processing () noexcept; + static void initialize_on_load (JNIEnv *env) noexcept; + static BridgeProcessingFtn initialize_callback ( + BridgeProcessingStartedFtn bridge_processing_started, + BridgeProcessingFinishedFtn bridge_processing_finished) noexcept + { + abort_if_invalid_pointer_argument (bridge_processing_started, "bridge_processing_started"); + abort_if_invalid_pointer_argument (bridge_processing_finished, "bridge_processing_finished"); + abort_unless (GCBridge::bridge_processing_started_callback == nullptr, "GC bridge processing started callback is already set"); + abort_unless (GCBridge::bridge_processing_finished_callback == nullptr, "GC bridge processing finished callback is already set"); + + GCBridge::bridge_processing_started_callback = bridge_processing_started; + GCBridge::bridge_processing_finished_callback = bridge_processing_finished; + + bridge_processing_thread = new std::thread(GCBridge::bridge_processing); + bridge_processing_thread->detach (); + + return mark_cross_references; + } + + private: + static inline BridgeProcessingStartedFtn bridge_processing_started_callback = nullptr; + static inline BridgeProcessingFinishedFtn bridge_processing_finished_callback = nullptr; + + static inline MarkCrossReferencesArgs shared_cross_refs; + static inline std::binary_semaphore bridge_processing_semaphore{0}; + + static inline JNIEnv* env = nullptr; + static inline std::shared_mutex processing_mutex; + static inline std::thread* bridge_processing_thread = nullptr; + + static void bridge_processing () noexcept; + static void mark_cross_references (MarkCrossReferencesArgs *cross_refs) noexcept; + + static void add_references (StronglyConnectedComponent *scc) noexcept; + static bool add_reference (jobject from, jobject to) noexcept; + static void clear_references (jobject handle) noexcept; + + static void prepare_for_java_collection (MarkCrossReferencesArgs *cross_refs) noexcept; + static void cleanup_after_java_collection (MarkCrossReferencesArgs *cross_refs) noexcept; + + static void take_weak_global_ref (HandleContext *context) noexcept; + static void take_global_ref (HandleContext *context) noexcept; + + static void trigger_java_gc () noexcept; + + static inline jobject Runtime_instance = nullptr; + static inline jmethodID Runtime_gc = nullptr; + + static inline jclass GCUserPeer_class = nullptr; + static inline jmethodID GCUserPeer_ctor = nullptr; + + static void log_mark_cross_references_args_if_enabled (MarkCrossReferencesArgs *cross_refs) noexcept; + }; +} diff --git a/src/native/clr/include/host/os-bridge.hh b/src/native/clr/include/host/os-bridge.hh index be3291f4866..b9da49b07f9 100644 --- a/src/native/clr/include/host/os-bridge.hh +++ b/src/native/clr/include/host/os-bridge.hh @@ -13,6 +13,7 @@ namespace xamarin::android { static void initialize_on_onload (JavaVM *vm, JNIEnv *env) noexcept; static void initialize_on_runtime_init (JNIEnv *env, jclass runtimeClass) noexcept; static auto lref_to_gref (JNIEnv *env, jobject lref) noexcept -> jobject; + static auto get_object_ref_type (JNIEnv *env, void *handle) noexcept -> char; static auto get_gc_gref_count () noexcept -> int { @@ -38,8 +39,11 @@ namespace xamarin::android { JNIEnv *env = nullptr; jvm->GetEnv ((void**)&env, JNI_VERSION_1_6); if (env == nullptr) { - // TODO: attach to the runtime thread here - jvm->GetEnv ((void**)&env, JNI_VERSION_1_6); + JavaVMAttachArgs args; + args.version = JNI_VERSION_1_6; + args.name = nullptr; + args.group = nullptr; + jvm->AttachCurrentThread (&env, &args); abort_unless (env != nullptr, "Unable to get a valid pointer to JNIEnv"); } diff --git a/src/native/clr/include/runtime-base/internal-pinvokes.hh b/src/native/clr/include/runtime-base/internal-pinvokes.hh index a3c69c6c6e7..0bc3f2d841f 100644 --- a/src/native/clr/include/runtime-base/internal-pinvokes.hh +++ b/src/native/clr/include/runtime-base/internal-pinvokes.hh @@ -3,6 +3,7 @@ #include #include +#include #include #include "logger.hh" #include @@ -14,6 +15,9 @@ extern "C" { void _monodroid_gref_log_delete (jobject handle, char type, const char *threadName, int threadId, const char *from, int from_writable) noexcept; const char* clr_typemap_managed_to_java (const char *typeName, const uint8_t *mvid) noexcept; bool clr_typemap_java_to_managed (const char *java_type_name, char const** assembly_name, uint32_t *managed_type_token_id) noexcept; + BridgeProcessingFtn clr_initialize_gc_bridge ( + BridgeProcessingStartedFtn bridge_processing_started_callback, + BridgeProcessingFinishedFtn mark_cross_references_callback) noexcept; void monodroid_log (xamarin::android::LogLevel level, LogCategories category, const char *message) noexcept; char* monodroid_TypeManager_get_java_class_name (jclass klass) noexcept; void monodroid_free (void *ptr) noexcept; diff --git a/src/native/clr/include/runtime-base/util.hh b/src/native/clr/include/runtime-base/util.hh index a301b841148..553b7aeab71 100644 --- a/src/native/clr/include/runtime-base/util.hh +++ b/src/native/clr/include/runtime-base/util.hh @@ -59,6 +59,8 @@ namespace xamarin::android { static auto monodroid_fopen (std::string_view const& filename, std::string_view const& mode) noexcept -> FILE*; static void set_world_accessable (std::string_view const& path); static auto set_world_accessible (int fd) noexcept -> bool; + static auto monodroid_strdup_printf (std::string_view const& format, ...) -> char*; + static auto monodroid_strdup_vprintf (std::string_view const& format, va_list vargs) -> char*; // Puts higher half of the `value` byte as a hexadecimal character in `high_half` and // the lower half in `low_half` diff --git a/src/native/clr/runtime-base/util.cc b/src/native/clr/runtime-base/util.cc index 85c2b339ad4..785a07f4a33 100644 --- a/src/native/clr/runtime-base/util.cc +++ b/src/native/clr/runtime-base/util.cc @@ -105,3 +105,24 @@ auto Util::set_world_accessible (int fd) noexcept -> bool return true; } + +char * +Util::monodroid_strdup_printf (std::string_view const& format, ...) +{ + va_list args; + + va_start (args, format); + char *ret = monodroid_strdup_vprintf (format, args); + va_end (args); + + return ret; +} + +char* +Util::monodroid_strdup_vprintf (std::string_view const& format, va_list vargs) +{ + char *ret = nullptr; + int n = vasprintf (&ret, format.data (), vargs); + + return n == -1 ? nullptr : ret; +} From 4e7c284d59d234eba337e4762b23db708603ad6b Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 16 Jun 2025 14:43:37 +0200 Subject: [PATCH 02/32] Address review --- .../ManagedValueManager.cs | 53 +++- src/native/clr/host/gc-bridge.cc | 280 +++++++++++------- src/native/clr/include/host/gc-bridge.hh | 49 +-- 3 files changed, 241 insertions(+), 141 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index 112c47e9d1a..75cae624385 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -309,30 +309,53 @@ internal static void BridgeProcessingStarted () } [UnmanagedCallersOnly] - internal static unsafe void BridgeProcessingFinished (MarkCrossReferencesArgs* mcr) + internal static unsafe void BridgeProcessingFinished(MarkCrossReferencesArgs* mcr) { - List handlesToFree = []; - ManagedValueManager instance = GetOrCreateInstance (); + Trace.Assert (mcr != null, "Bridge processing was not started before finishing it."); + + ManagedValueManager instance = GetOrCreateInstance(); + IEnumerable collectedContexts = GetCollectedContexts (mcr); + List handlesToFree; lock (instance.RegisteredInstances) { - for (int i = 0; (nuint)i < mcr->ComponentCount; i++) { - for (int j = 0; (nuint)j < mcr->Components [i].Count; j++) { - var context = (HandleContext*) mcr->Components [i].Contexts [j]; - if (context->ControlBlock == IntPtr.Zero) { - var handle = GCHandle.FromIntPtr (context->Handle); + handlesToFree = ProcessCollectedContexts (instance, collectedContexts); + } + + JavaMarshal.FinishCrossReferenceProcessing (mcr, CollectionsMarshal.AsSpan (handlesToFree)); + AndroidRuntimeInternal.BridgeProcessing = false; + } - // Clean up the RegisteredInstances dictionary + free the control block native memory associated with the handle - instance.RemoveRegisteredInstance ((IJavaPeerable)handle.Target, out WeakPeerReference? removedPeer); - removedPeer?.FreeContext (); + static unsafe List GetCollectedContexts (MarkCrossReferencesArgs* mcr) + { + List contexts = []; - handlesToFree.Add (handle); - } + for (int i = 0; (nuint)i < mcr->ComponentCount; i++) { + for (int j = 0; (nuint)j < mcr->Components [i].Count; j++) { + var context = (HandleContext*)mcr->Components [i].Contexts [j]; + if (context->ControlBlock == IntPtr.Zero) { + contexts.Add ((IntPtr)context); } } } - JavaMarshal.FinishCrossReferenceProcessing (mcr, CollectionsMarshal.AsSpan (handlesToFree)); - AndroidRuntimeInternal.BridgeProcessing = false; + return contexts; + } + + static unsafe List ProcessCollectedContexts (ManagedValueManager instance, IEnumerable collectedContexts) + { + List handlesToFree = []; + + foreach (HandleContext* context in collectedContexts) { + var handle = GCHandle.FromIntPtr (context->Handle); + + // Clean up the RegisteredInstances dictionary + free the control block native memory associated with the handle + instance.RemoveRegisteredInstance ((IJavaPeerable)handle.Target, out WeakPeerReference? removedPeer); + removedPeer?.FreeContext (); + + handlesToFree.Add (handle); + } + + return handlesToFree; } const BindingFlags ActivationConstructorBindingFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; diff --git a/src/native/clr/host/gc-bridge.cc b/src/native/clr/host/gc-bridge.cc index 714e52a03e5..ec65d891fa1 100644 --- a/src/native/clr/host/gc-bridge.cc +++ b/src/native/clr/host/gc-bridge.cc @@ -4,7 +4,6 @@ #include #include #include -#include using namespace xamarin::android; @@ -30,44 +29,39 @@ void GCBridge::initialize_on_load (JNIEnv *jniEnv) noexcept void GCBridge::trigger_java_gc () noexcept { env->CallVoidMethod (Runtime_instance, Runtime_gc); - if (env->ExceptionCheck ()) [[unlikely]] { - env->ExceptionDescribe (); - env->ExceptionClear (); - log_error (LOG_DEFAULT, "Java GC failed"); + + if (!env->ExceptionCheck ()) [[likely]] { + return; } + + env->ExceptionDescribe (); + env->ExceptionClear (); + log_error (LOG_DEFAULT, "Java GC failed"); } -void GCBridge::prepare_for_java_collection (MarkCrossReferencesArgs* cross_refs) noexcept +void GCBridge::prepare_for_java_collection () noexcept { // Some SCCs might have no IGCUserPeers associated with them, so we must create one - std::unordered_map temporary_peers; + std::unordered_map temporary_peers; // Before looking at xrefs, scan the SCCs. During collection, an SCC has to behave like a // single object. If the number of objects in the SCC is anything other than 1, the SCC // must be doctored to mimic that one-object nature. - for (size_t i = 0; i < cross_refs->ComponentCount; i++) { - StronglyConnectedComponent *scc = &cross_refs->Components [i]; + for (size_t i = 0; i < cross_refs.ComponentCount; i++) { + const StronglyConnectedComponent *scc = &cross_refs.Components [i]; // Count > 1 case: The SCC contains many objects which must be collected as one. // Solution: Make all objects within the SCC directly or indirectly reference each other if (scc->Count > 1) { - add_references (scc); + add_references (*scc); } else if (scc->Count == 0) { - temporary_peers [scc] = env->NewObject (GCUserPeer_class, GCUserPeer_ctor); + temporary_peers [i] = env->NewObject (GCUserPeer_class, GCUserPeer_ctor); } } // Add the cross scc refs - for (size_t i = 0; i < cross_refs->CrossReferenceCount; i++) { - ComponentCrossReference *xref = &cross_refs->CrossReferences [i]; - - StronglyConnectedComponent *source = &cross_refs->Components [xref->SourceGroupIndex]; - jobject from = source->Count > 0 ? source->Contexts [0]->control_block->handle : temporary_peers [source]; - - StronglyConnectedComponent *destination = &cross_refs->Components [xref->DestinationGroupIndex]; - jobject to = destination->Count > 0 ? destination->Contexts [0]->control_block->handle : temporary_peers [destination]; - - add_reference (from, to); + for (size_t i = 0; i < cross_refs.CrossReferenceCount; i++) { + add_cross_reference (i, temporary_peers); } // With cross references processed, the temporary peer list can be released @@ -76,38 +70,68 @@ void GCBridge::prepare_for_java_collection (MarkCrossReferencesArgs* cross_refs) } // Switch global to weak references - for (size_t i = 0; i < cross_refs->ComponentCount; i++) { - StronglyConnectedComponent *scc = &cross_refs->Components [i]; - for (size_t j = 0; j < scc->Count; j++) { - take_weak_global_ref (scc->Contexts [j]); + for (size_t i = 0; i < cross_refs.ComponentCount; i++) { + const StronglyConnectedComponent &scc = cross_refs.Components [i]; + for (size_t j = 0; j < scc.Count; j++) { + take_weak_global_ref (scc.Contexts [j]); } } } -void GCBridge::add_references (StronglyConnectedComponent *scc) noexcept +jobject GCBridge::pick_representative (size_t scc_index, const StronglyConnectedComponent &scc, const std::unordered_map &temporary_peers) noexcept +{ + if (scc.Count > 0) { + abort_unless (scc.Contexts [0] != nullptr, "SCC must have at least one context"); + abort_unless (scc.Contexts [0]->control_block != nullptr, "SCC must have at least one context with valid control block"); + return scc.Contexts [0]->control_block->handle; + } else { + const auto temporary_peer = temporary_peers.find (scc_index); + abort_unless (temporary_peer != temporary_peers.end(), "Temporary peer must be found in the map"); + return temporary_peer->second; + } +} + +void GCBridge::add_references (const StronglyConnectedComponent &scc) noexcept { - abort_if_invalid_pointer_argument (scc, "scc"); - abort_unless (scc->Count > 1, "SCC must have at least two items to add inner references"); + abort_unless (scc.Count > 1, "SCC must have at least two items to add inner references"); - JniObjectReferenceControlBlock *prev = scc->Contexts [scc->Count - 1]->control_block; + JniObjectReferenceControlBlock *prev = scc.Contexts [scc.Count - 1]->control_block; - for (size_t j = 1; j < scc->Count; j++) { - JniObjectReferenceControlBlock *current = scc->Contexts [j]->control_block; + for (size_t j = 1; j < scc.Count; j++) { + JniObjectReferenceControlBlock *current = scc.Contexts [j]->control_block; if (add_reference (prev->handle, current->handle)) { - prev->refs_added++; + prev->refs_added = 1; + } else { + // TODO } prev = current; } } +void GCBridge::add_cross_reference (size_t xref_index, const std::unordered_map &temporary_peers) noexcept +{ + const ComponentCrossReference &xref = cross_refs.CrossReferences [xref_index]; + + const StronglyConnectedComponent &source = cross_refs.Components [xref.SourceGroupIndex]; + jobject from = pick_representative (xref.SourceGroupIndex, source, temporary_peers); + + const StronglyConnectedComponent &dest = cross_refs.Components [xref.DestinationGroupIndex]; + jobject to = pick_representative (xref.DestinationGroupIndex, dest, temporary_peers); + + if (add_reference (from, to) && source.Count > 0) { + // If the source is a SCC with at least one item, we need to mark the first item it as having added a reference + source.Contexts [0]->control_block->refs_added = 1; + } +} + bool GCBridge::add_reference (jobject from, jobject to) noexcept { jclass java_class = env->GetObjectClass (from); jmethodID add_method_id = env->GetMethodID (java_class, "monodroidAddReference", "(Ljava/lang/Object;)V"); env->DeleteLocalRef (java_class); - if (add_method_id) { + if (add_method_id != nullptr) { env->CallVoidMethod (from, add_method_id, to); return true; } else { @@ -123,7 +147,7 @@ void GCBridge::clear_references (jobject handle) noexcept jmethodID clear_method_id = env->GetMethodID (java_class, "monodroidClearReferences", "()V"); env->DeleteLocalRef (java_class); // Clean up the local reference to the class - if (clear_method_id) { + if (clear_method_id != nullptr) { env->CallVoidMethod (handle, clear_method_id); } else { log_error (LOG_DEFAULT, "Failed to find monodroidClearReferences method"); @@ -140,16 +164,24 @@ void GCBridge::clear_references (jobject handle) noexcept void GCBridge::take_global_ref (HandleContext *context) noexcept { + abort_if_invalid_pointer_argument (context, "context"); + abort_unless (context->control_block != nullptr, "Control block must not be null"); + if (context->control_block->handle_type != JNIWeakGlobalRefType) [[unlikely]] { + log_error (LOG_DEFAULT, "Expected weak global reference type for handle, but got {} - handle: {:#x}", context->control_block->handle_type, reinterpret_cast (context->control_block->handle)); + return; + } abort_unless (context->control_block->handle_type == JNIWeakGlobalRefType, "Expected weak global reference type for handle"); jobject weak = context->control_block->handle; jobject handle = env->NewGlobalRef (weak); - if (Logger::gref_log ()) [[unlikely]] { - char *message = Util::monodroid_strdup_printf ("take_global_ref gchandle=%p -> wref=%p handle=%p\n"sv, context->gc_handle, weak, handle); - OSBridge::_monodroid_gref_log (message); - free (message); - } + // if (Logger::gref_log ()) [[unlikely]] { + // OSBridge::_monodroid_gref_log ( + // std::format ("take_global_ref gchandle={:#x} -> wref={:#x} handle={:#x}\n"sv, + // reinterpret_cast (context->gc_handle), + // reinterpret_cast (weak), + // reinterpret_cast (handle)).data ()); + // } if (handle != nullptr) { context->control_block->handle = handle; @@ -167,21 +199,25 @@ void GCBridge::take_global_ref (HandleContext *context) noexcept // The native memory of the control block will be freed in managed code as well as the weak global ref context->control_block = nullptr; - if (Logger::gc_spew_enabled ()) [[unlikely]] { - char *message = Util::monodroid_strdup_printf ("handle %p/W; was collected by a Java GC"sv, weak); - OSBridge::_monodroid_gref_log (message); - free (message); - } + // if (Logger::gc_spew_enabled ()) [[unlikely]] { + // OSBridge::_monodroid_gref_log ( + // std::format ("handle {:#x}/W; was collected by a Java GC"sv, reinterpret_cast (weak)).data ()); + // } } } void GCBridge::take_weak_global_ref (HandleContext *context) noexcept { + abort_if_invalid_pointer_argument (context, "context"); + abort_unless (context->control_block != nullptr, "Control block must not be null"); + abort_unless (context->control_block->handle_type == JNIGlobalRefType, "Expected global reference type for handle"); + jobject handle = context->control_block->handle; if (Logger::gref_log ()) [[unlikely]] { - char *message = Util::monodroid_strdup_printf ("take_weak_global_ref gchandle=%p; handle=%p\n"sv, context->gc_handle, handle); - OSBridge::_monodroid_gref_log (message); - free (message); + OSBridge::_monodroid_gref_log ( + std::format ("take_weak_global_ref gchandle={:#x}; handle={:#x}\n"sv, + reinterpret_cast (context->gc_handle), + reinterpret_cast (handle)).data ()); } jobject weak = env->NewWeakGlobalRef (handle); @@ -192,12 +228,18 @@ void GCBridge::take_weak_global_ref (HandleContext *context) noexcept context->control_block->handle = weak; context->control_block->handle_type = JNIWeakGlobalRefType; + log_error_fmt (LOG_GC, "take_weak_global_ref: gchandle={:#x} -> wref={:#x} handle={:#x} -> new type {}", + reinterpret_cast (context->gc_handle), + reinterpret_cast (weak), + reinterpret_cast (handle), + context->control_block->handle_type); + OSBridge::_monodroid_gref_log_delete (handle, OSBridge::get_object_ref_type (env, handle), "finalizer", gettid (), " at [[clr-gc:take_weak_global_ref]]", 0); env->DeleteGlobalRef (handle); } -void GCBridge::cleanup_after_java_collection (MarkCrossReferencesArgs* cross_refs) noexcept +void GCBridge::cleanup_after_java_collection () noexcept { #if DEBUG int total = 0; @@ -205,41 +247,24 @@ void GCBridge::cleanup_after_java_collection (MarkCrossReferencesArgs* cross_ref #endif // try to switch back to global refs to analyze what stayed alive - for (size_t i = 0; i < cross_refs->ComponentCount; i++) { - StronglyConnectedComponent *scc = &cross_refs->Components [i]; - for (size_t j = 0; j < scc->Count; j++) { -#if DEBUG - total++; -#endif - take_global_ref (scc->Contexts [j]); - } + for (size_t i = 0; i < cross_refs.ComponentCount; i++) { + const StronglyConnectedComponent &scc = cross_refs.Components [i]; + for (size_t j = 0; j < scc.Count; j++) { + take_global_ref (scc.Contexts [j]); + } } // clear the cross references on any remaining items - for (size_t i = 0; i < cross_refs->ComponentCount; i++) { - StronglyConnectedComponent *scc = &cross_refs->Components [i]; - bool is_alive = false; - - for (size_t j = 0; j < scc->Count; j++) { - JniObjectReferenceControlBlock *control_block = scc->Contexts [j]->control_block; + for (size_t i = 0; i < cross_refs.ComponentCount; i++) { + const StronglyConnectedComponent &scc = cross_refs.Components [i]; + [[maybe_unused]] bool is_alive = cleanup_strongly_connected_component (i, scc); - if (control_block != nullptr) { #if DEBUG - alive++; -#endif - if (j > 0) { - abort_unless (is_alive, [&i] { return detail::_format_message ("Bridge SCC at index %d must be alive", i); }); - } - - is_alive = true; - if (control_block->refs_added > 0) { - clear_references (control_block->handle); - control_block->refs_added = 0; - } - } else { - abort_unless (!is_alive, [&i] { return detail::_format_message ("Bridge SCC at index %d must NOT be alive", i); }); - } + total += scc.Count; + if (is_alive) { + alive += scc.Count; } +#endif } #if DEBUG @@ -247,6 +272,33 @@ void GCBridge::cleanup_after_java_collection (MarkCrossReferencesArgs* cross_ref #endif } +bool GCBridge::cleanup_strongly_connected_component (size_t i, const StronglyConnectedComponent &scc) noexcept +{ + // all contexts in the SCC must either be alive, or collected + bool is_alive = false; + + for (size_t j = 0; j < scc.Count; j++) { + abort_unless (scc.Contexts [j] != nullptr, "Context must not be null"); + JniObjectReferenceControlBlock *control_block = scc.Contexts [j]->control_block; + + if (control_block != nullptr) { + if (j > 0) { + abort_unless (is_alive, [&i] { return detail::_format_message ("Bridge SCC at index %d must be alive", i); }); + } + + is_alive = true; + if (control_block->refs_added > 0) { + clear_references (control_block->handle); + control_block->refs_added = 0; + } + } else { + abort_unless (!is_alive, [&i] { return detail::_format_message ("Bridge SCC at index %d must NOT be alive", i); }); + } + } + + return is_alive; +} + void GCBridge::bridge_processing () noexcept { abort_unless (bridge_processing_started_callback != nullptr, "GC bridge processing started callback is not set"); @@ -258,25 +310,32 @@ void GCBridge::bridge_processing () noexcept bridge_processing_semaphore.acquire (); std::unique_lock lock (processing_mutex); - MarkCrossReferencesArgs* args = &GCBridge::shared_cross_refs; - log_mark_cross_references_args_if_enabled (args); + log_mark_cross_references_args_if_enabled (); bridge_processing_started_callback (); - prepare_for_java_collection (args); + prepare_for_java_collection (); trigger_java_gc (); - cleanup_after_java_collection (args); - bridge_processing_finished_callback (args); + cleanup_after_java_collection (); + bridge_processing_finished_callback (&cross_refs); } } -void GCBridge::mark_cross_references (MarkCrossReferencesArgs* cross_refs) noexcept +void GCBridge::mark_cross_references (MarkCrossReferencesArgs *args) noexcept { + static int ctr = 0; + log_error (LOG_DEFAULT, "mark_cross_references called for the {} time", ++ctr); + + abort_if_invalid_pointer_argument (args, "cross_refs"); + abort_unless (args->Components != nullptr || args->ComponentCount == 0, "Components must not be null if ComponentCount is greater than 0"); + abort_unless (args->CrossReferences != nullptr || args->CrossReferenceCount == 0, "CrossReferences must not be null if CrossReferenceCount is greater than 0"); + std::unique_lock lock (processing_mutex); - GCBridge::shared_cross_refs.ComponentCount = cross_refs->ComponentCount; - GCBridge::shared_cross_refs.Components = cross_refs->Components; - GCBridge::shared_cross_refs.CrossReferenceCount = cross_refs->CrossReferenceCount; - GCBridge::shared_cross_refs.CrossReferences = cross_refs->CrossReferences; + log_error (LOG_DEFAULT, "setting cross_refs"); + cross_refs.ComponentCount = args->ComponentCount; + cross_refs.Components = args->Components; + cross_refs.CrossReferenceCount = args->CrossReferenceCount; + cross_refs.CrossReferences = args->CrossReferences; bridge_processing_semaphore.release (); } @@ -287,29 +346,40 @@ void GCBridge::wait_for_bridge_processing () noexcept } [[gnu::always_inline]] -void GCBridge::log_mark_cross_references_args_if_enabled (MarkCrossReferencesArgs* args) noexcept +void GCBridge::log_mark_cross_references_args_if_enabled () noexcept { - if (Logger::gc_spew_enabled ()) [[unlikely]] { - log_info (LOG_GC, "cross references callback invoked with {} sccs and {} xrefs.", args->ComponentCount, args->CrossReferenceCount); - - for (size_t i = 0; i < args->ComponentCount; ++i) { - log_info (LOG_GC, "group {} with {} objects", i, args->Components [i].Count); - for (size_t j = 0; j < args->Components [i].Count; ++j) { - HandleContext *ctx = args->Components [i].Contexts [j]; - jobject handle = ctx->control_block->handle; - jclass java_class = env->GetObjectClass (handle); + if (!Logger::gc_spew_enabled ()) [[likely]] { + return; + } + + log_info (LOG_GC, "cross references callback invoked with {} sccs and {} xrefs.", cross_refs.ComponentCount, cross_refs.CrossReferenceCount); + + for (size_t i = 0; i < cross_refs.ComponentCount; ++i) { + log_info (LOG_GC, "group {} with {} objects", i, cross_refs.Components [i].Count); + for (size_t j = 0; j < cross_refs.Components [i].Count; ++j) { + HandleContext *ctx = cross_refs.Components [i].Contexts [j]; + abort_unless (ctx != nullptr, "Context must not be null"); + abort_unless (ctx->control_block != nullptr, "Control block must not be null"); + + jobject handle = ctx->control_block->handle; + jclass java_class = env->GetObjectClass (handle); + if (java_class != nullptr) { char *class_name = Host::get_java_class_name_for_TypeManager (java_class); - log_info (LOG_GC, "\tgchandle {:p} gref {:p} [{}]", reinterpret_cast (ctx->gc_handle), reinterpret_cast (handle), class_name); + log_info (LOG_GC, "\tgchandle {:#x} gref {:#x} [{}]", reinterpret_cast (ctx->gc_handle), reinterpret_cast (handle), class_name); free (class_name); + } else { + log_info (LOG_GC, "\tgchandle {:#x} gref {:#x} [unknown class]", reinterpret_cast (ctx->gc_handle), reinterpret_cast (handle)); } } + } - if (Util::should_log (LOG_GC)) { - for (size_t i = 0; i < args->CrossReferenceCount; ++i) { - size_t source_index = args->CrossReferences [i].SourceGroupIndex; - size_t dest_index = args->CrossReferences [i].DestinationGroupIndex; - log_info_nocheck_fmt (LOG_GC, "xref [{}] {} -> {}", i, source_index, dest_index); - } - } + if (!Util::should_log (LOG_GC)) { + return; + } + + for (size_t i = 0; i < cross_refs.CrossReferenceCount; ++i) { + size_t source_index = cross_refs.CrossReferences [i].SourceGroupIndex; + size_t dest_index = cross_refs.CrossReferences [i].DestinationGroupIndex; + log_info_nocheck_fmt (LOG_GC, "xref [{}] {} -> {}", i, source_index, dest_index); } } diff --git a/src/native/clr/include/host/gc-bridge.hh b/src/native/clr/include/host/gc-bridge.hh index 2960d41cd36..8582352e4f9 100644 --- a/src/native/clr/include/host/gc-bridge.hh +++ b/src/native/clr/include/host/gc-bridge.hh @@ -4,6 +4,7 @@ #include #include #include +#include #include @@ -70,37 +71,43 @@ namespace xamarin::android { } private: - static inline BridgeProcessingStartedFtn bridge_processing_started_callback = nullptr; - static inline BridgeProcessingFinishedFtn bridge_processing_finished_callback = nullptr; - - static inline MarkCrossReferencesArgs shared_cross_refs; static inline std::binary_semaphore bridge_processing_semaphore{0}; - - static inline JNIEnv* env = nullptr; static inline std::shared_mutex processing_mutex; static inline std::thread* bridge_processing_thread = nullptr; + static inline JNIEnv* env = nullptr; + static inline jobject Runtime_instance = nullptr; + static inline jmethodID Runtime_gc = nullptr; + static inline jclass GCUserPeer_class = nullptr; + static inline jmethodID GCUserPeer_ctor = nullptr; + + static inline MarkCrossReferencesArgs cross_refs; + + static inline BridgeProcessingStartedFtn bridge_processing_started_callback = nullptr; + static inline BridgeProcessingFinishedFtn bridge_processing_finished_callback = nullptr; + static void bridge_processing () noexcept; static void mark_cross_references (MarkCrossReferencesArgs *cross_refs) noexcept; - static void add_references (StronglyConnectedComponent *scc) noexcept; - static bool add_reference (jobject from, jobject to) noexcept; - static void clear_references (jobject handle) noexcept; - - static void prepare_for_java_collection (MarkCrossReferencesArgs *cross_refs) noexcept; - static void cleanup_after_java_collection (MarkCrossReferencesArgs *cross_refs) noexcept; + static void prepare_for_java_collection () noexcept; + static void trigger_java_gc () noexcept; + static void cleanup_after_java_collection () noexcept; + static bool cleanup_strongly_connected_component (size_t i, const StronglyConnectedComponent &scc) noexcept; static void take_weak_global_ref (HandleContext *context) noexcept; static void take_global_ref (HandleContext *context) noexcept; - - static void trigger_java_gc () noexcept; - - static inline jobject Runtime_instance = nullptr; - static inline jmethodID Runtime_gc = nullptr; - static inline jclass GCUserPeer_class = nullptr; - static inline jmethodID GCUserPeer_ctor = nullptr; - - static void log_mark_cross_references_args_if_enabled (MarkCrossReferencesArgs *cross_refs) noexcept; + static bool add_reference (jobject from, jobject to) noexcept; + static void clear_references (jobject handle) noexcept; + static void add_references (const StronglyConnectedComponent &scc) noexcept; + static void add_cross_reference ( + size_t xref_index, + const std::unordered_map &temporary_peers) noexcept; + static jobject pick_representative ( + size_t scc_index, + const StronglyConnectedComponent &scc, + const std::unordered_map &temporary_peers) noexcept; + + static void log_mark_cross_references_args_if_enabled () noexcept; }; } From 1205f46116f4adb433393eafc06a95b014bac637 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 17 Jun 2025 10:21:59 +0200 Subject: [PATCH 03/32] Split GCBridge into two classes --- src/native/clr/host/CMakeLists.txt | 1 + src/native/clr/host/bridge-processing.cc | 318 ++++++++++++++++ src/native/clr/host/gc-bridge.cc | 342 +++--------------- src/native/clr/host/host.cc | 3 +- .../clr/include/host/bridge-processing.hh | 49 +++ src/native/clr/include/host/gc-bridge.hh | 34 +- 6 files changed, 420 insertions(+), 327 deletions(-) create mode 100644 src/native/clr/host/bridge-processing.cc create mode 100644 src/native/clr/include/host/bridge-processing.hh diff --git a/src/native/clr/host/CMakeLists.txt b/src/native/clr/host/CMakeLists.txt index 9ddaa13234f..b8d1785fe39 100644 --- a/src/native/clr/host/CMakeLists.txt +++ b/src/native/clr/host/CMakeLists.txt @@ -29,6 +29,7 @@ set(XAMARIN_NET_ANDROID_STATIC_LIB "${XAMARIN_NET_ANDROID_LIB}-static") set(XAMARIN_MONODROID_SOURCES assembly-store.cc + bridge-processing.cc gc-bridge.cc host.cc host-jni.cc diff --git a/src/native/clr/host/bridge-processing.cc b/src/native/clr/host/bridge-processing.cc new file mode 100644 index 00000000000..e4cf5edbe30 --- /dev/null +++ b/src/native/clr/host/bridge-processing.cc @@ -0,0 +1,318 @@ +#include +#include +#include +#include + +#include + +using namespace xamarin::android; + +void BridgeProcessing::initialize_on_runtime_init (JNIEnv *env, jclass runtimeClass) noexcept +{ + abort_if_invalid_pointer_argument (env, "env"); + abort_if_invalid_pointer_argument (runtimeClass, "runtimeClass"); + + GCUserPeer_class = RuntimeUtil::get_class_from_runtime_field (env, runtimeClass, "mono_android_GCUserPeer", true); + GCUserPeer_ctor = env->GetMethodID (GCUserPeer_class, "", "()V"); + + abort_unless (GCUserPeer_class != nullptr && GCUserPeer_ctor != nullptr, "Failed to load mono.android.GCUserPeer!"); +} + +BridgeProcessing::BridgeProcessing (MarkCrossReferencesArgs args) noexcept + : env (OSBridge::ensure_jnienv ()), + cross_refs (args) +{ +} + +void BridgeProcessing::process () noexcept +{ + prepare_for_java_collection (); + GCBridge::trigger_java_gc (env); + cleanup_after_java_collection (); +} + +void BridgeProcessing::prepare_for_java_collection () noexcept +{ + // Before looking at xrefs, scan the SCCs. During collection, an SCC has to behave like a + // single object. If the number of objects in the SCC is anything other than 1, the SCC + // must be doctored to mimic that one-object nature. + for (size_t i = 0; i < cross_refs.ComponentCount; i++) { + const StronglyConnectedComponent &scc = cross_refs.Components [i]; + + // Count > 1 case: The SCC contains many objects which must be collected as one. + // Solution: Make all objects within the SCC directly or indirectly reference each other + if (scc.Count > 1) { + add_circular_references (scc); + } else if (scc.Count == 0) { + // Some SCCs might have no IGCUserPeers associated with them, so we must create one + temporary_peers [i] = env->NewObject (GCUserPeer_class, GCUserPeer_ctor); + } + } + + // Add the cross scc refs + for (size_t i = 0; i < cross_refs.CrossReferenceCount; i++) { + const ComponentCrossReference &xref = cross_refs.CrossReferences [i]; + add_cross_reference (xref.SourceGroupIndex, xref.DestinationGroupIndex); + } + + // With cross references processed, the temporary peer list can be released + for (const auto& [scc, temporary_peer] : temporary_peers) { + env->DeleteLocalRef (temporary_peer); + } + + // Switch global to weak references + for (size_t i = 0; i < cross_refs.ComponentCount; i++) { + const StronglyConnectedComponent &scc = cross_refs.Components [i]; + for (size_t j = 0; j < scc.Count; j++) { + take_weak_global_ref (scc.Contexts [j]); + } + } +} + +CrossReferenceTarget BridgeProcessing::select_cross_reference_target (size_t scc_index) noexcept +{ + const StronglyConnectedComponent &scc = cross_refs.Components [scc_index]; + if (scc.Count > 0) { + abort_unless (scc.Contexts [0] != nullptr, "SCC must have at least one context"); + abort_unless (scc.Contexts [0]->control_block != nullptr, "SCC must have at least one context with valid control block"); + return { .is_temporary_peer = false, .context = scc.Contexts [0] }; + } else { + const auto temporary_peer = temporary_peers.find (scc_index); + abort_unless (temporary_peer != temporary_peers.end(), "Temporary peer must be found in the map"); + return { .is_temporary_peer = true, .temporary_peer = temporary_peer->second }; + } +} + +void BridgeProcessing::add_circular_references (const StronglyConnectedComponent &scc) noexcept +{ + abort_unless (scc.Count > 1, "SCC must have at least two items to add inner references"); + + JniObjectReferenceControlBlock *prev = scc.Contexts [scc.Count - 1]->control_block; + + for (size_t j = 1; j < scc.Count; j++) { + JniObjectReferenceControlBlock *next = scc.Contexts [j]->control_block; + bool reference_added = add_reference (prev->handle, next->handle); + + // TODO this doesn't seem to be handled in the Mono code but if we don't handle this case, then the component _might not be_ strongly connected on the Java side. + // What is the case when this fails though? All bridge objects should have the `monodroidAddReference` method, so this is unexpected. + abort_unless (reference_added, [this, &prev, &next] { + jclass prev_java_class = env->GetObjectClass (prev->handle); + const char *prev_class_name = Host::get_java_class_name_for_TypeManager (prev_java_class); + + jclass next_java_class = env->GetObjectClass (next->handle); + const char *next_class_name = Host::get_java_class_name_for_TypeManager (next_java_class); + + return detail::_format_message ( + "Failed to add reference between objects in a strongly connected component: %s -> %s.", + prev_class_name, + next_class_name); + }); + + prev->refs_added = 1; + prev = next; + } +} + +void BridgeProcessing::add_cross_reference (size_t source_index, size_t dest_index) noexcept +{ + CrossReferenceTarget from = select_cross_reference_target (source_index); + CrossReferenceTarget to = select_cross_reference_target (dest_index); + + if (add_reference (from.get_handle(), to.get_handle())) { + from.mark_refs_added_if_needed (); + } +} + +bool BridgeProcessing::add_reference (jobject from, jobject to) noexcept +{ + jclass java_class = env->GetObjectClass (from); + jmethodID add_method_id = env->GetMethodID (java_class, "monodroidAddReference", "(Ljava/lang/Object;)V"); + env->DeleteLocalRef (java_class); + + if (add_method_id != nullptr) { + env->CallVoidMethod (from, add_method_id, to); + return true; + } else { + env->ExceptionClear (); + return false; + } +} + +void BridgeProcessing::clear_references_if_needed (JniObjectReferenceControlBlock *control_block) noexcept +{ + abort_if_invalid_pointer_argument (control_block, "control_block"); + abort_unless (control_block->handle != nullptr, "Control block handle must not be null"); + abort_unless (control_block->handle_type == JNIGlobalRefType, "Control block handle type must be global reference"); + + if (control_block->refs_added == 0) { + return; + } + + // Clear references from the object + jclass java_class = env->GetObjectClass (control_block->handle); + jmethodID clear_method_id = env->GetMethodID (java_class, "monodroidClearReferences", "()V"); + env->DeleteLocalRef (java_class); // Clean up the local reference to the class + + if (clear_method_id != nullptr) [[likely]] { + env->CallVoidMethod (control_block->handle, clear_method_id); + control_block->refs_added = 0; + } else { + log_error (LOG_DEFAULT, "Failed to find monodroidClearReferences method"); + env->ExceptionClear (); +#if DEBUG + if (Logger::gc_spew_enabled ()) { + char *class_name = Host::get_java_class_name_for_TypeManager (java_class); + log_error (LOG_GC, "Missing monodroidClearReferences method for object of class {}", class_name); + free (class_name); + } +#endif + } +} + +void BridgeProcessing::take_global_ref (HandleContext *context) noexcept +{ + abort_if_invalid_pointer_argument (context, "context"); + abort_unless (context->control_block != nullptr, "Control block must not be null"); + if (context->control_block->handle_type != JNIWeakGlobalRefType) [[unlikely]] { + log_error (LOG_DEFAULT, "Expected weak global reference type for handle, but got {} - handle: {:#x}", context->control_block->handle_type, reinterpret_cast (context->control_block->handle)); + return; + } + abort_unless (context->control_block->handle_type == JNIWeakGlobalRefType, "Expected weak global reference type for handle"); + + jobject weak = context->control_block->handle; + jobject handle = env->NewGlobalRef (weak); + + // if (Logger::gref_log ()) [[unlikely]] { + // OSBridge::_monodroid_gref_log ( + // std::format ("take_global_ref gchandle={:#x} -> wref={:#x} handle={:#x}\n"sv, + // reinterpret_cast (context->gc_handle), + // reinterpret_cast (weak), + // reinterpret_cast (handle)).data ()); + // } + + if (handle != nullptr) { + context->control_block->handle = handle; + context->control_block->handle_type = JNIGlobalRefType; + + OSBridge::_monodroid_gref_log_new (weak, OSBridge::get_object_ref_type (env, weak), + handle, OSBridge::get_object_ref_type (env, handle), + "finalizer", gettid (), + " at [[clr-gc:take_global_ref]]", 0); + + OSBridge::_monodroid_weak_gref_delete (weak, OSBridge::get_object_ref_type (env, weak), + "finalizer", gettid (), " at [[clr-gc:take_global_ref]]", 0); + env->DeleteWeakGlobalRef (weak); + } else { + // The native memory of the control block will be freed in managed code as well as the weak global ref + context->control_block = nullptr; + + // if (Logger::gc_spew_enabled ()) [[unlikely]] { + // OSBridge::_monodroid_gref_log ( + // std::format ("handle {:#x}/W; was collected by a Java GC"sv, reinterpret_cast (weak)).data ()); + // } + } +} + +void BridgeProcessing::take_weak_global_ref (HandleContext *context) noexcept +{ + abort_if_invalid_pointer_argument (context, "context"); + abort_unless (context->control_block != nullptr, "Control block must not be null"); + abort_unless (context->control_block->handle_type == JNIGlobalRefType, "Expected global reference type for handle"); + + jobject handle = context->control_block->handle; + if (Logger::gref_log ()) [[unlikely]] { + OSBridge::_monodroid_gref_log ( + std::format ("take_weak_global_ref gchandle={:#x}; handle={:#x}\n"sv, + reinterpret_cast (context->gc_handle), + reinterpret_cast (handle)).data ()); + } + + jobject weak = env->NewWeakGlobalRef (handle); + OSBridge::_monodroid_weak_gref_new (handle, OSBridge::get_object_ref_type (env, handle), + weak, OSBridge::get_object_ref_type (env, weak), + "finalizer", gettid (), " at [[clr-gc:take_weak_global_ref]]", 0); + + context->control_block->handle = weak; + context->control_block->handle_type = JNIWeakGlobalRefType; + + log_error_fmt (LOG_GC, "take_weak_global_ref: gchandle={:#x} -> wref={:#x} handle={:#x} -> new type {}", + reinterpret_cast (context->gc_handle), + reinterpret_cast (weak), + reinterpret_cast (handle), + context->control_block->handle_type); + + OSBridge::_monodroid_gref_log_delete (handle, OSBridge::get_object_ref_type (env, handle), + "finalizer", gettid (), " at [[clr-gc:take_weak_global_ref]]", 0); + env->DeleteGlobalRef (handle); +} + +void BridgeProcessing::cleanup_after_java_collection () noexcept +{ +#if DEBUG + int total = 0; + int alive = 0; +#endif + + // try to switch back to global refs to analyze what stayed alive + for (size_t i = 0; i < cross_refs.ComponentCount; i++) { + const StronglyConnectedComponent &scc = cross_refs.Components [i]; + for (size_t j = 0; j < scc.Count; j++) { + take_global_ref (scc.Contexts [j]); + } + } + + // clear the cross references on any remaining items + for (size_t i = 0; i < cross_refs.ComponentCount; i++) { + const StronglyConnectedComponent &scc = cross_refs.Components [i]; + [[maybe_unused]] bool is_alive = cleanup_strongly_connected_component (i, scc); + +#if DEBUG + total += scc.Count; + if (is_alive) { + alive += scc.Count; + } +#endif + } + +#if DEBUG + log_info (LOG_GC, "GC cleanup summary: {} objects tested - resurrecting {}.", total, alive); +#endif +} + +bool BridgeProcessing::cleanup_strongly_connected_component (size_t i, const StronglyConnectedComponent &scc) noexcept +{ + // all contexts in the SCC must either be alive, or collected + bool is_alive = false; + + for (size_t j = 0; j < scc.Count; j++) { + abort_unless (scc.Contexts [j] != nullptr, "Context must not be null"); + JniObjectReferenceControlBlock *control_block = scc.Contexts [j]->control_block; + + if (control_block != nullptr) { + if (j > 0) { + abort_unless (is_alive, [&i] { return detail::_format_message ("Bridge SCC at index %d must be alive", i); }); + } + + is_alive = true; + clear_references_if_needed (control_block); + } else { + abort_unless (!is_alive, [&i] { return detail::_format_message ("Bridge SCC at index %d must NOT be alive", i); }); + } + } + + return is_alive; +} + +jobject CrossReferenceTarget::get_handle () const noexcept +{ + return is_temporary_peer ? temporary_peer : context->control_block->handle; +} + +void CrossReferenceTarget::mark_refs_added_if_needed () noexcept +{ + if (!is_temporary_peer) { + abort_unless (context != nullptr, "Context must not be null"); + abort_unless (context->control_block != nullptr, "Control block must not be null"); + context->control_block->refs_added = 1; + } +} diff --git a/src/native/clr/host/gc-bridge.cc b/src/native/clr/host/gc-bridge.cc index ec65d891fa1..fe3a5527841 100644 --- a/src/native/clr/host/gc-bridge.cc +++ b/src/native/clr/host/gc-bridge.cc @@ -1,32 +1,40 @@ #include +#include #include #include #include #include -#include using namespace xamarin::android; -void GCBridge::initialize_on_load (JNIEnv *jniEnv) noexcept +void GCBridge::initialize_on_load (JNIEnv *env) noexcept { - abort_if_invalid_pointer_argument (jniEnv, "jniEnv"); + abort_if_invalid_pointer_argument (env, "env"); - jclass lref = jniEnv->FindClass ("java/lang/Runtime"); + jclass lref = env->FindClass ("java/lang/Runtime"); abort_unless (lref != nullptr, "Failed to look up java/lang/Runtime class."); - jmethodID Runtime_getRuntime = jniEnv->GetStaticMethodID (lref, "getRuntime", "()Ljava/lang/Runtime;"); + jmethodID Runtime_getRuntime = env->GetStaticMethodID (lref, "getRuntime", "()Ljava/lang/Runtime;"); abort_unless (Runtime_getRuntime != nullptr, "Failed to look up the Runtime.getRuntime() method."); - Runtime_gc = jniEnv->GetMethodID (lref, "gc", "()V"); + Runtime_gc = env->GetMethodID (lref, "gc", "()V"); abort_unless (Runtime_gc != nullptr, "Failed to look up the Runtime.gc() method."); - Runtime_instance = OSBridge::lref_to_gref (jniEnv, jniEnv->CallStaticObjectMethod (lref, Runtime_getRuntime)); + Runtime_instance = OSBridge::lref_to_gref (env, env->CallStaticObjectMethod (lref, Runtime_getRuntime)); abort_unless (Runtime_instance != nullptr, "Failed to obtain Runtime instance."); - jniEnv->DeleteLocalRef (lref); + env->DeleteLocalRef (lref); } -void GCBridge::trigger_java_gc () noexcept +void GCBridge::initialize_on_runtime_init (JNIEnv *env, jclass runtimeClass) noexcept +{ + abort_if_invalid_pointer_argument (env, "env"); + abort_if_invalid_pointer_argument (runtimeClass, "runtimeClass"); + + BridgeProcessing::initialize_on_runtime_init (env, runtimeClass); +} + +void GCBridge::trigger_java_gc (JNIEnv *env) noexcept { env->CallVoidMethod (Runtime_instance, Runtime_gc); @@ -39,264 +47,21 @@ void GCBridge::trigger_java_gc () noexcept log_error (LOG_DEFAULT, "Java GC failed"); } -void GCBridge::prepare_for_java_collection () noexcept -{ - // Some SCCs might have no IGCUserPeers associated with them, so we must create one - std::unordered_map temporary_peers; - - // Before looking at xrefs, scan the SCCs. During collection, an SCC has to behave like a - // single object. If the number of objects in the SCC is anything other than 1, the SCC - // must be doctored to mimic that one-object nature. - for (size_t i = 0; i < cross_refs.ComponentCount; i++) { - const StronglyConnectedComponent *scc = &cross_refs.Components [i]; - - // Count > 1 case: The SCC contains many objects which must be collected as one. - // Solution: Make all objects within the SCC directly or indirectly reference each other - if (scc->Count > 1) { - add_references (*scc); - } else if (scc->Count == 0) { - temporary_peers [i] = env->NewObject (GCUserPeer_class, GCUserPeer_ctor); - } - } - - // Add the cross scc refs - for (size_t i = 0; i < cross_refs.CrossReferenceCount; i++) { - add_cross_reference (i, temporary_peers); - } - - // With cross references processed, the temporary peer list can be released - for (const auto& [scc, temporary_peer] : temporary_peers) { - env->DeleteLocalRef (temporary_peer); - } - - // Switch global to weak references - for (size_t i = 0; i < cross_refs.ComponentCount; i++) { - const StronglyConnectedComponent &scc = cross_refs.Components [i]; - for (size_t j = 0; j < scc.Count; j++) { - take_weak_global_ref (scc.Contexts [j]); - } - } -} - -jobject GCBridge::pick_representative (size_t scc_index, const StronglyConnectedComponent &scc, const std::unordered_map &temporary_peers) noexcept -{ - if (scc.Count > 0) { - abort_unless (scc.Contexts [0] != nullptr, "SCC must have at least one context"); - abort_unless (scc.Contexts [0]->control_block != nullptr, "SCC must have at least one context with valid control block"); - return scc.Contexts [0]->control_block->handle; - } else { - const auto temporary_peer = temporary_peers.find (scc_index); - abort_unless (temporary_peer != temporary_peers.end(), "Temporary peer must be found in the map"); - return temporary_peer->second; - } -} - -void GCBridge::add_references (const StronglyConnectedComponent &scc) noexcept -{ - abort_unless (scc.Count > 1, "SCC must have at least two items to add inner references"); - - JniObjectReferenceControlBlock *prev = scc.Contexts [scc.Count - 1]->control_block; - - for (size_t j = 1; j < scc.Count; j++) { - JniObjectReferenceControlBlock *current = scc.Contexts [j]->control_block; - if (add_reference (prev->handle, current->handle)) { - prev->refs_added = 1; - } else { - // TODO - } - - prev = current; - } -} - -void GCBridge::add_cross_reference (size_t xref_index, const std::unordered_map &temporary_peers) noexcept -{ - const ComponentCrossReference &xref = cross_refs.CrossReferences [xref_index]; - - const StronglyConnectedComponent &source = cross_refs.Components [xref.SourceGroupIndex]; - jobject from = pick_representative (xref.SourceGroupIndex, source, temporary_peers); - - const StronglyConnectedComponent &dest = cross_refs.Components [xref.DestinationGroupIndex]; - jobject to = pick_representative (xref.DestinationGroupIndex, dest, temporary_peers); - - if (add_reference (from, to) && source.Count > 0) { - // If the source is a SCC with at least one item, we need to mark the first item it as having added a reference - source.Contexts [0]->control_block->refs_added = 1; - } -} - -bool GCBridge::add_reference (jobject from, jobject to) noexcept -{ - jclass java_class = env->GetObjectClass (from); - jmethodID add_method_id = env->GetMethodID (java_class, "monodroidAddReference", "(Ljava/lang/Object;)V"); - env->DeleteLocalRef (java_class); - - if (add_method_id != nullptr) { - env->CallVoidMethod (from, add_method_id, to); - return true; - } else { - env->ExceptionClear (); - return false; - } -} - -void GCBridge::clear_references (jobject handle) noexcept -{ - // Clear references from the object - jclass java_class = env->GetObjectClass (handle); - jmethodID clear_method_id = env->GetMethodID (java_class, "monodroidClearReferences", "()V"); - env->DeleteLocalRef (java_class); // Clean up the local reference to the class - - if (clear_method_id != nullptr) { - env->CallVoidMethod (handle, clear_method_id); - } else { - log_error (LOG_DEFAULT, "Failed to find monodroidClearReferences method"); - env->ExceptionClear (); -#if DEBUG - if (Logger::gc_spew_enabled ()) { - char *class_name = Host::get_java_class_name_for_TypeManager (java_class); - log_error (LOG_GC, "Missing monodroidClearReferences method for object of class {}", class_name); - free (class_name); - } -#endif - } -} - -void GCBridge::take_global_ref (HandleContext *context) noexcept -{ - abort_if_invalid_pointer_argument (context, "context"); - abort_unless (context->control_block != nullptr, "Control block must not be null"); - if (context->control_block->handle_type != JNIWeakGlobalRefType) [[unlikely]] { - log_error (LOG_DEFAULT, "Expected weak global reference type for handle, but got {} - handle: {:#x}", context->control_block->handle_type, reinterpret_cast (context->control_block->handle)); - return; - } - abort_unless (context->control_block->handle_type == JNIWeakGlobalRefType, "Expected weak global reference type for handle"); - - jobject weak = context->control_block->handle; - jobject handle = env->NewGlobalRef (weak); - - // if (Logger::gref_log ()) [[unlikely]] { - // OSBridge::_monodroid_gref_log ( - // std::format ("take_global_ref gchandle={:#x} -> wref={:#x} handle={:#x}\n"sv, - // reinterpret_cast (context->gc_handle), - // reinterpret_cast (weak), - // reinterpret_cast (handle)).data ()); - // } - - if (handle != nullptr) { - context->control_block->handle = handle; - context->control_block->handle_type = JNIGlobalRefType; - - OSBridge::_monodroid_gref_log_new (weak, OSBridge::get_object_ref_type (env, weak), - handle, OSBridge::get_object_ref_type (env, handle), - "finalizer", gettid (), - " at [[clr-gc:take_global_ref]]", 0); - - OSBridge::_monodroid_weak_gref_delete (weak, OSBridge::get_object_ref_type (env, weak), - "finalizer", gettid (), " at [[clr-gc:take_global_ref]]", 0); - env->DeleteWeakGlobalRef (weak); - } else { - // The native memory of the control block will be freed in managed code as well as the weak global ref - context->control_block = nullptr; - - // if (Logger::gc_spew_enabled ()) [[unlikely]] { - // OSBridge::_monodroid_gref_log ( - // std::format ("handle {:#x}/W; was collected by a Java GC"sv, reinterpret_cast (weak)).data ()); - // } - } -} - -void GCBridge::take_weak_global_ref (HandleContext *context) noexcept -{ - abort_if_invalid_pointer_argument (context, "context"); - abort_unless (context->control_block != nullptr, "Control block must not be null"); - abort_unless (context->control_block->handle_type == JNIGlobalRefType, "Expected global reference type for handle"); - - jobject handle = context->control_block->handle; - if (Logger::gref_log ()) [[unlikely]] { - OSBridge::_monodroid_gref_log ( - std::format ("take_weak_global_ref gchandle={:#x}; handle={:#x}\n"sv, - reinterpret_cast (context->gc_handle), - reinterpret_cast (handle)).data ()); - } - - jobject weak = env->NewWeakGlobalRef (handle); - OSBridge::_monodroid_weak_gref_new (handle, OSBridge::get_object_ref_type (env, handle), - weak, OSBridge::get_object_ref_type (env, weak), - "finalizer", gettid (), " at [[clr-gc:take_weak_global_ref]]", 0); - - context->control_block->handle = weak; - context->control_block->handle_type = JNIWeakGlobalRefType; - - log_error_fmt (LOG_GC, "take_weak_global_ref: gchandle={:#x} -> wref={:#x} handle={:#x} -> new type {}", - reinterpret_cast (context->gc_handle), - reinterpret_cast (weak), - reinterpret_cast (handle), - context->control_block->handle_type); - - OSBridge::_monodroid_gref_log_delete (handle, OSBridge::get_object_ref_type (env, handle), - "finalizer", gettid (), " at [[clr-gc:take_weak_global_ref]]", 0); - env->DeleteGlobalRef (handle); -} - -void GCBridge::cleanup_after_java_collection () noexcept -{ -#if DEBUG - int total = 0; - int alive = 0; -#endif - - // try to switch back to global refs to analyze what stayed alive - for (size_t i = 0; i < cross_refs.ComponentCount; i++) { - const StronglyConnectedComponent &scc = cross_refs.Components [i]; - for (size_t j = 0; j < scc.Count; j++) { - take_global_ref (scc.Contexts [j]); - } - } - - // clear the cross references on any remaining items - for (size_t i = 0; i < cross_refs.ComponentCount; i++) { - const StronglyConnectedComponent &scc = cross_refs.Components [i]; - [[maybe_unused]] bool is_alive = cleanup_strongly_connected_component (i, scc); - -#if DEBUG - total += scc.Count; - if (is_alive) { - alive += scc.Count; - } -#endif - } - -#if DEBUG - log_info (LOG_GC, "GC cleanup summary: {} objects tested - resurrecting {}.", total, alive); -#endif -} - -bool GCBridge::cleanup_strongly_connected_component (size_t i, const StronglyConnectedComponent &scc) noexcept +void GCBridge::mark_cross_references (MarkCrossReferencesArgs *args) noexcept { - // all contexts in the SCC must either be alive, or collected - bool is_alive = false; + abort_if_invalid_pointer_argument (args, "cross_refs"); + abort_unless (args->Components != nullptr || args->ComponentCount == 0, "Components must not be null if ComponentCount is greater than 0"); + abort_unless (args->CrossReferences != nullptr || args->CrossReferenceCount == 0, "CrossReferences must not be null if CrossReferenceCount is greater than 0"); + log_mark_cross_references_args_if_enabled (args); - for (size_t j = 0; j < scc.Count; j++) { - abort_unless (scc.Contexts [j] != nullptr, "Context must not be null"); - JniObjectReferenceControlBlock *control_block = scc.Contexts [j]->control_block; + std::unique_lock lock (processing_mutex); - if (control_block != nullptr) { - if (j > 0) { - abort_unless (is_alive, [&i] { return detail::_format_message ("Bridge SCC at index %d must be alive", i); }); - } + shared_cross_refs.ComponentCount = args->ComponentCount; + shared_cross_refs.Components = args->Components; + shared_cross_refs.CrossReferenceCount = args->CrossReferenceCount; + shared_cross_refs.CrossReferences = args->CrossReferences; - is_alive = true; - if (control_block->refs_added > 0) { - clear_references (control_block->handle); - control_block->refs_added = 0; - } - } else { - abort_unless (!is_alive, [&i] { return detail::_format_message ("Bridge SCC at index %d must NOT be alive", i); }); - } - } - - return is_alive; + bridge_processing_semaphore.release (); } void GCBridge::bridge_processing () noexcept @@ -304,40 +69,17 @@ void GCBridge::bridge_processing () noexcept abort_unless (bridge_processing_started_callback != nullptr, "GC bridge processing started callback is not set"); abort_unless (bridge_processing_finished_callback != nullptr, "GC bridge processing finished callback is not set"); - env = OSBridge::ensure_jnienv (); - while (true) { bridge_processing_semaphore.acquire (); std::unique_lock lock (processing_mutex); - log_mark_cross_references_args_if_enabled (); - bridge_processing_started_callback (); - prepare_for_java_collection (); - trigger_java_gc (); - cleanup_after_java_collection (); - bridge_processing_finished_callback (&cross_refs); - } -} -void GCBridge::mark_cross_references (MarkCrossReferencesArgs *args) noexcept -{ - static int ctr = 0; - log_error (LOG_DEFAULT, "mark_cross_references called for the {} time", ++ctr); - - abort_if_invalid_pointer_argument (args, "cross_refs"); - abort_unless (args->Components != nullptr || args->ComponentCount == 0, "Components must not be null if ComponentCount is greater than 0"); - abort_unless (args->CrossReferences != nullptr || args->CrossReferenceCount == 0, "CrossReferences must not be null if CrossReferenceCount is greater than 0"); + BridgeProcessing bridge_processing {shared_cross_refs}; + bridge_processing.process (); - std::unique_lock lock (processing_mutex); - - log_error (LOG_DEFAULT, "setting cross_refs"); - cross_refs.ComponentCount = args->ComponentCount; - cross_refs.Components = args->Components; - cross_refs.CrossReferenceCount = args->CrossReferenceCount; - cross_refs.CrossReferences = args->CrossReferences; - - bridge_processing_semaphore.release (); + bridge_processing_finished_callback (&shared_cross_refs); + } } void GCBridge::wait_for_bridge_processing () noexcept @@ -346,18 +88,20 @@ void GCBridge::wait_for_bridge_processing () noexcept } [[gnu::always_inline]] -void GCBridge::log_mark_cross_references_args_if_enabled () noexcept +void GCBridge::log_mark_cross_references_args_if_enabled (MarkCrossReferencesArgs *args) noexcept { if (!Logger::gc_spew_enabled ()) [[likely]] { return; } - log_info (LOG_GC, "cross references callback invoked with {} sccs and {} xrefs.", cross_refs.ComponentCount, cross_refs.CrossReferenceCount); + log_info (LOG_GC, "cross references callback invoked with {} sccs and {} xrefs.", args->ComponentCount, args->CrossReferenceCount); - for (size_t i = 0; i < cross_refs.ComponentCount; ++i) { - log_info (LOG_GC, "group {} with {} objects", i, cross_refs.Components [i].Count); - for (size_t j = 0; j < cross_refs.Components [i].Count; ++j) { - HandleContext *ctx = cross_refs.Components [i].Contexts [j]; + JNIEnv *env = OSBridge::ensure_jnienv (); + + for (size_t i = 0; i < args->ComponentCount; ++i) { + log_info (LOG_GC, "group {} with {} objects", i, args->Components [i].Count); + for (size_t j = 0; j < args->Components [i].Count; ++j) { + HandleContext *ctx = args->Components [i].Contexts [j]; abort_unless (ctx != nullptr, "Context must not be null"); abort_unless (ctx->control_block != nullptr, "Control block must not be null"); @@ -377,9 +121,9 @@ void GCBridge::log_mark_cross_references_args_if_enabled () noexcept return; } - for (size_t i = 0; i < cross_refs.CrossReferenceCount; ++i) { - size_t source_index = cross_refs.CrossReferences [i].SourceGroupIndex; - size_t dest_index = cross_refs.CrossReferences [i].DestinationGroupIndex; + for (size_t i = 0; i < args->CrossReferenceCount; ++i) { + size_t source_index = args->CrossReferences [i].SourceGroupIndex; + size_t dest_index = args->CrossReferences [i].DestinationGroupIndex; log_info_nocheck_fmt (LOG_GC, "xref [{}] {} -> {}", i, source_index, dest_index); } } diff --git a/src/native/clr/host/host.cc b/src/native/clr/host/host.cc index d5b4463f63f..17554133bd3 100644 --- a/src/native/clr/host/host.cc +++ b/src/native/clr/host/host.cc @@ -453,9 +453,8 @@ void Host::Java_mono_android_Runtime_initInternal ( log_info (LOG_GC, "GREF GC Threshold: {}"sv, init.grefGcThreshold); - // TODO: GC bridge to initialize here - OSBridge::initialize_on_runtime_init (env, runtimeClass); + GCBridge::initialize_on_runtime_init (env, runtimeClass); if (FastTiming::enabled ()) [[unlikely]] { internal_timing.start_event (TimingEventKind::NativeToManagedTransition); diff --git a/src/native/clr/include/host/bridge-processing.hh b/src/native/clr/include/host/bridge-processing.hh new file mode 100644 index 00000000000..44544e9c556 --- /dev/null +++ b/src/native/clr/include/host/bridge-processing.hh @@ -0,0 +1,49 @@ +#pragma once + +#include +#include + +#include +#include +#include + +struct CrossReferenceTarget +{ + bool is_temporary_peer; + union + { + jobject temporary_peer; + HandleContext* context; + }; + + jobject get_handle () const noexcept; + void mark_refs_added_if_needed () noexcept; +}; + +class BridgeProcessing +{ +public: + BridgeProcessing (MarkCrossReferencesArgs args) noexcept; + static void initialize_on_runtime_init (JNIEnv *jniEnv, jclass runtimeClass) noexcept; + void process () noexcept; +private: + JNIEnv* env; + MarkCrossReferencesArgs cross_refs; + std::unordered_map temporary_peers; + + static inline jclass GCUserPeer_class = nullptr; + static inline jmethodID GCUserPeer_ctor = nullptr; + + void prepare_for_java_collection () noexcept; + void cleanup_after_java_collection () noexcept; + bool cleanup_strongly_connected_component (size_t i, const StronglyConnectedComponent &scc) noexcept; + + void take_weak_global_ref (HandleContext *context) noexcept; + void take_global_ref (HandleContext *context) noexcept; + + void add_circular_references (const StronglyConnectedComponent &scc) noexcept; + void add_cross_reference (size_t source_index, size_t dest_index) noexcept; + CrossReferenceTarget select_cross_reference_target (size_t scc_index) noexcept; + bool add_reference (jobject from, jobject to) noexcept; + void clear_references_if_needed (JniObjectReferenceControlBlock *control_block) noexcept; +}; diff --git a/src/native/clr/include/host/gc-bridge.hh b/src/native/clr/include/host/gc-bridge.hh index 8582352e4f9..be7950f9c5a 100644 --- a/src/native/clr/include/host/gc-bridge.hh +++ b/src/native/clr/include/host/gc-bridge.hh @@ -52,6 +52,8 @@ namespace xamarin::android { public: static void wait_for_bridge_processing () noexcept; static void initialize_on_load (JNIEnv *env) noexcept; + static void initialize_on_runtime_init (JNIEnv *env, jclass runtimeClass) noexcept; + static BridgeProcessingFtn initialize_callback ( BridgeProcessingStartedFtn bridge_processing_started, BridgeProcessingFinishedFtn bridge_processing_finished) noexcept @@ -70,44 +72,24 @@ namespace xamarin::android { return mark_cross_references; } + static void trigger_java_gc (JNIEnv *env) noexcept; + private: static inline std::binary_semaphore bridge_processing_semaphore{0}; static inline std::shared_mutex processing_mutex; static inline std::thread* bridge_processing_thread = nullptr; + + static inline MarkCrossReferencesArgs shared_cross_refs; - static inline JNIEnv* env = nullptr; static inline jobject Runtime_instance = nullptr; static inline jmethodID Runtime_gc = nullptr; - static inline jclass GCUserPeer_class = nullptr; - static inline jmethodID GCUserPeer_ctor = nullptr; - - static inline MarkCrossReferencesArgs cross_refs; static inline BridgeProcessingStartedFtn bridge_processing_started_callback = nullptr; static inline BridgeProcessingFinishedFtn bridge_processing_finished_callback = nullptr; static void bridge_processing () noexcept; static void mark_cross_references (MarkCrossReferencesArgs *cross_refs) noexcept; - - static void prepare_for_java_collection () noexcept; - static void trigger_java_gc () noexcept; - static void cleanup_after_java_collection () noexcept; - static bool cleanup_strongly_connected_component (size_t i, const StronglyConnectedComponent &scc) noexcept; - - static void take_weak_global_ref (HandleContext *context) noexcept; - static void take_global_ref (HandleContext *context) noexcept; - - static bool add_reference (jobject from, jobject to) noexcept; - static void clear_references (jobject handle) noexcept; - static void add_references (const StronglyConnectedComponent &scc) noexcept; - static void add_cross_reference ( - size_t xref_index, - const std::unordered_map &temporary_peers) noexcept; - static jobject pick_representative ( - size_t scc_index, - const StronglyConnectedComponent &scc, - const std::unordered_map &temporary_peers) noexcept; - - static void log_mark_cross_references_args_if_enabled () noexcept; + + static void log_mark_cross_references_args_if_enabled (MarkCrossReferencesArgs *args) noexcept; }; } From 369dd188c15a196061cd975dcd13b9e15e621bf4 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 17 Jun 2025 11:23:41 +0200 Subject: [PATCH 04/32] Refactor HandleContext and ReferenceTrackingHandle --- .../ManagedValueManager.cs | 176 ++++++++++-------- src/native/clr/host/bridge-processing.cc | 32 ++-- src/native/clr/host/gc-bridge.cc | 16 +- src/native/clr/include/host/gc-bridge.hh | 1 - 4 files changed, 120 insertions(+), 105 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index 75cae624385..0f01a47facd 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -21,7 +21,7 @@ class ManagedValueManager : JniRuntime.JniValueManager { const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors; - Dictionary>? RegisteredInstances = new (); + Dictionary>? RegisteredInstances = new (); static Lazy s_instance = new (() => new ManagedValueManager ()); public static ManagedValueManager GetOrCreateInstance () => s_instance.Value; @@ -58,9 +58,9 @@ public override void AddPeer (IJavaPeerable value) } int key = value.JniIdentityHashCode; lock (RegisteredInstances) { - List? peers; + List? peers; if (!RegisteredInstances.TryGetValue (key, out peers)) { - peers = [new WeakPeerReference (value)]; + peers = [new ReferenceTrackingHandle (value)]; RegisteredInstances.Add (key, peers); return; } @@ -73,7 +73,7 @@ public override void AddPeer (IJavaPeerable value) continue; if (peer.JniManagedPeerState.HasFlag (JniManagedPeerStates.Replaceable)) { p.Dispose (); - peers [i] = new WeakPeerReference (value); + peers [i] = new ReferenceTrackingHandle (value); GC.KeepAlive (peer); } else { WarnNotReplacing (key, value, peer); @@ -81,7 +81,7 @@ public override void AddPeer (IJavaPeerable value) return; } - peers.Add (new WeakPeerReference (value)); + peers.Add (new ReferenceTrackingHandle (value)); } } @@ -112,7 +112,7 @@ void WarnNotReplacing (int key, IJavaPeerable ignoreValue, IJavaPeerable keepVal int key = GetJniIdentityHashCode (reference); lock (RegisteredInstances) { - if (!RegisteredInstances.TryGetValue (key, out List? peers)) + if (!RegisteredInstances.TryGetValue (key, out List? peers)) return null; for (int i = peers.Count - 1; i >= 0; i--) { @@ -138,25 +138,23 @@ public override void RemovePeer (IJavaPeerable value) throw new ArgumentNullException (nameof (value)); lock (RegisteredInstances) { - RemoveRegisteredInstance (value, out WeakPeerReference? removedPeer); - removedPeer?.Dispose (); - GC.KeepAlive (value); + RemoveRegisteredInstance (value, disposeReferenceTrackingHandle: true); } } - private void RemoveRegisteredInstance (IJavaPeerable target, out WeakPeerReference? removedPeer) + private void RemoveRegisteredInstance (IJavaPeerable target, bool disposeReferenceTrackingHandle) { - removedPeer = default; - int key = target.JniIdentityHashCode; - if (!RegisteredInstances.TryGetValue (key, out List? peers)) + if (!RegisteredInstances.TryGetValue (key, out List? peers)) return; for (int i = peers.Count - 1; i >= 0; i--) { var peer = peers [i]; if (object.ReferenceEquals (target, peer.Target)) { peers.RemoveAt (i); - removedPeer = peer; + if (disposeReferenceTrackingHandle) { + peer.Dispose (); + } } } if (peers.Count == 0) @@ -251,111 +249,139 @@ public override List GetSurfacedPeers () } } - unsafe struct WeakPeerReference : IDisposable + unsafe struct ReferenceTrackingHandle : IDisposable { private WeakReference _weakReference; - private GCHandle _handle; + private HandleContext* _context; - public WeakPeerReference (IJavaPeerable peer) + public ReferenceTrackingHandle (IJavaPeerable peer) { + _context = HandleContext.Alloc (peer); _weakReference = new WeakReference (peer); + } - var handleContext = (HandleContext*) NativeMemory.AllocZeroed (1, HandleContext.Size); - if (handleContext == null) { - throw new OutOfMemoryException ("Failed to allocate memory for HandleContext."); - } + public IJavaPeerable? Target => _weakReference.TryGetTarget (out var target) ? target : null; - _handle = JavaMarshal.CreateReferenceTrackingHandle (peer, handleContext); + public void Dispose () + { + if (_context == null) + return; - handleContext->Handle = GCHandle.ToIntPtr (_handle); - handleContext->ControlBlock = peer.JniObjectReferenceControlBlock; + IJavaPeerable? target = Target; + + GCHandle handle = HandleContext.GetAssociatedGCHandle (_context); + HandleContext.Free (ref _context); + _weakReference.SetTarget (null); + if (handle.IsAllocated) { + handle.Free (); + } + + // Make sure the target is not collected before we finish disposing + GC.KeepAlive (target); } + } - public void FreeContext () + [StructLayout (LayoutKind.Sequential)] + unsafe struct HandleContext + { + static readonly nuint Size = (nuint)Marshal.SizeOf (); + static readonly Dictionary referenceTrackingHandles = new (); + + IntPtr controlBlock; + + public bool IsCollected => controlBlock == IntPtr.Zero; + + public static GCHandle GetAssociatedGCHandle (HandleContext* context) { - var context = (HandleContext*) JavaMarshal.GetContext (_handle); - if (context != null) { - NativeMemory.Free (context); + lock (referenceTrackingHandles) + { + if (!referenceTrackingHandles.TryGetValue ((IntPtr)context, out GCHandle handle)) { + throw new InvalidOperationException ("Unknown reference tracking handle."); + } + + return handle; } } - /// - /// Calling this Dispose method is legal only if the caller holds a reference to the Target - /// to prevent data race with the GC. - /// - public void Dispose() + public static HandleContext* Alloc (IJavaPeerable peer) { - FreeContext (); - _handle.Free (); - } + var context = (HandleContext*)NativeMemory.AllocZeroed (1, Size); + if (context == null) { + throw new OutOfMemoryException ("Failed to allocate memory for HandleContext."); + } - public IJavaPeerable? Target - => _weakReference.TryGetTarget (out var target) ? target : null; - } + context->controlBlock = peer.JniObjectReferenceControlBlock; - [StructLayout (LayoutKind.Sequential)] - struct HandleContext - { - public static readonly nuint Size = (nuint) Marshal.SizeOf (); + GCHandle handle = JavaMarshal.CreateReferenceTrackingHandle (peer, context); + lock (referenceTrackingHandles) { + referenceTrackingHandles [(IntPtr)context] = handle; + } + + return context; + } - public IntPtr Handle; - public IntPtr ControlBlock; + public static void Free (ref HandleContext* context) + { + if (context != null) { + NativeMemory.Free (context); + context = null; + } + } } [UnmanagedCallersOnly] - internal static void BridgeProcessingStarted () + static void BridgeProcessingStarted () { AndroidRuntimeInternal.BridgeProcessing = true; } [UnmanagedCallersOnly] - internal static unsafe void BridgeProcessingFinished(MarkCrossReferencesArgs* mcr) + static unsafe void BridgeProcessingFinished (MarkCrossReferencesArgs* mcr) { Trace.Assert (mcr != null, "Bridge processing was not started before finishing it."); - ManagedValueManager instance = GetOrCreateInstance(); - IEnumerable collectedContexts = GetCollectedContexts (mcr); - - List handlesToFree; - lock (instance.RegisteredInstances) { - handlesToFree = ProcessCollectedContexts (instance, collectedContexts); - } + ReadOnlySpan handlesToFree = ProcessCollectedContexts (mcr); + JavaMarshal.FinishCrossReferenceProcessing (mcr, handlesToFree); - JavaMarshal.FinishCrossReferenceProcessing (mcr, CollectionsMarshal.AsSpan (handlesToFree)); AndroidRuntimeInternal.BridgeProcessing = false; } - static unsafe List GetCollectedContexts (MarkCrossReferencesArgs* mcr) + static unsafe ReadOnlySpan ProcessCollectedContexts (MarkCrossReferencesArgs* mcr) { - List contexts = []; + List handlesToFree = []; + ManagedValueManager instance = GetOrCreateInstance (); - for (int i = 0; (nuint)i < mcr->ComponentCount; i++) { - for (int j = 0; (nuint)j < mcr->Components [i].Count; j++) { - var context = (HandleContext*)mcr->Components [i].Contexts [j]; - if (context->ControlBlock == IntPtr.Zero) { - contexts.Add ((IntPtr)context); - } + lock (instance.RegisteredInstances) { + for (int i = 0; (nuint)i < mcr->ComponentCount; i++) { + ProcessComponent (mcr->Components [i]); } } - return contexts; - } - - static unsafe List ProcessCollectedContexts (ManagedValueManager instance, IEnumerable collectedContexts) - { - List handlesToFree = []; + void ProcessComponent (StronglyConnectedComponent component) + { + for (int j = 0; (nuint)j < component.Count; j++) { + ProcessContext ((HandleContext*)component.Contexts [j]); + } + } - foreach (HandleContext* context in collectedContexts) { - var handle = GCHandle.FromIntPtr (context->Handle); + void ProcessContext (HandleContext* context) + { + Trace.Assert (context != null, "Context should never be null."); - // Clean up the RegisteredInstances dictionary + free the control block native memory associated with the handle - instance.RemoveRegisteredInstance ((IJavaPeerable)handle.Target, out WeakPeerReference? removedPeer); - removedPeer?.FreeContext (); + // ignore contexts which were not collected + if (!context->IsCollected) { + return; + } + // ignore contexts which were not allocated by the ManagedValueManager + GCHandle handle = HandleContext.GetAssociatedGCHandle (context); + IJavaPeerable peer = (IJavaPeerable)handle.Target; + instance.RemoveRegisteredInstance (peer, disposeReferenceTrackingHandle: false); + HandleContext.Free (ref context); handlesToFree.Add (handle); } - return handlesToFree; + return CollectionsMarshal.AsSpan (handlesToFree); } const BindingFlags ActivationConstructorBindingFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; diff --git a/src/native/clr/host/bridge-processing.cc b/src/native/clr/host/bridge-processing.cc index e4cf5edbe30..b7980ac6055 100644 --- a/src/native/clr/host/bridge-processing.cc +++ b/src/native/clr/host/bridge-processing.cc @@ -182,13 +182,12 @@ void BridgeProcessing::take_global_ref (HandleContext *context) noexcept jobject weak = context->control_block->handle; jobject handle = env->NewGlobalRef (weak); - // if (Logger::gref_log ()) [[unlikely]] { - // OSBridge::_monodroid_gref_log ( - // std::format ("take_global_ref gchandle={:#x} -> wref={:#x} handle={:#x}\n"sv, - // reinterpret_cast (context->gc_handle), - // reinterpret_cast (weak), - // reinterpret_cast (handle)).data ()); - // } + if (Logger::gref_log ()) [[unlikely]] { + OSBridge::_monodroid_gref_log ( + std::format ("take_global_ref wref={:#x} -> handle={:#x}\n"sv, + reinterpret_cast (weak), + reinterpret_cast (handle)).data ()); + } if (handle != nullptr) { context->control_block->handle = handle; @@ -206,10 +205,10 @@ void BridgeProcessing::take_global_ref (HandleContext *context) noexcept // The native memory of the control block will be freed in managed code as well as the weak global ref context->control_block = nullptr; - // if (Logger::gc_spew_enabled ()) [[unlikely]] { - // OSBridge::_monodroid_gref_log ( - // std::format ("handle {:#x}/W; was collected by a Java GC"sv, reinterpret_cast (weak)).data ()); - // } + if (Logger::gc_spew_enabled ()) [[unlikely]] { + OSBridge::_monodroid_gref_log ( + std::format ("handle {:#x}/W; was collected by a Java GC"sv, reinterpret_cast (weak)).data ()); + } } } @@ -221,10 +220,7 @@ void BridgeProcessing::take_weak_global_ref (HandleContext *context) noexcept jobject handle = context->control_block->handle; if (Logger::gref_log ()) [[unlikely]] { - OSBridge::_monodroid_gref_log ( - std::format ("take_weak_global_ref gchandle={:#x}; handle={:#x}\n"sv, - reinterpret_cast (context->gc_handle), - reinterpret_cast (handle)).data ()); + OSBridge::_monodroid_gref_log (std::format ("take_weak_global_ref handle={:#x}\n"sv, reinterpret_cast (handle)).data ()); } jobject weak = env->NewWeakGlobalRef (handle); @@ -235,12 +231,6 @@ void BridgeProcessing::take_weak_global_ref (HandleContext *context) noexcept context->control_block->handle = weak; context->control_block->handle_type = JNIWeakGlobalRefType; - log_error_fmt (LOG_GC, "take_weak_global_ref: gchandle={:#x} -> wref={:#x} handle={:#x} -> new type {}", - reinterpret_cast (context->gc_handle), - reinterpret_cast (weak), - reinterpret_cast (handle), - context->control_block->handle_type); - OSBridge::_monodroid_gref_log_delete (handle, OSBridge::get_object_ref_type (env, handle), "finalizer", gettid (), " at [[clr-gc:take_weak_global_ref]]", 0); env->DeleteGlobalRef (handle); diff --git a/src/native/clr/host/gc-bridge.cc b/src/native/clr/host/gc-bridge.cc index fe3a5527841..859515f4e9d 100644 --- a/src/native/clr/host/gc-bridge.cc +++ b/src/native/clr/host/gc-bridge.cc @@ -11,19 +11,19 @@ void GCBridge::initialize_on_load (JNIEnv *env) noexcept { abort_if_invalid_pointer_argument (env, "env"); - jclass lref = env->FindClass ("java/lang/Runtime"); - abort_unless (lref != nullptr, "Failed to look up java/lang/Runtime class."); + jclass Runtime_class = env->FindClass ("java/lang/Runtime"); + abort_unless (Runtime_class != nullptr, "Failed to look up java/lang/Runtime class."); - jmethodID Runtime_getRuntime = env->GetStaticMethodID (lref, "getRuntime", "()Ljava/lang/Runtime;"); + jmethodID Runtime_getRuntime = env->GetStaticMethodID (Runtime_class, "getRuntime", "()Ljava/lang/Runtime;"); abort_unless (Runtime_getRuntime != nullptr, "Failed to look up the Runtime.getRuntime() method."); - Runtime_gc = env->GetMethodID (lref, "gc", "()V"); + Runtime_gc = env->GetMethodID (Runtime_class, "gc", "()V"); abort_unless (Runtime_gc != nullptr, "Failed to look up the Runtime.gc() method."); - Runtime_instance = OSBridge::lref_to_gref (env, env->CallStaticObjectMethod (lref, Runtime_getRuntime)); + Runtime_instance = OSBridge::lref_to_gref (env, env->CallStaticObjectMethod (Runtime_class, Runtime_getRuntime)); abort_unless (Runtime_instance != nullptr, "Failed to obtain Runtime instance."); - env->DeleteLocalRef (lref); + env->DeleteLocalRef (Runtime_class); } void GCBridge::initialize_on_runtime_init (JNIEnv *env, jclass runtimeClass) noexcept @@ -109,10 +109,10 @@ void GCBridge::log_mark_cross_references_args_if_enabled (MarkCrossReferencesArg jclass java_class = env->GetObjectClass (handle); if (java_class != nullptr) { char *class_name = Host::get_java_class_name_for_TypeManager (java_class); - log_info (LOG_GC, "\tgchandle {:#x} gref {:#x} [{}]", reinterpret_cast (ctx->gc_handle), reinterpret_cast (handle), class_name); + log_info (LOG_GC, "gref {:#x} [{}]", reinterpret_cast (handle), class_name); free (class_name); } else { - log_info (LOG_GC, "\tgchandle {:#x} gref {:#x} [unknown class]", reinterpret_cast (ctx->gc_handle), reinterpret_cast (handle)); + log_info (LOG_GC, "gref {:#x} [unknown class]", reinterpret_cast (handle)); } } } diff --git a/src/native/clr/include/host/gc-bridge.hh b/src/native/clr/include/host/gc-bridge.hh index be7950f9c5a..21dc38fe767 100644 --- a/src/native/clr/include/host/gc-bridge.hh +++ b/src/native/clr/include/host/gc-bridge.hh @@ -18,7 +18,6 @@ struct JniObjectReferenceControlBlock struct HandleContext { - intptr_t gc_handle; JniObjectReferenceControlBlock* control_block; }; From 960945d48e030b71dc160c5e772bd8011331681c Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 18 Jun 2025 09:03:59 +0200 Subject: [PATCH 05/32] Update GC Bridge API to respect changes to cross_refs lifetime changes --- src/native/clr/host/bridge-processing.cc | 30 +++++++++---------- src/native/clr/host/gc-bridge.cc | 9 ++---- .../clr/include/host/bridge-processing.hh | 4 +-- src/native/clr/include/host/gc-bridge.hh | 4 +-- 4 files changed, 22 insertions(+), 25 deletions(-) diff --git a/src/native/clr/host/bridge-processing.cc b/src/native/clr/host/bridge-processing.cc index b7980ac6055..2503efa5982 100644 --- a/src/native/clr/host/bridge-processing.cc +++ b/src/native/clr/host/bridge-processing.cc @@ -18,9 +18,9 @@ void BridgeProcessing::initialize_on_runtime_init (JNIEnv *env, jclass runtimeCl abort_unless (GCUserPeer_class != nullptr && GCUserPeer_ctor != nullptr, "Failed to load mono.android.GCUserPeer!"); } -BridgeProcessing::BridgeProcessing (MarkCrossReferencesArgs args) noexcept - : env (OSBridge::ensure_jnienv ()), - cross_refs (args) +BridgeProcessing::BridgeProcessing (MarkCrossReferencesArgs *args) noexcept + : env{ OSBridge::ensure_jnienv () }, + cross_refs{ args } { } @@ -36,8 +36,8 @@ void BridgeProcessing::prepare_for_java_collection () noexcept // Before looking at xrefs, scan the SCCs. During collection, an SCC has to behave like a // single object. If the number of objects in the SCC is anything other than 1, the SCC // must be doctored to mimic that one-object nature. - for (size_t i = 0; i < cross_refs.ComponentCount; i++) { - const StronglyConnectedComponent &scc = cross_refs.Components [i]; + for (size_t i = 0; i < cross_refs->ComponentCount; i++) { + const StronglyConnectedComponent &scc = cross_refs->Components [i]; // Count > 1 case: The SCC contains many objects which must be collected as one. // Solution: Make all objects within the SCC directly or indirectly reference each other @@ -50,8 +50,8 @@ void BridgeProcessing::prepare_for_java_collection () noexcept } // Add the cross scc refs - for (size_t i = 0; i < cross_refs.CrossReferenceCount; i++) { - const ComponentCrossReference &xref = cross_refs.CrossReferences [i]; + for (size_t i = 0; i < cross_refs->CrossReferenceCount; i++) { + const ComponentCrossReference &xref = cross_refs->CrossReferences [i]; add_cross_reference (xref.SourceGroupIndex, xref.DestinationGroupIndex); } @@ -61,8 +61,8 @@ void BridgeProcessing::prepare_for_java_collection () noexcept } // Switch global to weak references - for (size_t i = 0; i < cross_refs.ComponentCount; i++) { - const StronglyConnectedComponent &scc = cross_refs.Components [i]; + for (size_t i = 0; i < cross_refs->ComponentCount; i++) { + const StronglyConnectedComponent &scc = cross_refs->Components [i]; for (size_t j = 0; j < scc.Count; j++) { take_weak_global_ref (scc.Contexts [j]); } @@ -71,7 +71,7 @@ void BridgeProcessing::prepare_for_java_collection () noexcept CrossReferenceTarget BridgeProcessing::select_cross_reference_target (size_t scc_index) noexcept { - const StronglyConnectedComponent &scc = cross_refs.Components [scc_index]; + const StronglyConnectedComponent &scc = cross_refs->Components [scc_index]; if (scc.Count > 0) { abort_unless (scc.Contexts [0] != nullptr, "SCC must have at least one context"); abort_unless (scc.Contexts [0]->control_block != nullptr, "SCC must have at least one context with valid control block"); @@ -172,7 +172,7 @@ void BridgeProcessing::clear_references_if_needed (JniObjectReferenceControlBloc void BridgeProcessing::take_global_ref (HandleContext *context) noexcept { abort_if_invalid_pointer_argument (context, "context"); - abort_unless (context->control_block != nullptr, "Control block must not be null"); + abort_unless (context->control_block != nullptr, "Control block must not be null"); // TODO: this sometimes throws -- looks like a race condition? if (context->control_block->handle_type != JNIWeakGlobalRefType) [[unlikely]] { log_error (LOG_DEFAULT, "Expected weak global reference type for handle, but got {} - handle: {:#x}", context->control_block->handle_type, reinterpret_cast (context->control_block->handle)); return; @@ -244,16 +244,16 @@ void BridgeProcessing::cleanup_after_java_collection () noexcept #endif // try to switch back to global refs to analyze what stayed alive - for (size_t i = 0; i < cross_refs.ComponentCount; i++) { - const StronglyConnectedComponent &scc = cross_refs.Components [i]; + for (size_t i = 0; i < cross_refs->ComponentCount; i++) { + const StronglyConnectedComponent &scc = cross_refs->Components [i]; for (size_t j = 0; j < scc.Count; j++) { take_global_ref (scc.Contexts [j]); } } // clear the cross references on any remaining items - for (size_t i = 0; i < cross_refs.ComponentCount; i++) { - const StronglyConnectedComponent &scc = cross_refs.Components [i]; + for (size_t i = 0; i < cross_refs->ComponentCount; i++) { + const StronglyConnectedComponent &scc = cross_refs->Components [i]; [[maybe_unused]] bool is_alive = cleanup_strongly_connected_component (i, scc); #if DEBUG diff --git a/src/native/clr/host/gc-bridge.cc b/src/native/clr/host/gc-bridge.cc index 859515f4e9d..993058a493c 100644 --- a/src/native/clr/host/gc-bridge.cc +++ b/src/native/clr/host/gc-bridge.cc @@ -56,10 +56,7 @@ void GCBridge::mark_cross_references (MarkCrossReferencesArgs *args) noexcept std::unique_lock lock (processing_mutex); - shared_cross_refs.ComponentCount = args->ComponentCount; - shared_cross_refs.Components = args->Components; - shared_cross_refs.CrossReferenceCount = args->CrossReferenceCount; - shared_cross_refs.CrossReferences = args->CrossReferences; + cross_refs = args; bridge_processing_semaphore.release (); } @@ -75,10 +72,10 @@ void GCBridge::bridge_processing () noexcept bridge_processing_started_callback (); - BridgeProcessing bridge_processing {shared_cross_refs}; + BridgeProcessing bridge_processing {cross_refs}; bridge_processing.process (); - bridge_processing_finished_callback (&shared_cross_refs); + bridge_processing_finished_callback (cross_refs); } } diff --git a/src/native/clr/include/host/bridge-processing.hh b/src/native/clr/include/host/bridge-processing.hh index 44544e9c556..e02f8919cd0 100644 --- a/src/native/clr/include/host/bridge-processing.hh +++ b/src/native/clr/include/host/bridge-processing.hh @@ -23,12 +23,12 @@ struct CrossReferenceTarget class BridgeProcessing { public: - BridgeProcessing (MarkCrossReferencesArgs args) noexcept; + BridgeProcessing (MarkCrossReferencesArgs *args) noexcept; static void initialize_on_runtime_init (JNIEnv *jniEnv, jclass runtimeClass) noexcept; void process () noexcept; private: JNIEnv* env; - MarkCrossReferencesArgs cross_refs; + MarkCrossReferencesArgs *cross_refs; std::unordered_map temporary_peers; static inline jclass GCUserPeer_class = nullptr; diff --git a/src/native/clr/include/host/gc-bridge.hh b/src/native/clr/include/host/gc-bridge.hh index 21dc38fe767..46885c27fd6 100644 --- a/src/native/clr/include/host/gc-bridge.hh +++ b/src/native/clr/include/host/gc-bridge.hh @@ -76,9 +76,9 @@ namespace xamarin::android { private: static inline std::binary_semaphore bridge_processing_semaphore{0}; static inline std::shared_mutex processing_mutex; - static inline std::thread* bridge_processing_thread = nullptr; + static inline std::thread *bridge_processing_thread = nullptr; - static inline MarkCrossReferencesArgs shared_cross_refs; + static inline MarkCrossReferencesArgs *cross_refs; static inline jobject Runtime_instance = nullptr; static inline jmethodID Runtime_gc = nullptr; From 212c1cc9b7aebb6a9d0a0046a43a36f4d57a0f3a Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 18 Jun 2025 09:38:47 +0200 Subject: [PATCH 06/32] Remove unnecessary util methods --- src/native/clr/host/bridge-processing.cc | 1 - src/native/clr/include/runtime-base/util.hh | 2 -- src/native/clr/runtime-base/util.cc | 21 --------------------- 3 files changed, 24 deletions(-) diff --git a/src/native/clr/host/bridge-processing.cc b/src/native/clr/host/bridge-processing.cc index 2503efa5982..9742fa1ecb1 100644 --- a/src/native/clr/host/bridge-processing.cc +++ b/src/native/clr/host/bridge-processing.cc @@ -1,6 +1,5 @@ #include #include -#include #include #include diff --git a/src/native/clr/include/runtime-base/util.hh b/src/native/clr/include/runtime-base/util.hh index 553b7aeab71..a301b841148 100644 --- a/src/native/clr/include/runtime-base/util.hh +++ b/src/native/clr/include/runtime-base/util.hh @@ -59,8 +59,6 @@ namespace xamarin::android { static auto monodroid_fopen (std::string_view const& filename, std::string_view const& mode) noexcept -> FILE*; static void set_world_accessable (std::string_view const& path); static auto set_world_accessible (int fd) noexcept -> bool; - static auto monodroid_strdup_printf (std::string_view const& format, ...) -> char*; - static auto monodroid_strdup_vprintf (std::string_view const& format, va_list vargs) -> char*; // Puts higher half of the `value` byte as a hexadecimal character in `high_half` and // the lower half in `low_half` diff --git a/src/native/clr/runtime-base/util.cc b/src/native/clr/runtime-base/util.cc index 785a07f4a33..85c2b339ad4 100644 --- a/src/native/clr/runtime-base/util.cc +++ b/src/native/clr/runtime-base/util.cc @@ -105,24 +105,3 @@ auto Util::set_world_accessible (int fd) noexcept -> bool return true; } - -char * -Util::monodroid_strdup_printf (std::string_view const& format, ...) -{ - va_list args; - - va_start (args, format); - char *ret = monodroid_strdup_vprintf (format, args); - va_end (args); - - return ret; -} - -char* -Util::monodroid_strdup_vprintf (std::string_view const& format, va_list vargs) -{ - char *ret = nullptr; - int n = vasprintf (&ret, format.data (), vargs); - - return n == -1 ? nullptr : ret; -} From 72ba81b7dd08e8d150d79f19f7bbc35d102deef5 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 18 Jun 2025 11:27:37 +0200 Subject: [PATCH 07/32] Move logging to separate methods to reduce noise --- src/native/clr/host/bridge-processing.cc | 230 +++++++++++------- .../clr/include/host/bridge-processing.hh | 20 +- 2 files changed, 164 insertions(+), 86 deletions(-) diff --git a/src/native/clr/host/bridge-processing.cc b/src/native/clr/host/bridge-processing.cc index 9742fa1ecb1..9b54307d3f4 100644 --- a/src/native/clr/host/bridge-processing.cc +++ b/src/native/clr/host/bridge-processing.cc @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -37,15 +38,7 @@ void BridgeProcessing::prepare_for_java_collection () noexcept // must be doctored to mimic that one-object nature. for (size_t i = 0; i < cross_refs->ComponentCount; i++) { const StronglyConnectedComponent &scc = cross_refs->Components [i]; - - // Count > 1 case: The SCC contains many objects which must be collected as one. - // Solution: Make all objects within the SCC directly or indirectly reference each other - if (scc.Count > 1) { - add_circular_references (scc); - } else if (scc.Count == 0) { - // Some SCCs might have no IGCUserPeers associated with them, so we must create one - temporary_peers [i] = env->NewObject (GCUserPeer_class, GCUserPeer_ctor); - } + prepare_scc_for_java_collection (i, scc); } // Add the cross scc refs @@ -63,11 +56,24 @@ void BridgeProcessing::prepare_for_java_collection () noexcept for (size_t i = 0; i < cross_refs->ComponentCount; i++) { const StronglyConnectedComponent &scc = cross_refs->Components [i]; for (size_t j = 0; j < scc.Count; j++) { - take_weak_global_ref (scc.Contexts [j]); + HandleContext *context = scc.Contexts [j]; + take_weak_global_ref (context); } } } +void BridgeProcessing::prepare_scc_for_java_collection (size_t scc_index, const StronglyConnectedComponent &scc) noexcept +{ + // Count > 1 case: The SCC contains many objects which must be collected as one. + // Solution: Make all objects within the SCC directly or indirectly reference each other + if (scc.Count > 1) { + add_circular_references (scc); + } else if (scc.Count == 0) { + // Some SCCs might have no IGCUserPeers associated with them, so we must create one + temporary_peers [scc_index] = env->NewObject (GCUserPeer_class, GCUserPeer_ctor); + } +} + CrossReferenceTarget BridgeProcessing::select_cross_reference_target (size_t scc_index) noexcept { const StronglyConnectedComponent &scc = cross_refs->Components [scc_index]; @@ -128,13 +134,14 @@ bool BridgeProcessing::add_reference (jobject from, jobject to) noexcept jmethodID add_method_id = env->GetMethodID (java_class, "monodroidAddReference", "(Ljava/lang/Object;)V"); env->DeleteLocalRef (java_class); - if (add_method_id != nullptr) { - env->CallVoidMethod (from, add_method_id, to); - return true; - } else { + if (add_method_id == nullptr) [[unlikely]] { env->ExceptionClear (); + log_missing_add_references_method (java_class); return false; } + + env->CallVoidMethod (from, add_method_id, to); + return true; } void BridgeProcessing::clear_references_if_needed (JniObjectReferenceControlBlock *control_block) noexcept @@ -147,67 +154,48 @@ void BridgeProcessing::clear_references_if_needed (JniObjectReferenceControlBloc return; } - // Clear references from the object - jclass java_class = env->GetObjectClass (control_block->handle); + clear_references (control_block->handle); + control_block->refs_added = 0; +} + +void BridgeProcessing::clear_references (jobject handle) noexcept +{ + abort_if_invalid_pointer_argument (handle, "handle"); + + jclass java_class = env->GetObjectClass (handle); jmethodID clear_method_id = env->GetMethodID (java_class, "monodroidClearReferences", "()V"); - env->DeleteLocalRef (java_class); // Clean up the local reference to the class + env->DeleteLocalRef (java_class); - if (clear_method_id != nullptr) [[likely]] { - env->CallVoidMethod (control_block->handle, clear_method_id); - control_block->refs_added = 0; - } else { - log_error (LOG_DEFAULT, "Failed to find monodroidClearReferences method"); + if (clear_method_id == nullptr) [[unlikely]] { env->ExceptionClear (); -#if DEBUG - if (Logger::gc_spew_enabled ()) { - char *class_name = Host::get_java_class_name_for_TypeManager (java_class); - log_error (LOG_GC, "Missing monodroidClearReferences method for object of class {}", class_name); - free (class_name); - } -#endif + log_missing_clear_references_method (java_class); + return; } + + env->CallVoidMethod (handle, clear_method_id); } void BridgeProcessing::take_global_ref (HandleContext *context) noexcept { abort_if_invalid_pointer_argument (context, "context"); - abort_unless (context->control_block != nullptr, "Control block must not be null"); // TODO: this sometimes throws -- looks like a race condition? - if (context->control_block->handle_type != JNIWeakGlobalRefType) [[unlikely]] { - log_error (LOG_DEFAULT, "Expected weak global reference type for handle, but got {} - handle: {:#x}", context->control_block->handle_type, reinterpret_cast (context->control_block->handle)); - return; - } + abort_unless (context->control_block != nullptr, "Control block must not be null"); abort_unless (context->control_block->handle_type == JNIWeakGlobalRefType, "Expected weak global reference type for handle"); jobject weak = context->control_block->handle; jobject handle = env->NewGlobalRef (weak); - - if (Logger::gref_log ()) [[unlikely]] { - OSBridge::_monodroid_gref_log ( - std::format ("take_global_ref wref={:#x} -> handle={:#x}\n"sv, - reinterpret_cast (weak), - reinterpret_cast (handle)).data ()); - } + log_weak_to_gref (weak, handle); if (handle != nullptr) { + log_weak_ref_survived (weak, handle); + context->control_block->handle = handle; context->control_block->handle_type = JNIGlobalRefType; - OSBridge::_monodroid_gref_log_new (weak, OSBridge::get_object_ref_type (env, weak), - handle, OSBridge::get_object_ref_type (env, handle), - "finalizer", gettid (), - " at [[clr-gc:take_global_ref]]", 0); - - OSBridge::_monodroid_weak_gref_delete (weak, OSBridge::get_object_ref_type (env, weak), - "finalizer", gettid (), " at [[clr-gc:take_global_ref]]", 0); env->DeleteWeakGlobalRef (weak); } else { // The native memory of the control block will be freed in managed code as well as the weak global ref context->control_block = nullptr; - - if (Logger::gc_spew_enabled ()) [[unlikely]] { - OSBridge::_monodroid_gref_log ( - std::format ("handle {:#x}/W; was collected by a Java GC"sv, reinterpret_cast (weak)).data ()); - } + log_weak_ref_collected (weak); } } @@ -218,20 +206,15 @@ void BridgeProcessing::take_weak_global_ref (HandleContext *context) noexcept abort_unless (context->control_block->handle_type == JNIGlobalRefType, "Expected global reference type for handle"); jobject handle = context->control_block->handle; - if (Logger::gref_log ()) [[unlikely]] { - OSBridge::_monodroid_gref_log (std::format ("take_weak_global_ref handle={:#x}\n"sv, reinterpret_cast (handle)).data ()); - } + log_take_weak_global_ref (handle); jobject weak = env->NewWeakGlobalRef (handle); - OSBridge::_monodroid_weak_gref_new (handle, OSBridge::get_object_ref_type (env, handle), - weak, OSBridge::get_object_ref_type (env, weak), - "finalizer", gettid (), " at [[clr-gc:take_weak_global_ref]]", 0); + log_weak_gref_new (handle, weak); context->control_block->handle = weak; context->control_block->handle_type = JNIWeakGlobalRefType; - OSBridge::_monodroid_gref_log_delete (handle, OSBridge::get_object_ref_type (env, handle), - "finalizer", gettid (), " at [[clr-gc:take_weak_global_ref]]", 0); + log_gref_delete (handle); env->DeleteGlobalRef (handle); } @@ -253,7 +236,7 @@ void BridgeProcessing::cleanup_after_java_collection () noexcept // clear the cross references on any remaining items for (size_t i = 0; i < cross_refs->ComponentCount; i++) { const StronglyConnectedComponent &scc = cross_refs->Components [i]; - [[maybe_unused]] bool is_alive = cleanup_strongly_connected_component (i, scc); + [[maybe_unused]] bool is_alive = cleanup_strongly_connected_component (scc); #if DEBUG total += scc.Count; @@ -268,40 +251,123 @@ void BridgeProcessing::cleanup_after_java_collection () noexcept #endif } -bool BridgeProcessing::cleanup_strongly_connected_component (size_t i, const StronglyConnectedComponent &scc) noexcept +bool BridgeProcessing::cleanup_strongly_connected_component (const StronglyConnectedComponent &scc) noexcept { // all contexts in the SCC must either be alive, or collected - bool is_alive = false; + int alive = 0; for (size_t j = 0; j < scc.Count; j++) { - abort_unless (scc.Contexts [j] != nullptr, "Context must not be null"); - JniObjectReferenceControlBlock *control_block = scc.Contexts [j]->control_block; - - if (control_block != nullptr) { - if (j > 0) { - abort_unless (is_alive, [&i] { return detail::_format_message ("Bridge SCC at index %d must be alive", i); }); - } - - is_alive = true; - clear_references_if_needed (control_block); - } else { - abort_unless (!is_alive, [&i] { return detail::_format_message ("Bridge SCC at index %d must NOT be alive", i); }); + HandleContext *context = scc.Contexts [j]; + abort_unless (context != nullptr, "Context must not be null"); + + bool is_alive = context->control_block != nullptr; + if (is_alive) { + alive++; + clear_references_if_needed (context->control_block); } } - return is_alive; + abort_unless (alive == 0 || alive == scc.Count, "All contexts in the SCC must be either collected or alive"); + return alive == scc.Count; } jobject CrossReferenceTarget::get_handle () const noexcept { - return is_temporary_peer ? temporary_peer : context->control_block->handle; + return is_temporary_peer + ? temporary_peer + : context->control_block->handle; } void CrossReferenceTarget::mark_refs_added_if_needed () noexcept { - if (!is_temporary_peer) { - abort_unless (context != nullptr, "Context must not be null"); - abort_unless (context->control_block != nullptr, "Control block must not be null"); - context->control_block->refs_added = 1; + if (is_temporary_peer) { + return; + } + + abort_unless (context != nullptr, "Context must not be null"); + abort_unless (context->control_block != nullptr, "Control block must not be null"); + context->control_block->refs_added = 1; +} + +[[gnu::always_inline]] +void BridgeProcessing::log_missing_add_references_method (jclass java_class) noexcept +{ + log_error (LOG_DEFAULT, "Failed to find monodroidAddReferences method"); +#if DEBUG + abort_if_invalid_pointer_argument (java_class, "java_class"); + if (Logger::gc_spew_enabled ()) [[unlikely]] { + char *class_name = Host::get_java_class_name_for_TypeManager (java_class); + log_error (LOG_GC, "Missing monodroidAddReferences method for object of class {}", class_name); + free (class_name); + } +#endif +} + +[[gnu::always_inline]] +void BridgeProcessing::log_missing_clear_references_method (jclass java_class) noexcept +{ + log_error (LOG_DEFAULT, "Failed to find monodroidClearReferences method"); +#if DEBUG + abort_if_invalid_pointer_argument (java_class, "java_class"); + if (Logger::gc_spew_enabled ()) [[unlikely]] { + char *class_name = Host::get_java_class_name_for_TypeManager (java_class); + log_error (LOG_GC, "Missing monodroidClearReferences method for object of class {}", class_name); + free (class_name); } +#endif +} + +[[gnu::always_inline]] +void BridgeProcessing::log_weak_to_gref (jobject weak, jobject handle) noexcept +{ + if (Logger::gref_log ()) [[unlikely]] { + OSBridge::_monodroid_gref_log ( + std::format ("take_global_ref wref={:#x} -> handle={:#x}\n"sv, + reinterpret_cast (weak), + reinterpret_cast (handle)).data ()); + } +} + +[[gnu::always_inline]] +void BridgeProcessing::log_weak_ref_survived (jobject weak, jobject handle) noexcept +{ + OSBridge::_monodroid_gref_log_new (weak, OSBridge::get_object_ref_type (env, weak), + handle, OSBridge::get_object_ref_type (env, handle), + "finalizer", gettid (), + " at [[clr-gc:take_global_ref]]", 0); + + OSBridge::_monodroid_weak_gref_delete (weak, OSBridge::get_object_ref_type (env, weak), + "finalizer", gettid (), " at [[clr-gc:take_global_ref]]", 0); +} + +[[gnu::always_inline]] +void BridgeProcessing::log_weak_ref_collected (jobject weak) noexcept +{ + if (Logger::gc_spew_enabled ()) [[unlikely]] { + OSBridge::_monodroid_gref_log ( + std::format ("handle {:#x}/W; was collected by a Java GC"sv, reinterpret_cast (weak)).data ()); + } +} + +[[gnu::always_inline]] +void BridgeProcessing::log_take_weak_global_ref (jobject handle) noexcept +{ + if (Logger::gref_log ()) [[unlikely]] { + OSBridge::_monodroid_gref_log (std::format ("take_weak_global_ref handle={:#x}\n"sv, reinterpret_cast (handle)).data ()); + } +} + +[[gnu::always_inline]] +void BridgeProcessing::log_weak_gref_new (jobject handle, jobject weak) noexcept +{ + OSBridge::_monodroid_weak_gref_new (handle, OSBridge::get_object_ref_type (env, handle), + weak, OSBridge::get_object_ref_type (env, weak), + "finalizer", gettid (), " at [[clr-gc:take_weak_global_ref]]", 0); +} + +[[gnu::always_inline]] +void BridgeProcessing::log_gref_delete (jobject handle) noexcept +{ + OSBridge::_monodroid_gref_log_delete (handle, OSBridge::get_object_ref_type (env, handle), + "finalizer", gettid (), " at [[clr-gc:take_weak_global_ref]]", 0); } diff --git a/src/native/clr/include/host/bridge-processing.hh b/src/native/clr/include/host/bridge-processing.hh index e02f8919cd0..1194ab0718c 100644 --- a/src/native/clr/include/host/bridge-processing.hh +++ b/src/native/clr/include/host/bridge-processing.hh @@ -35,15 +35,27 @@ private: static inline jmethodID GCUserPeer_ctor = nullptr; void prepare_for_java_collection () noexcept; - void cleanup_after_java_collection () noexcept; - bool cleanup_strongly_connected_component (size_t i, const StronglyConnectedComponent &scc) noexcept; - + void prepare_scc_for_java_collection (size_t scc_index, const StronglyConnectedComponent &scc) noexcept; void take_weak_global_ref (HandleContext *context) noexcept; - void take_global_ref (HandleContext *context) noexcept; void add_circular_references (const StronglyConnectedComponent &scc) noexcept; void add_cross_reference (size_t source_index, size_t dest_index) noexcept; CrossReferenceTarget select_cross_reference_target (size_t scc_index) noexcept; bool add_reference (jobject from, jobject to) noexcept; + + void cleanup_after_java_collection () noexcept; + bool cleanup_strongly_connected_component (const StronglyConnectedComponent &scc) noexcept; + void take_global_ref (HandleContext *context) noexcept; + void clear_references_if_needed (JniObjectReferenceControlBlock *control_block) noexcept; + void clear_references (jobject handle) noexcept; + + void log_missing_add_references_method (jclass java_class) noexcept; + void log_missing_clear_references_method (jclass java_class) noexcept; + void log_weak_to_gref (jobject weak, jobject handle) noexcept; + void log_weak_ref_survived (jobject weak, jobject handle) noexcept; + void log_weak_ref_collected (jobject weak) noexcept; + void log_take_weak_global_ref (jobject handle) noexcept; + void log_weak_gref_new (jobject handle, jobject weak) noexcept; + void log_gref_delete (jobject handle) noexcept; }; From c798a7fe2ee08c42a0d45c3f041a3d2de08b56f6 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 18 Jun 2025 12:19:20 +0200 Subject: [PATCH 08/32] Reduce nesting in gc-bridge.cc --- src/native/clr/host/gc-bridge.cc | 36 ++++++++++++++---------- src/native/clr/include/host/gc-bridge.hh | 1 + 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/native/clr/host/gc-bridge.cc b/src/native/clr/host/gc-bridge.cc index 993058a493c..7b190250aa1 100644 --- a/src/native/clr/host/gc-bridge.cc +++ b/src/native/clr/host/gc-bridge.cc @@ -96,21 +96,10 @@ void GCBridge::log_mark_cross_references_args_if_enabled (MarkCrossReferencesArg JNIEnv *env = OSBridge::ensure_jnienv (); for (size_t i = 0; i < args->ComponentCount; ++i) { - log_info (LOG_GC, "group {} with {} objects", i, args->Components [i].Count); - for (size_t j = 0; j < args->Components [i].Count; ++j) { - HandleContext *ctx = args->Components [i].Contexts [j]; - abort_unless (ctx != nullptr, "Context must not be null"); - abort_unless (ctx->control_block != nullptr, "Control block must not be null"); - - jobject handle = ctx->control_block->handle; - jclass java_class = env->GetObjectClass (handle); - if (java_class != nullptr) { - char *class_name = Host::get_java_class_name_for_TypeManager (java_class); - log_info (LOG_GC, "gref {:#x} [{}]", reinterpret_cast (handle), class_name); - free (class_name); - } else { - log_info (LOG_GC, "gref {:#x} [unknown class]", reinterpret_cast (handle)); - } + const StronglyConnectedComponent &scc = args->Components [i]; + log_info (LOG_GC, "group {} with {} objects", i, scc.Count); + for (size_t j = 0; j < scc.Count; ++j) { + log_handle_context (scc.Contexts [j]); } } @@ -124,3 +113,20 @@ void GCBridge::log_mark_cross_references_args_if_enabled (MarkCrossReferencesArg log_info_nocheck_fmt (LOG_GC, "xref [{}] {} -> {}", i, source_index, dest_index); } } + +[[gnu::always_inline]] +void GCBridge::log_handle_context (HandleContext *ctx) noexcept +{ + abort_unless (ctx != nullptr, "Context must not be null"); + abort_unless (ctx->control_block != nullptr, "Control block must not be null"); + + jobject handle = ctx->control_block->handle; + jclass java_class = env->GetObjectClass (handle); + if (java_class != nullptr) { + char *class_name = Host::get_java_class_name_for_TypeManager (java_class); + log_info (LOG_GC, "gref {:#x} [{}]", reinterpret_cast (handle), class_name); + free (class_name); + } else { + log_info (LOG_GC, "gref {:#x} [unknown class]", reinterpret_cast (handle)); + } +} diff --git a/src/native/clr/include/host/gc-bridge.hh b/src/native/clr/include/host/gc-bridge.hh index 46885c27fd6..21a2ff84032 100644 --- a/src/native/clr/include/host/gc-bridge.hh +++ b/src/native/clr/include/host/gc-bridge.hh @@ -90,5 +90,6 @@ namespace xamarin::android { static void mark_cross_references (MarkCrossReferencesArgs *cross_refs) noexcept; static void log_mark_cross_references_args_if_enabled (MarkCrossReferencesArgs *args) noexcept; + static void log_handle_context (HandleContext *ctx) noexcept; }; } From 333b1312daab6e57960272a719574861a42d33ae Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 18 Jun 2025 12:54:19 +0200 Subject: [PATCH 09/32] Reduce nesting and other code cleanups --- src/native/clr/host/bridge-processing.cc | 114 ++++++++++-------- src/native/clr/host/gc-bridge.cc | 4 +- .../clr/include/host/bridge-processing.hh | 6 +- src/native/clr/include/host/gc-bridge.hh | 15 ++- 4 files changed, 81 insertions(+), 58 deletions(-) diff --git a/src/native/clr/host/bridge-processing.cc b/src/native/clr/host/bridge-processing.cc index 9b54307d3f4..99fa416d0c1 100644 --- a/src/native/clr/host/bridge-processing.cc +++ b/src/native/clr/host/bridge-processing.cc @@ -29,6 +29,7 @@ void BridgeProcessing::process () noexcept prepare_for_java_collection (); GCBridge::trigger_java_gc (env); cleanup_after_java_collection (); + log_gc_summary (); } void BridgeProcessing::prepare_for_java_collection () noexcept @@ -64,14 +65,20 @@ void BridgeProcessing::prepare_for_java_collection () noexcept void BridgeProcessing::prepare_scc_for_java_collection (size_t scc_index, const StronglyConnectedComponent &scc) noexcept { - // Count > 1 case: The SCC contains many objects which must be collected as one. - // Solution: Make all objects within the SCC directly or indirectly reference each other - if (scc.Count > 1) { - add_circular_references (scc); - } else if (scc.Count == 0) { - // Some SCCs might have no IGCUserPeers associated with them, so we must create one + // Count == 0 case: Some SCCs might have no IGCUserPeers associated with them, so we must create one + if (scc.Count == 0) { temporary_peers [scc_index] = env->NewObject (GCUserPeer_class, GCUserPeer_ctor); + return; + } + + // Count == 1 case: The SCC contains a single object, there is no need to do anything special. + if (scc.Count == 1) { + return; } + + // Count > 1 case: The SCC contains many objects which must be collected as one. + // Solution: Make all objects within the SCC directly or indirectly reference each other + add_circular_references (scc); } CrossReferenceTarget BridgeProcessing::select_cross_reference_target (size_t scc_index) noexcept @@ -98,8 +105,6 @@ void BridgeProcessing::add_circular_references (const StronglyConnectedComponent JniObjectReferenceControlBlock *next = scc.Contexts [j]->control_block; bool reference_added = add_reference (prev->handle, next->handle); - // TODO this doesn't seem to be handled in the Mono code but if we don't handle this case, then the component _might not be_ strongly connected on the Java side. - // What is the case when this fails though? All bridge objects should have the `monodroidAddReference` method, so this is unexpected. abort_unless (reference_added, [this, &prev, &next] { jclass prev_java_class = env->GetObjectClass (prev->handle); const char *prev_class_name = Host::get_java_class_name_for_TypeManager (prev_java_class); @@ -112,7 +117,7 @@ void BridgeProcessing::add_circular_references (const StronglyConnectedComponent prev_class_name, next_class_name); }); - + prev->refs_added = 1; prev = next; } @@ -144,9 +149,17 @@ bool BridgeProcessing::add_reference (jobject from, jobject to) noexcept return true; } -void BridgeProcessing::clear_references_if_needed (JniObjectReferenceControlBlock *control_block) noexcept +void BridgeProcessing::clear_references_if_needed (HandleContext *context) noexcept { - abort_if_invalid_pointer_argument (control_block, "control_block"); + abort_if_invalid_pointer_argument (context, "context"); + + if (context->is_collected ()) { + return; + } + + JniObjectReferenceControlBlock *control_block = context->control_block; + + abort_unless (control_block != nullptr, "Control block must not be null"); abort_unless (control_block->handle != nullptr, "Control block handle must not be null"); abort_unless (control_block->handle_type == JNIGlobalRefType, "Control block handle type must be global reference"); @@ -220,55 +233,34 @@ void BridgeProcessing::take_weak_global_ref (HandleContext *context) noexcept void BridgeProcessing::cleanup_after_java_collection () noexcept { -#if DEBUG - int total = 0; - int alive = 0; -#endif - - // try to switch back to global refs to analyze what stayed alive for (size_t i = 0; i < cross_refs->ComponentCount; i++) { const StronglyConnectedComponent &scc = cross_refs->Components [i]; + + // try to switch back to global refs to analyze what stayed alive for (size_t j = 0; j < scc.Count; j++) { - take_global_ref (scc.Contexts [j]); + HandleContext *context = scc.Contexts [j]; + take_global_ref (context); + clear_references_if_needed (context); } - } - - // clear the cross references on any remaining items - for (size_t i = 0; i < cross_refs->ComponentCount; i++) { - const StronglyConnectedComponent &scc = cross_refs->Components [i]; - [[maybe_unused]] bool is_alive = cleanup_strongly_connected_component (scc); -#if DEBUG - total += scc.Count; - if (is_alive) { - alive += scc.Count; - } -#endif + abort_unless_all_collected_or_all_alive (scc); } - -#if DEBUG - log_info (LOG_GC, "GC cleanup summary: {} objects tested - resurrecting {}.", total, alive); -#endif } -bool BridgeProcessing::cleanup_strongly_connected_component (const StronglyConnectedComponent &scc) noexcept +void BridgeProcessing::abort_unless_all_collected_or_all_alive (const StronglyConnectedComponent &scc) noexcept { - // all contexts in the SCC must either be alive, or collected - int alive = 0; + if (scc.Count == 0) { + return; + } - for (size_t j = 0; j < scc.Count; j++) { + abort_unless (scc.Contexts [0] != nullptr, "Context must not be null"); + bool is_collected = scc.Contexts [0]->is_collected (); + + for (size_t j = 1; j < scc.Count; j++) { HandleContext *context = scc.Contexts [j]; abort_unless (context != nullptr, "Context must not be null"); - - bool is_alive = context->control_block != nullptr; - if (is_alive) { - alive++; - clear_references_if_needed (context->control_block); - } + abort_unless (context->is_collected () == is_collected, "Cannot have a mix of collected and alive contexts in the SCC"); } - - abort_unless (alive == 0 || alive == scc.Count, "All contexts in the SCC must be either collected or alive"); - return alive == scc.Count; } jobject CrossReferenceTarget::get_handle () const noexcept @@ -290,7 +282,7 @@ void CrossReferenceTarget::mark_refs_added_if_needed () noexcept } [[gnu::always_inline]] -void BridgeProcessing::log_missing_add_references_method (jclass java_class) noexcept +void BridgeProcessing::log_missing_add_references_method ([[maybe_unused]] jclass java_class) noexcept { log_error (LOG_DEFAULT, "Failed to find monodroidAddReferences method"); #if DEBUG @@ -304,7 +296,7 @@ void BridgeProcessing::log_missing_add_references_method (jclass java_class) noe } [[gnu::always_inline]] -void BridgeProcessing::log_missing_clear_references_method (jclass java_class) noexcept +void BridgeProcessing::log_missing_clear_references_method ([[maybe_unused]] jclass java_class) noexcept { log_error (LOG_DEFAULT, "Failed to find monodroidClearReferences method"); #if DEBUG @@ -371,3 +363,27 @@ void BridgeProcessing::log_gref_delete (jobject handle) noexcept OSBridge::_monodroid_gref_log_delete (handle, OSBridge::get_object_ref_type (env, handle), "finalizer", gettid (), " at [[clr-gc:take_weak_global_ref]]", 0); } + +[[gnu::always_inline]] +void BridgeProcessing::log_gc_summary () noexcept +{ +#if DEBUG + int total = 0; + int alive = 0; + + for (size_t i = 0; i < cross_refs->ComponentCount; i++) { + const StronglyConnectedComponent &scc = cross_refs->Components [i]; + + for (size_t j = 0; j < scc.Count; j++) { + HandleContext *context = scc.Contexts [j]; + + total++; + if (!context->is_collected ()) { + alive++; + } + } + } + + log_info (LOG_GC, "GC cleanup summary: {} objects tested - resurrecting {}.", total, alive); +#endif // DEBUG +} diff --git a/src/native/clr/host/gc-bridge.cc b/src/native/clr/host/gc-bridge.cc index 7b190250aa1..e9bae014890 100644 --- a/src/native/clr/host/gc-bridge.cc +++ b/src/native/clr/host/gc-bridge.cc @@ -99,7 +99,7 @@ void GCBridge::log_mark_cross_references_args_if_enabled (MarkCrossReferencesArg const StronglyConnectedComponent &scc = args->Components [i]; log_info (LOG_GC, "group {} with {} objects", i, scc.Count); for (size_t j = 0; j < scc.Count; ++j) { - log_handle_context (scc.Contexts [j]); + log_handle_context (env, scc.Contexts [j]); } } @@ -115,7 +115,7 @@ void GCBridge::log_mark_cross_references_args_if_enabled (MarkCrossReferencesArg } [[gnu::always_inline]] -void GCBridge::log_handle_context (HandleContext *ctx) noexcept +void GCBridge::log_handle_context (JNIEnv *env, HandleContext *ctx) noexcept { abort_unless (ctx != nullptr, "Context must not be null"); abort_unless (ctx->control_block != nullptr, "Control block must not be null"); diff --git a/src/native/clr/include/host/bridge-processing.hh b/src/native/clr/include/host/bridge-processing.hh index 1194ab0718c..3b66b9590a7 100644 --- a/src/native/clr/include/host/bridge-processing.hh +++ b/src/native/clr/include/host/bridge-processing.hh @@ -44,10 +44,11 @@ private: bool add_reference (jobject from, jobject to) noexcept; void cleanup_after_java_collection () noexcept; - bool cleanup_strongly_connected_component (const StronglyConnectedComponent &scc) noexcept; + void cleanup_scc_for_java_collection (const StronglyConnectedComponent &scc) noexcept; + void abort_unless_all_collected_or_all_alive (const StronglyConnectedComponent &scc) noexcept; void take_global_ref (HandleContext *context) noexcept; - void clear_references_if_needed (JniObjectReferenceControlBlock *control_block) noexcept; + void clear_references_if_needed (HandleContext *context) noexcept; void clear_references (jobject handle) noexcept; void log_missing_add_references_method (jclass java_class) noexcept; @@ -58,4 +59,5 @@ private: void log_take_weak_global_ref (jobject handle) noexcept; void log_weak_gref_new (jobject handle, jobject weak) noexcept; void log_gref_delete (jobject handle) noexcept; + void log_gc_summary () noexcept; }; diff --git a/src/native/clr/include/host/gc-bridge.hh b/src/native/clr/include/host/gc-bridge.hh index 21a2ff84032..2bf34cda362 100644 --- a/src/native/clr/include/host/gc-bridge.hh +++ b/src/native/clr/include/host/gc-bridge.hh @@ -18,13 +18,18 @@ struct JniObjectReferenceControlBlock struct HandleContext { - JniObjectReferenceControlBlock* control_block; + JniObjectReferenceControlBlock *control_block; + + bool is_collected () const noexcept + { + return control_block == nullptr; + } }; struct StronglyConnectedComponent { size_t Count; - HandleContext** Contexts; + HandleContext **Contexts; }; struct ComponentCrossReference @@ -36,9 +41,9 @@ struct ComponentCrossReference struct MarkCrossReferencesArgs { size_t ComponentCount; - StronglyConnectedComponent* Components; + StronglyConnectedComponent *Components; size_t CrossReferenceCount; - ComponentCrossReference* CrossReferences; + ComponentCrossReference *CrossReferences; }; using BridgeProcessingStartedFtn = void (*)(); @@ -90,6 +95,6 @@ namespace xamarin::android { static void mark_cross_references (MarkCrossReferencesArgs *cross_refs) noexcept; static void log_mark_cross_references_args_if_enabled (MarkCrossReferencesArgs *args) noexcept; - static void log_handle_context (HandleContext *ctx) noexcept; + static void log_handle_context (JNIEnv *env, HandleContext *ctx) noexcept; }; } From be2ed7787baaffbc550dca4d1c53aab375be2cf2 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 18 Jun 2025 13:20:45 +0200 Subject: [PATCH 10/32] Address feedback --- src/native/clr/host/bridge-processing.cc | 109 +++++++++++++++-------- 1 file changed, 73 insertions(+), 36 deletions(-) diff --git a/src/native/clr/host/bridge-processing.cc b/src/native/clr/host/bridge-processing.cc index 99fa416d0c1..4deef3e68d4 100644 --- a/src/native/clr/host/bridge-processing.cc +++ b/src/native/clr/host/bridge-processing.cc @@ -2,6 +2,7 @@ #include #include #include +#include #include @@ -22,6 +23,17 @@ BridgeProcessing::BridgeProcessing (MarkCrossReferencesArgs *args) noexcept : env{ OSBridge::ensure_jnienv () }, cross_refs{ args } { + if (args == nullptr) [[unlikely]] { + Helpers::abort_application (LOG_GC, "Cross references argument is a NULL pointer"sv); + } + + if (args->ComponentCount > 0 && args->Components == nullptr) [[unlikely]] { + Helpers::abort_application (LOG_GC, "Components member of the cross references arguments structure is NULL"sv); + } + + if (args->CrossReferenceCount > 0 && args->CrossReferences == nullptr) [[unlikely]] { + Helpers::abort_application (LOG_GC, "CrossReferences member of the cross references arguments structure is NULL"sv); + } } void BridgeProcessing::process () noexcept @@ -84,27 +96,34 @@ void BridgeProcessing::prepare_scc_for_java_collection (size_t scc_index, const CrossReferenceTarget BridgeProcessing::select_cross_reference_target (size_t scc_index) noexcept { const StronglyConnectedComponent &scc = cross_refs->Components [scc_index]; - if (scc.Count > 0) { - abort_unless (scc.Contexts [0] != nullptr, "SCC must have at least one context"); - abort_unless (scc.Contexts [0]->control_block != nullptr, "SCC must have at least one context with valid control block"); - return { .is_temporary_peer = false, .context = scc.Contexts [0] }; - } else { + + if (scc.Count == 0) { const auto temporary_peer = temporary_peers.find (scc_index); abort_unless (temporary_peer != temporary_peers.end(), "Temporary peer must be found in the map"); return { .is_temporary_peer = true, .temporary_peer = temporary_peer->second }; } + + abort_unless (scc.Contexts [0] != nullptr, "SCC must have at least one context"); + return { .is_temporary_peer = false, .context = scc.Contexts [0] }; } +// caller must ensure that scc.Count > 1 void BridgeProcessing::add_circular_references (const StronglyConnectedComponent &scc) noexcept { - abort_unless (scc.Count > 1, "SCC must have at least two items to add inner references"); + auto get_control_block = [&scc](size_t index) -> JniObjectReferenceControlBlock* { + abort_unless (scc.Contexts [index] != nullptr, "Context in SCC must not be null"); + JniObjectReferenceControlBlock *control_block = scc.Contexts [index]->control_block; + abort_unless (control_block != nullptr, "Control block in SCC must not be null"); + return control_block; + }; - JniObjectReferenceControlBlock *prev = scc.Contexts [scc.Count - 1]->control_block; + JniObjectReferenceControlBlock *prev = get_control_block (scc.Count - 1); for (size_t j = 1; j < scc.Count; j++) { - JniObjectReferenceControlBlock *next = scc.Contexts [j]->control_block; + JniObjectReferenceControlBlock *next = get_control_block (j); + bool reference_added = add_reference (prev->handle, next->handle); - + abort_unless (reference_added, [this, &prev, &next] { jclass prev_java_class = env->GetObjectClass (prev->handle); const char *prev_class_name = Host::get_java_class_name_for_TypeManager (prev_java_class); @@ -135,6 +154,9 @@ void BridgeProcessing::add_cross_reference (size_t source_index, size_t dest_ind bool BridgeProcessing::add_reference (jobject from, jobject to) noexcept { + abort_if_invalid_pointer_argument (from, "from"); + abort_if_invalid_pointer_argument (to, "to"); + jclass java_class = env->GetObjectClass (from); jmethodID add_method_id = env->GetMethodID (java_class, "monodroidAddReference", "(Ljava/lang/Object;)V"); env->DeleteLocalRef (java_class); @@ -151,7 +173,7 @@ bool BridgeProcessing::add_reference (jobject from, jobject to) noexcept void BridgeProcessing::clear_references_if_needed (HandleContext *context) noexcept { - abort_if_invalid_pointer_argument (context, "context"); + // context is a valid pointer - validated at callsite if (context->is_collected ()) { return; @@ -190,7 +212,7 @@ void BridgeProcessing::clear_references (jobject handle) noexcept void BridgeProcessing::take_global_ref (HandleContext *context) noexcept { - abort_if_invalid_pointer_argument (context, "context"); + // context is a valid pointer - validated at callsite abort_unless (context->control_block != nullptr, "Control block must not be null"); abort_unless (context->control_block->handle_type == JNIWeakGlobalRefType, "Expected weak global reference type for handle"); @@ -239,6 +261,8 @@ void BridgeProcessing::cleanup_after_java_collection () noexcept // try to switch back to global refs to analyze what stayed alive for (size_t j = 0; j < scc.Count; j++) { HandleContext *context = scc.Contexts [j]; + abort_unless (context != nullptr, "Context must not be null"); + take_global_ref (context); clear_references_if_needed (context); } @@ -287,11 +311,13 @@ void BridgeProcessing::log_missing_add_references_method ([[maybe_unused]] jclas log_error (LOG_DEFAULT, "Failed to find monodroidAddReferences method"); #if DEBUG abort_if_invalid_pointer_argument (java_class, "java_class"); - if (Logger::gc_spew_enabled ()) [[unlikely]] { - char *class_name = Host::get_java_class_name_for_TypeManager (java_class); - log_error (LOG_GC, "Missing monodroidAddReferences method for object of class {}", class_name); - free (class_name); + if (!Logger::gc_spew_enabled ()) [[likely]] { + return; } + + char *class_name = Host::get_java_class_name_for_TypeManager (java_class); + log_error (LOG_GC, "Missing monodroidAddReferences method for object of class {}", optional_string (class_name)); + free (class_name); #endif } @@ -301,23 +327,27 @@ void BridgeProcessing::log_missing_clear_references_method ([[maybe_unused]] jcl log_error (LOG_DEFAULT, "Failed to find monodroidClearReferences method"); #if DEBUG abort_if_invalid_pointer_argument (java_class, "java_class"); - if (Logger::gc_spew_enabled ()) [[unlikely]] { - char *class_name = Host::get_java_class_name_for_TypeManager (java_class); - log_error (LOG_GC, "Missing monodroidClearReferences method for object of class {}", class_name); - free (class_name); + if (!Logger::gc_spew_enabled ()) [[likely]] { + return; } + + char *class_name = Host::get_java_class_name_for_TypeManager (java_class); + log_error (LOG_GC, "Missing monodroidClearReferences method for object of class {}", optional_string (class_name)); + free (class_name); #endif } [[gnu::always_inline]] void BridgeProcessing::log_weak_to_gref (jobject weak, jobject handle) noexcept { - if (Logger::gref_log ()) [[unlikely]] { - OSBridge::_monodroid_gref_log ( - std::format ("take_global_ref wref={:#x} -> handle={:#x}\n"sv, - reinterpret_cast (weak), - reinterpret_cast (handle)).data ()); + if (!Logger::gref_log ()) [[likely]] { + return; } + + OSBridge::_monodroid_gref_log ( + std::format ("take_global_ref wref={:#x} -> handle={:#x}\n"sv, + reinterpret_cast (weak), + reinterpret_cast (handle)).data ()); } [[gnu::always_inline]] @@ -335,18 +365,22 @@ void BridgeProcessing::log_weak_ref_survived (jobject weak, jobject handle) noex [[gnu::always_inline]] void BridgeProcessing::log_weak_ref_collected (jobject weak) noexcept { - if (Logger::gc_spew_enabled ()) [[unlikely]] { - OSBridge::_monodroid_gref_log ( - std::format ("handle {:#x}/W; was collected by a Java GC"sv, reinterpret_cast (weak)).data ()); + if (Logger::gc_spew_enabled ()) [[likely]] { + return; } + + OSBridge::_monodroid_gref_log ( + std::format ("handle {:#x}/W; was collected by a Java GC"sv, reinterpret_cast (weak)).data ()); } [[gnu::always_inline]] void BridgeProcessing::log_take_weak_global_ref (jobject handle) noexcept { - if (Logger::gref_log ()) [[unlikely]] { - OSBridge::_monodroid_gref_log (std::format ("take_weak_global_ref handle={:#x}\n"sv, reinterpret_cast (handle)).data ()); + if (!Logger::gref_log ()) [[likely]] { + return; } + + OSBridge::_monodroid_gref_log (std::format ("take_weak_global_ref handle={:#x}\n"sv, reinterpret_cast (handle)).data ()); } [[gnu::always_inline]] @@ -367,23 +401,26 @@ void BridgeProcessing::log_gref_delete (jobject handle) noexcept [[gnu::always_inline]] void BridgeProcessing::log_gc_summary () noexcept { -#if DEBUG - int total = 0; - int alive = 0; - + if (!Logger::gc_spew_enabled ()) [[likely]] { + return; + } + + size_t total = 0; + size_t alive = 0; + for (size_t i = 0; i < cross_refs->ComponentCount; i++) { const StronglyConnectedComponent &scc = cross_refs->Components [i]; for (size_t j = 0; j < scc.Count; j++) { HandleContext *context = scc.Contexts [j]; + abort_unless (context != nullptr, "Context must not be null"); - total++; + total = Helpers::add_with_overflow_check (total, 1); if (!context->is_collected ()) { - alive++; + alive = Helpers::add_with_overflow_check (alive, 1); } } } log_info (LOG_GC, "GC cleanup summary: {} objects tested - resurrecting {}.", total, alive); -#endif // DEBUG } From f54d58033f716bd503a3330c79307cfac7178b27 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 18 Jun 2025 13:28:14 +0200 Subject: [PATCH 11/32] Thanks copilot --- src/native/clr/host/bridge-processing.cc | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/native/clr/host/bridge-processing.cc b/src/native/clr/host/bridge-processing.cc index 4deef3e68d4..4f615b63923 100644 --- a/src/native/clr/host/bridge-processing.cc +++ b/src/native/clr/host/bridge-processing.cc @@ -70,6 +70,7 @@ void BridgeProcessing::prepare_for_java_collection () noexcept const StronglyConnectedComponent &scc = cross_refs->Components [i]; for (size_t j = 0; j < scc.Count; j++) { HandleContext *context = scc.Contexts [j]; + abort_unless (context != nullptr, "Context must not be null"); take_weak_global_ref (context); } } @@ -159,14 +160,15 @@ bool BridgeProcessing::add_reference (jobject from, jobject to) noexcept jclass java_class = env->GetObjectClass (from); jmethodID add_method_id = env->GetMethodID (java_class, "monodroidAddReference", "(Ljava/lang/Object;)V"); - env->DeleteLocalRef (java_class); if (add_method_id == nullptr) [[unlikely]] { env->ExceptionClear (); log_missing_add_references_method (java_class); + env->DeleteLocalRef (java_class); return false; } + env->DeleteLocalRef (java_class); env->CallVoidMethod (from, add_method_id, to); return true; } @@ -199,14 +201,15 @@ void BridgeProcessing::clear_references (jobject handle) noexcept jclass java_class = env->GetObjectClass (handle); jmethodID clear_method_id = env->GetMethodID (java_class, "monodroidClearReferences", "()V"); - env->DeleteLocalRef (java_class); if (clear_method_id == nullptr) [[unlikely]] { env->ExceptionClear (); log_missing_clear_references_method (java_class); + env->DeleteLocalRef (java_class); return; } + env->DeleteLocalRef (java_class); env->CallVoidMethod (handle, clear_method_id); } @@ -236,7 +239,7 @@ void BridgeProcessing::take_global_ref (HandleContext *context) noexcept void BridgeProcessing::take_weak_global_ref (HandleContext *context) noexcept { - abort_if_invalid_pointer_argument (context, "context"); + // context is a valid pointer - validated at callsite abort_unless (context->control_block != nullptr, "Control block must not be null"); abort_unless (context->control_block->handle_type == JNIGlobalRefType, "Expected global reference type for handle"); @@ -289,9 +292,13 @@ void BridgeProcessing::abort_unless_all_collected_or_all_alive (const StronglyCo jobject CrossReferenceTarget::get_handle () const noexcept { - return is_temporary_peer - ? temporary_peer - : context->control_block->handle; + if (is_temporary_peer) { + return temporary_peer; + } + + abort_unless (context != nullptr, "Context must not be null"); + abort_unless (context->control_block != nullptr, "Control block must not be null"); + return context->control_block->handle; } void CrossReferenceTarget::mark_refs_added_if_needed () noexcept From 74f843d88c867caa800d2c1a5931dc807e493e77 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 19 Jun 2025 12:17:48 +0200 Subject: [PATCH 12/32] Fix deadlocks --- .../Android.Runtime/AndroidRuntime.cs | 4 +- .../Android.Runtime/AndroidRuntimeInternal.cs | 4 +- .../ManagedValueManager.cs | 114 ++++++++++++------ src/native/clr/host/bridge-processing.cc | 2 - src/native/clr/host/gc-bridge.cc | 26 ++-- src/native/clr/host/internal-pinvokes.cc | 3 +- src/native/clr/include/host/gc-bridge.hh | 14 +-- 7 files changed, 100 insertions(+), 67 deletions(-) diff --git a/src/Mono.Android/Android.Runtime/AndroidRuntime.cs b/src/Mono.Android/Android.Runtime/AndroidRuntime.cs index f6a1ee4e3c3..52691aa18f4 100644 --- a/src/Mono.Android/Android.Runtime/AndroidRuntime.cs +++ b/src/Mono.Android/Android.Runtime/AndroidRuntime.cs @@ -641,7 +641,9 @@ class AndroidValueManager : JniRuntime.JniValueManager { public override void WaitForGCBridgeProcessing () { - AndroidRuntimeInternal.WaitForBridgeProcessing (); + if (!AndroidRuntimeInternal.BridgeProcessing) + return; + RuntimeNativeMethods._monodroid_gc_wait_for_bridge_processing (); } public override IJavaPeerable? CreatePeer ( diff --git a/src/Mono.Android/Android.Runtime/AndroidRuntimeInternal.cs b/src/Mono.Android/Android.Runtime/AndroidRuntimeInternal.cs index 31d80445485..c8d3452af40 100644 --- a/src/Mono.Android/Android.Runtime/AndroidRuntimeInternal.cs +++ b/src/Mono.Android/Android.Runtime/AndroidRuntimeInternal.cs @@ -64,9 +64,7 @@ static void MonoUnhandledException (Exception ex) public static void WaitForBridgeProcessing () { - if (!BridgeProcessing) - return; - RuntimeNativeMethods._monodroid_gc_wait_for_bridge_processing (); + Java.Interop.JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); } } } diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index 0f01a47facd..ee25ea614ba 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -1,5 +1,6 @@ // Originally from: https://github.com/dotnet/java-interop/blob/9b1d8781e8e322849d05efac32119c913b21c192/src/Java.Runtime.Environment/Java.Interop/ManagedValueManager.cs using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; @@ -22,6 +23,9 @@ class ManagedValueManager : JniRuntime.JniValueManager const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors; Dictionary>? RegisteredInstances = new (); + readonly ConcurrentQueue CollectedContexts = new (); + + static readonly SemaphoreSlim bridgeProcessingSemaphore = new (1, 1); static Lazy s_instance = new (() => new ManagedValueManager ()); public static ManagedValueManager GetOrCreateInstance () => s_instance.Value; @@ -35,12 +39,42 @@ unsafe ManagedValueManager () public override void WaitForGCBridgeProcessing () { - AndroidRuntimeInternal.WaitForBridgeProcessing (); + bridgeProcessingSemaphore.Wait (); + bridgeProcessingSemaphore.Release (); } - public override void CollectPeers () + public unsafe override void CollectPeers () { - GC.Collect (); + if (RegisteredInstances == null) + throw new ObjectDisposedException (nameof (ManagedValueManager)); + + while (CollectedContexts.TryDequeue (out IntPtr contextPtr)) { + HandleContext* context = (HandleContext*)contextPtr; + + lock (RegisteredInstances) { + Remove (context); + } + + HandleContext.Free (ref context); + } + + void Remove (HandleContext* context) + { + int key = context->PeerIdentityHashCode; + if (!RegisteredInstances.TryGetValue (key, out List? peers)) + return; + + for (int i = peers.Count - 1; i >= 0; i--) { + var peer = peers [i]; + if (peer.BelongsToContext (context)) { + peers.RemoveAt (i); + } + } + + if (peers.Count == 0) { + RegisteredInstances.Remove (key); + } + } } public override void AddPeer (IJavaPeerable value) @@ -48,6 +82,9 @@ public override void AddPeer (IJavaPeerable value) if (RegisteredInstances == null) throw new ObjectDisposedException (nameof (ManagedValueManager)); + // Remove any collected contexts before adding a new peer. + CollectPeers (); + var r = value.PeerReference; if (!r.IsValid) throw new ObjectDisposedException (value.GetType ().FullName); @@ -134,31 +171,29 @@ public override void RemovePeer (IJavaPeerable value) if (RegisteredInstances == null) throw new ObjectDisposedException (nameof (ManagedValueManager)); + // Remove any collected contexts before modifying RegisteredInstances + CollectPeers (); + if (value == null) throw new ArgumentNullException (nameof (value)); lock (RegisteredInstances) { - RemoveRegisteredInstance (value, disposeReferenceTrackingHandle: true); - } - } - - private void RemoveRegisteredInstance (IJavaPeerable target, bool disposeReferenceTrackingHandle) - { - int key = target.JniIdentityHashCode; - if (!RegisteredInstances.TryGetValue (key, out List? peers)) - return; + int key = value.JniIdentityHashCode; + if (!RegisteredInstances.TryGetValue (key, out List? peers)) + return; - for (int i = peers.Count - 1; i >= 0; i--) { - var peer = peers [i]; - if (object.ReferenceEquals (target, peer.Target)) { - peers.RemoveAt (i); - if (disposeReferenceTrackingHandle) { + for (int i = peers.Count - 1; i >= 0; i--) { + var peer = peers [i]; + var target = peer.Target; + if (object.ReferenceEquals (value, target)) { + peers.RemoveAt (i); peer.Dispose (); + GC.KeepAlive (target); } } + if (peers.Count == 0) + RegisteredInstances.Remove (key); } - if (peers.Count == 0) - RegisteredInstances.Remove (key); } public override void FinalizePeer (IJavaPeerable value) @@ -236,6 +271,9 @@ public override List GetSurfacedPeers () if (RegisteredInstances == null) throw new ObjectDisposedException (nameof (ManagedValueManager)); + // Remove any collected contexts before iterating over all the registered instances + CollectPeers (); + lock (RegisteredInstances) { var peers = new List (RegisteredInstances.Count); foreach (var e in RegisteredInstances) { @@ -254,13 +292,17 @@ unsafe struct ReferenceTrackingHandle : IDisposable private WeakReference _weakReference; private HandleContext* _context; + public bool BelongsToContext (HandleContext* context) + => _context == context; + public ReferenceTrackingHandle (IJavaPeerable peer) { _context = HandleContext.Alloc (peer); _weakReference = new WeakReference (peer); } - public IJavaPeerable? Target => _weakReference.TryGetTarget (out var target) ? target : null; + public IJavaPeerable? Target + => _weakReference.TryGetTarget (out var target) ? target : null; public void Dispose () { @@ -287,14 +329,14 @@ unsafe struct HandleContext static readonly nuint Size = (nuint)Marshal.SizeOf (); static readonly Dictionary referenceTrackingHandles = new (); + public int PeerIdentityHashCode; IntPtr controlBlock; public bool IsCollected => controlBlock == IntPtr.Zero; public static GCHandle GetAssociatedGCHandle (HandleContext* context) { - lock (referenceTrackingHandles) - { + lock (referenceTrackingHandles) { if (!referenceTrackingHandles.TryGetValue ((IntPtr)context, out GCHandle handle)) { throw new InvalidOperationException ("Unknown reference tracking handle."); } @@ -310,6 +352,7 @@ public static GCHandle GetAssociatedGCHandle (HandleContext* context) throw new OutOfMemoryException ("Failed to allocate memory for HandleContext."); } + context->PeerIdentityHashCode = peer.JniIdentityHashCode; context->controlBlock = peer.JniObjectReferenceControlBlock; GCHandle handle = JavaMarshal.CreateReferenceTrackingHandle (peer, context); @@ -332,7 +375,7 @@ public static void Free (ref HandleContext* context) [UnmanagedCallersOnly] static void BridgeProcessingStarted () { - AndroidRuntimeInternal.BridgeProcessing = true; + bridgeProcessingSemaphore.Wait (); } [UnmanagedCallersOnly] @@ -343,7 +386,7 @@ static unsafe void BridgeProcessingFinished (MarkCrossReferencesArgs* mcr) ReadOnlySpan handlesToFree = ProcessCollectedContexts (mcr); JavaMarshal.FinishCrossReferenceProcessing (mcr, handlesToFree); - AndroidRuntimeInternal.BridgeProcessing = false; + bridgeProcessingSemaphore.Release (); } static unsafe ReadOnlySpan ProcessCollectedContexts (MarkCrossReferencesArgs* mcr) @@ -351,14 +394,8 @@ static unsafe ReadOnlySpan ProcessCollectedContexts (MarkCrossReferenc List handlesToFree = []; ManagedValueManager instance = GetOrCreateInstance (); - lock (instance.RegisteredInstances) { - for (int i = 0; (nuint)i < mcr->ComponentCount; i++) { - ProcessComponent (mcr->Components [i]); - } - } - - void ProcessComponent (StronglyConnectedComponent component) - { + for (int i = 0; (nuint)i < mcr->ComponentCount; i++) { + StronglyConnectedComponent component = mcr->Components [i]; for (int j = 0; (nuint)j < component.Count; j++) { ProcessContext ((HandleContext*)component.Contexts [j]); } @@ -368,16 +405,19 @@ void ProcessContext (HandleContext* context) { Trace.Assert (context != null, "Context should never be null."); - // ignore contexts which were not collected + // Ignore contexts which were not collected if (!context->IsCollected) { return; } - // ignore contexts which were not allocated by the ManagedValueManager GCHandle handle = HandleContext.GetAssociatedGCHandle (context); - IJavaPeerable peer = (IJavaPeerable)handle.Target; - instance.RemoveRegisteredInstance (peer, disposeReferenceTrackingHandle: false); - HandleContext.Free (ref context); + + // Note: modifying the RegisteredInstances dictionary while processing the collected contexts + // is tricky and can lead to deadlocks, so we remember which contexts were collected and we will free + // them later outside of the bridge processing loop. + instance.CollectedContexts.Enqueue ((IntPtr)context); + + // important: we must not free the handle before passing it to JavaMarshal.FinishCrossReferenceProcessing handlesToFree.Add (handle); } diff --git a/src/native/clr/host/bridge-processing.cc b/src/native/clr/host/bridge-processing.cc index 4f615b63923..6fee7e10f07 100644 --- a/src/native/clr/host/bridge-processing.cc +++ b/src/native/clr/host/bridge-processing.cc @@ -4,8 +4,6 @@ #include #include -#include - using namespace xamarin::android; void BridgeProcessing::initialize_on_runtime_init (JNIEnv *env, jclass runtimeClass) noexcept diff --git a/src/native/clr/host/gc-bridge.cc b/src/native/clr/host/gc-bridge.cc index e9bae014890..9f310bebebf 100644 --- a/src/native/clr/host/gc-bridge.cc +++ b/src/native/clr/host/gc-bridge.cc @@ -36,8 +36,9 @@ void GCBridge::initialize_on_runtime_init (JNIEnv *env, jclass runtimeClass) noe void GCBridge::trigger_java_gc (JNIEnv *env) noexcept { - env->CallVoidMethod (Runtime_instance, Runtime_gc); + abort_if_invalid_pointer_argument (env, "env"); + env->CallVoidMethod (Runtime_instance, Runtime_gc); if (!env->ExceptionCheck ()) [[likely]] { return; } @@ -49,16 +50,13 @@ void GCBridge::trigger_java_gc (JNIEnv *env) noexcept void GCBridge::mark_cross_references (MarkCrossReferencesArgs *args) noexcept { - abort_if_invalid_pointer_argument (args, "cross_refs"); + abort_if_invalid_pointer_argument (args, "args"); abort_unless (args->Components != nullptr || args->ComponentCount == 0, "Components must not be null if ComponentCount is greater than 0"); abort_unless (args->CrossReferences != nullptr || args->CrossReferenceCount == 0, "CrossReferences must not be null if CrossReferenceCount is greater than 0"); log_mark_cross_references_args_if_enabled (args); - std::unique_lock lock (processing_mutex); - - cross_refs = args; - - bridge_processing_semaphore.release (); + shared_args.store (args); + shared_args_semaphore.release (); } void GCBridge::bridge_processing () noexcept @@ -67,23 +65,19 @@ void GCBridge::bridge_processing () noexcept abort_unless (bridge_processing_finished_callback != nullptr, "GC bridge processing finished callback is not set"); while (true) { - bridge_processing_semaphore.acquire (); - std::unique_lock lock (processing_mutex); + // wait until mark cross references args are set by the GC callback + shared_args_semaphore.acquire (); + MarkCrossReferencesArgs *args = shared_args.load (); bridge_processing_started_callback (); - BridgeProcessing bridge_processing {cross_refs}; + BridgeProcessing bridge_processing {args}; bridge_processing.process (); - bridge_processing_finished_callback (cross_refs); + bridge_processing_finished_callback (args); } } -void GCBridge::wait_for_bridge_processing () noexcept -{ - std::shared_lock lock (processing_mutex); -} - [[gnu::always_inline]] void GCBridge::log_mark_cross_references_args_if_enabled (MarkCrossReferencesArgs *args) noexcept { diff --git a/src/native/clr/host/internal-pinvokes.cc b/src/native/clr/host/internal-pinvokes.cc index 0fb9dc341f0..9b0bc817ad0 100644 --- a/src/native/clr/host/internal-pinvokes.cc +++ b/src/native/clr/host/internal-pinvokes.cc @@ -183,7 +183,8 @@ void _monodroid_lref_log_delete (int lrefc, jobject handle, char type, const cha void _monodroid_gc_wait_for_bridge_processing () { - GCBridge::wait_for_bridge_processing (); + // TODO do we need this method? + Helpers::abort_application (LOG_DEFAULT, "The method _monodroid_gc_wait_for_bridge_processing is not implemented. This is a stub and should not be called."sv); } void _monodroid_detect_cpu_and_architecture (uint16_t *built_for_cpu, uint16_t *running_on_cpu, unsigned char *is64bit) diff --git a/src/native/clr/include/host/gc-bridge.hh b/src/native/clr/include/host/gc-bridge.hh index 2bf34cda362..bb318c2a1b2 100644 --- a/src/native/clr/include/host/gc-bridge.hh +++ b/src/native/clr/include/host/gc-bridge.hh @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -18,6 +19,7 @@ struct JniObjectReferenceControlBlock struct HandleContext { + int32_t identity_hash_code; JniObjectReferenceControlBlock *control_block; bool is_collected () const noexcept @@ -54,7 +56,6 @@ namespace xamarin::android { class GCBridge { public: - static void wait_for_bridge_processing () noexcept; static void initialize_on_load (JNIEnv *env) noexcept; static void initialize_on_runtime_init (JNIEnv *env, jclass runtimeClass) noexcept; @@ -70,7 +71,7 @@ namespace xamarin::android { GCBridge::bridge_processing_started_callback = bridge_processing_started; GCBridge::bridge_processing_finished_callback = bridge_processing_finished; - bridge_processing_thread = new std::thread(GCBridge::bridge_processing); + bridge_processing_thread = new std::thread { GCBridge::bridge_processing }; bridge_processing_thread->detach (); return mark_cross_references; @@ -79,11 +80,10 @@ namespace xamarin::android { static void trigger_java_gc (JNIEnv *env) noexcept; private: - static inline std::binary_semaphore bridge_processing_semaphore{0}; - static inline std::shared_mutex processing_mutex; static inline std::thread *bridge_processing_thread = nullptr; - - static inline MarkCrossReferencesArgs *cross_refs; + + static inline std::binary_semaphore shared_args_semaphore{0}; + static inline std::atomic shared_args; static inline jobject Runtime_instance = nullptr; static inline jmethodID Runtime_gc = nullptr; @@ -92,7 +92,7 @@ namespace xamarin::android { static inline BridgeProcessingFinishedFtn bridge_processing_finished_callback = nullptr; static void bridge_processing () noexcept; - static void mark_cross_references (MarkCrossReferencesArgs *cross_refs) noexcept; + static void mark_cross_references (MarkCrossReferencesArgs *args) noexcept; static void log_mark_cross_references_args_if_enabled (MarkCrossReferencesArgs *args) noexcept; static void log_handle_context (JNIEnv *env, HandleContext *ctx) noexcept; From 058d6b2fcd5c77c2bd4cae8d63d5c20f6fc3afd5 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 19 Jun 2025 16:36:08 +0200 Subject: [PATCH 13/32] Cleanup --- .../Microsoft.Android.Runtime/ManagedValueManager.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index ee25ea614ba..ad7517283e9 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -329,9 +329,10 @@ unsafe struct HandleContext static readonly nuint Size = (nuint)Marshal.SizeOf (); static readonly Dictionary referenceTrackingHandles = new (); - public int PeerIdentityHashCode; + int identityHashCode; IntPtr controlBlock; + public int PeerIdentityHashCode => identityHashCode; public bool IsCollected => controlBlock == IntPtr.Zero; public static GCHandle GetAssociatedGCHandle (HandleContext* context) @@ -352,7 +353,7 @@ public static GCHandle GetAssociatedGCHandle (HandleContext* context) throw new OutOfMemoryException ("Failed to allocate memory for HandleContext."); } - context->PeerIdentityHashCode = peer.JniIdentityHashCode; + context->identityHashCode = peer.JniIdentityHashCode; context->controlBlock = peer.JniObjectReferenceControlBlock; GCHandle handle = JavaMarshal.CreateReferenceTrackingHandle (peer, context); @@ -381,7 +382,7 @@ static void BridgeProcessingStarted () [UnmanagedCallersOnly] static unsafe void BridgeProcessingFinished (MarkCrossReferencesArgs* mcr) { - Trace.Assert (mcr != null, "Bridge processing was not started before finishing it."); + Trace.Assert (mcr != null, "MarkCrossReferenceArgs should never be null."); ReadOnlySpan handlesToFree = ProcessCollectedContexts (mcr); JavaMarshal.FinishCrossReferenceProcessing (mcr, handlesToFree); From 59da00b5229c4fd450e136eeed79eb848abdf9d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Rozs=C3=ADval?= Date: Mon, 23 Jun 2025 13:16:58 +0200 Subject: [PATCH 14/32] Apply suggestions from code review Co-authored-by: Jonathan Peppers Co-authored-by: Marek Habersack --- .../Microsoft.Android.Runtime/ManagedValueManager.cs | 2 +- src/native/clr/include/host/gc-bridge.hh | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index ad7517283e9..93339f59dab 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -427,7 +427,7 @@ void ProcessContext (HandleContext* context) const BindingFlags ActivationConstructorBindingFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; - static readonly Type[] XAConstructorSignature = new Type[] { typeof (IntPtr), typeof (JniHandleOwnership) }; + static readonly Type[] XAConstructorSignature = new Type [] { typeof (IntPtr), typeof (JniHandleOwnership) }; protected override bool TryConstructPeer ( IJavaPeerable self, diff --git a/src/native/clr/include/host/gc-bridge.hh b/src/native/clr/include/host/gc-bridge.hh index bb318c2a1b2..e4fea8afe5f 100644 --- a/src/native/clr/include/host/gc-bridge.hh +++ b/src/native/clr/include/host/gc-bridge.hh @@ -71,7 +71,7 @@ namespace xamarin::android { GCBridge::bridge_processing_started_callback = bridge_processing_started; GCBridge::bridge_processing_finished_callback = bridge_processing_finished; - bridge_processing_thread = new std::thread { GCBridge::bridge_processing }; + bridge_processing_thread = std::thread { GCBridge::bridge_processing }; bridge_processing_thread->detach (); return mark_cross_references; @@ -80,10 +80,10 @@ namespace xamarin::android { static void trigger_java_gc (JNIEnv *env) noexcept; private: - static inline std::thread *bridge_processing_thread = nullptr; + static inline std::thread bridge_processing_thread {}; static inline std::binary_semaphore shared_args_semaphore{0}; - static inline std::atomic shared_args; + static inline std::atomic shared_args; static inline jobject Runtime_instance = nullptr; static inline jmethodID Runtime_gc = nullptr; From bd9945396813727e90d2e8577f3ea6ee27da269b Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 23 Jun 2025 14:11:05 +0200 Subject: [PATCH 15/32] Address feedback --- .../ManagedValueManager.cs | 44 ++++++++----- src/native/clr/host/bridge-processing.cc | 63 +++++++++---------- .../clr/include/host/bridge-processing.hh | 6 +- src/native/clr/include/host/gc-bridge.hh | 2 +- 4 files changed, 61 insertions(+), 54 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index 93339f59dab..325329f8c48 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -22,10 +22,12 @@ class ManagedValueManager : JniRuntime.JniValueManager { const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors; - Dictionary>? RegisteredInstances = new (); + readonly Dictionary> RegisteredInstances = new (); readonly ConcurrentQueue CollectedContexts = new (); - static readonly SemaphoreSlim bridgeProcessingSemaphore = new (1, 1); + bool disposed; + + static readonly SemaphoreSlim bridgeProcessingSemaphore = new(1, 1); static Lazy s_instance = new (() => new ManagedValueManager ()); public static ManagedValueManager GetOrCreateInstance () => s_instance.Value; @@ -37,16 +39,27 @@ unsafe ManagedValueManager () JavaMarshal.Initialize (mark_cross_references_ftn); } - public override void WaitForGCBridgeProcessing () + protected override void Dispose (bool disposing) { - bridgeProcessingSemaphore.Wait (); - bridgeProcessingSemaphore.Release (); + disposed = true; + base.Dispose (disposing); } - public unsafe override void CollectPeers () + void ThrowIfDisposed () { - if (RegisteredInstances == null) + if (disposed) throw new ObjectDisposedException (nameof (ManagedValueManager)); + } + + public override void WaitForGCBridgeProcessing() + { + bridgeProcessingSemaphore.Wait(); + bridgeProcessingSemaphore.Release(); + } + + public unsafe override void CollectPeers () + { + ThrowIfDisposed (); while (CollectedContexts.TryDequeue (out IntPtr contextPtr)) { HandleContext* context = (HandleContext*)contextPtr; @@ -79,8 +92,7 @@ void Remove (HandleContext* context) public override void AddPeer (IJavaPeerable value) { - if (RegisteredInstances == null) - throw new ObjectDisposedException (nameof (ManagedValueManager)); + ThrowIfDisposed (); // Remove any collected contexts before adding a new peer. CollectPeers (); @@ -140,8 +152,7 @@ void WarnNotReplacing (int key, IJavaPeerable ignoreValue, IJavaPeerable keepVal public override IJavaPeerable? PeekPeer (JniObjectReference reference) { - if (RegisteredInstances == null) - throw new ObjectDisposedException (nameof (ManagedValueManager)); + ThrowIfDisposed (); if (!reference.IsValid) return null; @@ -168,8 +179,7 @@ void WarnNotReplacing (int key, IJavaPeerable ignoreValue, IJavaPeerable keepVal public override void RemovePeer (IJavaPeerable value) { - if (RegisteredInstances == null) - throw new ObjectDisposedException (nameof (ManagedValueManager)); + ThrowIfDisposed (); // Remove any collected contexts before modifying RegisteredInstances CollectPeers (); @@ -268,8 +278,7 @@ Type GetDeclaringType (ConstructorInfo cinfo) => public override List GetSurfacedPeers () { - if (RegisteredInstances == null) - throw new ObjectDisposedException (nameof (ManagedValueManager)); + ThrowIfDisposed (); // Remove any collected contexts before iterating over all the registered instances CollectPeers (); @@ -451,8 +460,9 @@ protected override bool TryConstructPeer ( protected override bool TryUnboxPeerObject (IJavaPeerable value, [NotNullWhen (true)]out object? result) { - if (value is JavaProxyThrowable proxy) { - result = proxy.InnerException; + var proxy = value as JavaProxyThrowable; + if (proxy != null) { + result = proxy.InnerException; return true; } return base.TryUnboxPeerObject (value, out result); diff --git a/src/native/clr/host/bridge-processing.cc b/src/native/clr/host/bridge-processing.cc index 6fee7e10f07..29948f2d3e7 100644 --- a/src/native/clr/host/bridge-processing.cc +++ b/src/native/clr/host/bridge-processing.cc @@ -67,9 +67,10 @@ void BridgeProcessing::prepare_for_java_collection () noexcept for (size_t i = 0; i < cross_refs->ComponentCount; i++) { const StronglyConnectedComponent &scc = cross_refs->Components [i]; for (size_t j = 0; j < scc.Count; j++) { - HandleContext *context = scc.Contexts [j]; + const HandleContext *context = scc.Contexts [j]; abort_unless (context != nullptr, "Context must not be null"); - take_weak_global_ref (context); + + take_weak_global_ref (*context); } } } @@ -109,25 +110,25 @@ CrossReferenceTarget BridgeProcessing::select_cross_reference_target (size_t scc // caller must ensure that scc.Count > 1 void BridgeProcessing::add_circular_references (const StronglyConnectedComponent &scc) noexcept { - auto get_control_block = [&scc](size_t index) -> JniObjectReferenceControlBlock* { + auto get_control_block = [&scc](size_t index) -> JniObjectReferenceControlBlock& { abort_unless (scc.Contexts [index] != nullptr, "Context in SCC must not be null"); JniObjectReferenceControlBlock *control_block = scc.Contexts [index]->control_block; abort_unless (control_block != nullptr, "Control block in SCC must not be null"); - return control_block; + return *control_block; }; - JniObjectReferenceControlBlock *prev = get_control_block (scc.Count - 1); + JniObjectReferenceControlBlock &prev = get_control_block (scc.Count - 1); for (size_t j = 1; j < scc.Count; j++) { - JniObjectReferenceControlBlock *next = get_control_block (j); + JniObjectReferenceControlBlock &next = get_control_block (j); - bool reference_added = add_reference (prev->handle, next->handle); + bool reference_added = add_reference (prev.handle, next.handle); abort_unless (reference_added, [this, &prev, &next] { - jclass prev_java_class = env->GetObjectClass (prev->handle); + jclass prev_java_class = env->GetObjectClass (prev.handle); const char *prev_class_name = Host::get_java_class_name_for_TypeManager (prev_java_class); - jclass next_java_class = env->GetObjectClass (next->handle); + jclass next_java_class = env->GetObjectClass (next.handle); const char *next_class_name = Host::get_java_class_name_for_TypeManager (next_java_class); return detail::_format_message ( @@ -136,7 +137,7 @@ void BridgeProcessing::add_circular_references (const StronglyConnectedComponent next_class_name); }); - prev->refs_added = 1; + prev.refs_added = 1; prev = next; } } @@ -171,15 +172,13 @@ bool BridgeProcessing::add_reference (jobject from, jobject to) noexcept return true; } -void BridgeProcessing::clear_references_if_needed (HandleContext *context) noexcept +void BridgeProcessing::clear_references_if_needed (const HandleContext &context) noexcept { - // context is a valid pointer - validated at callsite - - if (context->is_collected ()) { + if (context.is_collected ()) { return; } - JniObjectReferenceControlBlock *control_block = context->control_block; + JniObjectReferenceControlBlock *control_block = context.control_block; abort_unless (control_block != nullptr, "Control block must not be null"); abort_unless (control_block->handle != nullptr, "Control block handle must not be null"); @@ -211,44 +210,42 @@ void BridgeProcessing::clear_references (jobject handle) noexcept env->CallVoidMethod (handle, clear_method_id); } -void BridgeProcessing::take_global_ref (HandleContext *context) noexcept +void BridgeProcessing::take_global_ref (HandleContext &context) noexcept { - // context is a valid pointer - validated at callsite - abort_unless (context->control_block != nullptr, "Control block must not be null"); - abort_unless (context->control_block->handle_type == JNIWeakGlobalRefType, "Expected weak global reference type for handle"); + abort_unless (context.control_block != nullptr, "Control block must not be null"); + abort_unless (context.control_block->handle_type == JNIWeakGlobalRefType, "Expected weak global reference type for handle"); - jobject weak = context->control_block->handle; + jobject weak = context.control_block->handle; jobject handle = env->NewGlobalRef (weak); log_weak_to_gref (weak, handle); if (handle != nullptr) { log_weak_ref_survived (weak, handle); - context->control_block->handle = handle; - context->control_block->handle_type = JNIGlobalRefType; + context.control_block->handle = handle; + context.control_block->handle_type = JNIGlobalRefType; env->DeleteWeakGlobalRef (weak); } else { // The native memory of the control block will be freed in managed code as well as the weak global ref - context->control_block = nullptr; + context.control_block = nullptr; log_weak_ref_collected (weak); } } -void BridgeProcessing::take_weak_global_ref (HandleContext *context) noexcept +void BridgeProcessing::take_weak_global_ref (const HandleContext &context) noexcept { - // context is a valid pointer - validated at callsite - abort_unless (context->control_block != nullptr, "Control block must not be null"); - abort_unless (context->control_block->handle_type == JNIGlobalRefType, "Expected global reference type for handle"); + abort_unless (context.control_block != nullptr, "Control block must not be null"); + abort_unless (context.control_block->handle_type == JNIGlobalRefType, "Expected global reference type for handle"); - jobject handle = context->control_block->handle; + jobject handle = context.control_block->handle; log_take_weak_global_ref (handle); jobject weak = env->NewWeakGlobalRef (handle); log_weak_gref_new (handle, weak); - context->control_block->handle = weak; - context->control_block->handle_type = JNIWeakGlobalRefType; + context.control_block->handle = weak; + context.control_block->handle_type = JNIWeakGlobalRefType; log_gref_delete (handle); env->DeleteGlobalRef (handle); @@ -264,8 +261,8 @@ void BridgeProcessing::cleanup_after_java_collection () noexcept HandleContext *context = scc.Contexts [j]; abort_unless (context != nullptr, "Context must not be null"); - take_global_ref (context); - clear_references_if_needed (context); + take_global_ref (*context); + clear_references_if_needed (*context); } abort_unless_all_collected_or_all_alive (scc); @@ -282,7 +279,7 @@ void BridgeProcessing::abort_unless_all_collected_or_all_alive (const StronglyCo bool is_collected = scc.Contexts [0]->is_collected (); for (size_t j = 1; j < scc.Count; j++) { - HandleContext *context = scc.Contexts [j]; + const HandleContext *context = scc.Contexts [j]; abort_unless (context != nullptr, "Context must not be null"); abort_unless (context->is_collected () == is_collected, "Cannot have a mix of collected and alive contexts in the SCC"); } diff --git a/src/native/clr/include/host/bridge-processing.hh b/src/native/clr/include/host/bridge-processing.hh index 3b66b9590a7..9e1ec5407bb 100644 --- a/src/native/clr/include/host/bridge-processing.hh +++ b/src/native/clr/include/host/bridge-processing.hh @@ -36,7 +36,7 @@ private: void prepare_for_java_collection () noexcept; void prepare_scc_for_java_collection (size_t scc_index, const StronglyConnectedComponent &scc) noexcept; - void take_weak_global_ref (HandleContext *context) noexcept; + void take_weak_global_ref (const HandleContext &context) noexcept; void add_circular_references (const StronglyConnectedComponent &scc) noexcept; void add_cross_reference (size_t source_index, size_t dest_index) noexcept; @@ -46,9 +46,9 @@ private: void cleanup_after_java_collection () noexcept; void cleanup_scc_for_java_collection (const StronglyConnectedComponent &scc) noexcept; void abort_unless_all_collected_or_all_alive (const StronglyConnectedComponent &scc) noexcept; - void take_global_ref (HandleContext *context) noexcept; + void take_global_ref (HandleContext &context) noexcept; - void clear_references_if_needed (HandleContext *context) noexcept; + void clear_references_if_needed (const HandleContext &context) noexcept; void clear_references (jobject handle) noexcept; void log_missing_add_references_method (jclass java_class) noexcept; diff --git a/src/native/clr/include/host/gc-bridge.hh b/src/native/clr/include/host/gc-bridge.hh index e4fea8afe5f..fe1bb1c39f6 100644 --- a/src/native/clr/include/host/gc-bridge.hh +++ b/src/native/clr/include/host/gc-bridge.hh @@ -72,7 +72,7 @@ namespace xamarin::android { GCBridge::bridge_processing_finished_callback = bridge_processing_finished; bridge_processing_thread = std::thread { GCBridge::bridge_processing }; - bridge_processing_thread->detach (); + bridge_processing_thread.detach (); return mark_cross_references; } From 76041d15e3d364d1fdba5c160a6ebedacefc36b0 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 3 Jul 2025 17:38:19 +0200 Subject: [PATCH 16/32] Fix bug when adding circular references --- .../Microsoft.Android.Runtime/ManagedValueManager.cs | 6 +++--- src/native/clr/host/bridge-processing.cc | 11 ++++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index 325329f8c48..f2f643d77ed 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -123,10 +123,10 @@ public override void AddPeer (IJavaPeerable value) if (peer.JniManagedPeerState.HasFlag (JniManagedPeerStates.Replaceable)) { p.Dispose (); peers [i] = new ReferenceTrackingHandle (value); - GC.KeepAlive (peer); } else { WarnNotReplacing (key, value, peer); } + GC.KeepAlive (peer); return; } @@ -198,8 +198,8 @@ public override void RemovePeer (IJavaPeerable value) if (object.ReferenceEquals (value, target)) { peers.RemoveAt (i); peer.Dispose (); - GC.KeepAlive (target); } + GC.KeepAlive (target); } if (peers.Count == 0) RegisteredInstances.Remove (key); @@ -458,7 +458,7 @@ protected override bool TryConstructPeer ( return base.TryConstructPeer (self, ref reference, options, type); } - protected override bool TryUnboxPeerObject (IJavaPeerable value, [NotNullWhen (true)]out object? result) + protected override bool TryUnboxPeerObject (IJavaPeerable value, [NotNullWhen (true)] out object? result) { var proxy = value as JavaProxyThrowable; if (proxy != null) { diff --git a/src/native/clr/host/bridge-processing.cc b/src/native/clr/host/bridge-processing.cc index 29948f2d3e7..73067abbcbb 100644 --- a/src/native/clr/host/bridge-processing.cc +++ b/src/native/clr/host/bridge-processing.cc @@ -117,10 +117,10 @@ void BridgeProcessing::add_circular_references (const StronglyConnectedComponent return *control_block; }; - JniObjectReferenceControlBlock &prev = get_control_block (scc.Count - 1); - - for (size_t j = 1; j < scc.Count; j++) { - JniObjectReferenceControlBlock &next = get_control_block (j); + size_t prev_index = scc.Count - 1; + for (size_t next_index = 0; next_index < scc.Count; next_index++) { + JniObjectReferenceControlBlock &prev = get_control_block (prev_index); + const JniObjectReferenceControlBlock &next = get_control_block (next_index); bool reference_added = add_reference (prev.handle, next.handle); @@ -138,7 +138,7 @@ void BridgeProcessing::add_circular_references (const StronglyConnectedComponent }); prev.refs_added = 1; - prev = next; + prev_index = next_index; } } @@ -161,6 +161,7 @@ bool BridgeProcessing::add_reference (jobject from, jobject to) noexcept jmethodID add_method_id = env->GetMethodID (java_class, "monodroidAddReference", "(Ljava/lang/Object;)V"); if (add_method_id == nullptr) [[unlikely]] { + // TODO: is this a fatal error? env->ExceptionClear (); log_missing_add_references_method (java_class); env->DeleteLocalRef (java_class); From 45d2460f2a4154328dd6a581d348cfdc51158dba Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 4 Jul 2025 11:46:27 +0200 Subject: [PATCH 17/32] Adjust code formatting --- .../ManagedValueManager.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index f2f643d77ed..52e6d00c20f 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -27,7 +27,7 @@ class ManagedValueManager : JniRuntime.JniValueManager bool disposed; - static readonly SemaphoreSlim bridgeProcessingSemaphore = new(1, 1); + static readonly SemaphoreSlim bridgeProcessingSemaphore = new (1, 1); static Lazy s_instance = new (() => new ManagedValueManager ()); public static ManagedValueManager GetOrCreateInstance () => s_instance.Value; @@ -51,10 +51,10 @@ void ThrowIfDisposed () throw new ObjectDisposedException (nameof (ManagedValueManager)); } - public override void WaitForGCBridgeProcessing() + public override void WaitForGCBridgeProcessing () { - bridgeProcessingSemaphore.Wait(); - bridgeProcessingSemaphore.Release(); + bridgeProcessingSemaphore.Wait (); + bridgeProcessingSemaphore.Release (); } public unsafe override void CollectPeers () @@ -298,8 +298,8 @@ public override List GetSurfacedPeers () unsafe struct ReferenceTrackingHandle : IDisposable { - private WeakReference _weakReference; - private HandleContext* _context; + WeakReference _weakReference; + HandleContext* _context; public bool BelongsToContext (HandleContext* context) => _context == context; @@ -357,7 +357,7 @@ public static GCHandle GetAssociatedGCHandle (HandleContext* context) public static HandleContext* Alloc (IJavaPeerable peer) { - var context = (HandleContext*)NativeMemory.AllocZeroed (1, Size); + var context = (HandleContext*) NativeMemory.AllocZeroed (1, Size); if (context == null) { throw new OutOfMemoryException ("Failed to allocate memory for HandleContext."); } @@ -367,7 +367,7 @@ public static GCHandle GetAssociatedGCHandle (HandleContext* context) GCHandle handle = JavaMarshal.CreateReferenceTrackingHandle (peer, context); lock (referenceTrackingHandles) { - referenceTrackingHandles [(IntPtr)context] = handle; + referenceTrackingHandles [(IntPtr) context] = handle; } return context; From a95e24724de8a3ce720bc7c4617a93aa5ff6c55e Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 4 Jul 2025 12:10:34 +0200 Subject: [PATCH 18/32] Add validation of context pointers before we start iterating on them in native code --- .../Android.Runtime/RuntimeNativeMethods.cs | 2 +- .../ManagedValueManager.cs | 22 ++++++++++++++++++- src/native/clr/host/gc-bridge.cc | 2 +- src/native/clr/include/host/gc-bridge.hh | 2 +- 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/Mono.Android/Android.Runtime/RuntimeNativeMethods.cs b/src/Mono.Android/Android.Runtime/RuntimeNativeMethods.cs index ae7042b4efa..e4c68c98f7a 100644 --- a/src/Mono.Android/Android.Runtime/RuntimeNativeMethods.cs +++ b/src/Mono.Android/Android.Runtime/RuntimeNativeMethods.cs @@ -95,7 +95,7 @@ internal unsafe static class RuntimeNativeMethods [DllImport (RuntimeConstants.InternalDllName, CallingConvention = CallingConvention.Cdecl)] internal static extern delegate* unmanaged clr_initialize_gc_bridge ( - delegate* unmanaged bridge_processing_started_callback, + delegate* unmanaged bridge_processing_started_callback, delegate* unmanaged bridge_processing_finished_callback); [MethodImplAttribute(MethodImplOptions.InternalCall)] diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index 52e6d00c20f..f6efac40677 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -355,6 +355,22 @@ public static GCHandle GetAssociatedGCHandle (HandleContext* context) } } + public static void EnsureAllContextsAreOurs (ReadOnlySpan contexts) + { + lock (referenceTrackingHandles) { + foreach (var context in contexts) { + EnsureContextIsOurs (context); + } + } + + static void EnsureContextIsOurs (IntPtr context) + { + if (!referenceTrackingHandles.ContainsKey (context)) { + throw new InvalidOperationException ("Unknown reference tracking handle."); + } + } + } + public static HandleContext* Alloc (IJavaPeerable peer) { var context = (HandleContext*) NativeMemory.AllocZeroed (1, Size); @@ -383,9 +399,13 @@ public static void Free (ref HandleContext* context) } [UnmanagedCallersOnly] - static void BridgeProcessingStarted () + static unsafe void BridgeProcessingStarted (MarkCrossReferencesArgs* mcr) { bridgeProcessingSemaphore.Wait (); + + int count = checked ((int)mcr->ComponentCount); + ReadOnlySpan contexts = new (mcr->Components, count); + HandleContext.EnsureAllContextsAreOurs (contexts); } [UnmanagedCallersOnly] diff --git a/src/native/clr/host/gc-bridge.cc b/src/native/clr/host/gc-bridge.cc index 9f310bebebf..d5eecdae4e2 100644 --- a/src/native/clr/host/gc-bridge.cc +++ b/src/native/clr/host/gc-bridge.cc @@ -69,7 +69,7 @@ void GCBridge::bridge_processing () noexcept shared_args_semaphore.acquire (); MarkCrossReferencesArgs *args = shared_args.load (); - bridge_processing_started_callback (); + bridge_processing_started_callback (args); BridgeProcessing bridge_processing {args}; bridge_processing.process (); diff --git a/src/native/clr/include/host/gc-bridge.hh b/src/native/clr/include/host/gc-bridge.hh index fe1bb1c39f6..e3d24e1ab3a 100644 --- a/src/native/clr/include/host/gc-bridge.hh +++ b/src/native/clr/include/host/gc-bridge.hh @@ -48,7 +48,7 @@ struct MarkCrossReferencesArgs ComponentCrossReference *CrossReferences; }; -using BridgeProcessingStartedFtn = void (*)(); +using BridgeProcessingStartedFtn = void (*)(MarkCrossReferencesArgs*); using BridgeProcessingFinishedFtn = void (*)(MarkCrossReferencesArgs*); using BridgeProcessingFtn = void (*)(MarkCrossReferencesArgs*); From 233bcad97e88e63a616f765243f436f9bb89545d Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 4 Jul 2025 12:18:44 +0200 Subject: [PATCH 19/32] Improve code readability --- .../ManagedValueManager.cs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index f6efac40677..5b8d56487f7 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -115,18 +115,18 @@ public override void AddPeer (IJavaPeerable value) } for (int i = peers.Count - 1; i >= 0; i--) { - var p = peers [i]; - if (p.Target is not IJavaPeerable peer) + ReferenceTrackingHandle peer = peers [i]; + if (peer.Target is not IJavaPeerable target) continue; if (!JniEnvironment.Types.IsSameObject (peer.PeerReference, value.PeerReference)) continue; - if (peer.JniManagedPeerState.HasFlag (JniManagedPeerStates.Replaceable)) { - p.Dispose (); + if (target.JniManagedPeerState.HasFlag (JniManagedPeerStates.Replaceable)) { + peer.Dispose (); peers [i] = new ReferenceTrackingHandle (value); } else { - WarnNotReplacing (key, value, peer); + WarnNotReplacing (key, value, target); } - GC.KeepAlive (peer); + GC.KeepAlive (target); return; } @@ -193,9 +193,9 @@ public override void RemovePeer (IJavaPeerable value) return; for (int i = peers.Count - 1; i >= 0; i--) { - var peer = peers [i]; - var target = peer.Target; - if (object.ReferenceEquals (value, target)) { + ReferenceTrackingHandle peer = peers [i]; + IJavaPeerable target = peer.Target; + if (ReferenceEquals (value, target)) { peers.RemoveAt (i); peer.Dispose (); } @@ -285,10 +285,10 @@ public override List GetSurfacedPeers () lock (RegisteredInstances) { var peers = new List (RegisteredInstances.Count); - foreach (var e in RegisteredInstances) { - foreach (var p in e.Value) { - if (p.Target is IJavaPeerable peer) { - peers.Add (new JniSurfacedPeerInfo (e.Key, new WeakReference (peer))); + foreach (var (identityHashCode, referenceTrackingHandles) in RegisteredInstances) { + foreach (var peer in referenceTrackingHandles) { + if (peer.Target is IJavaPeerable target) { + peers.Add (new JniSurfacedPeerInfo (identityHashCode, new WeakReference (target))); } } } From 61b6d8aa3b6d293b8bf2e4f15a6c8cce14d597ee Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 14 Jul 2025 18:56:31 +0200 Subject: [PATCH 20/32] Fix renaming issue --- .../Microsoft.Android.Runtime/ManagedValueManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index 5b8d56487f7..d8949f01994 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -118,7 +118,7 @@ public override void AddPeer (IJavaPeerable value) ReferenceTrackingHandle peer = peers [i]; if (peer.Target is not IJavaPeerable target) continue; - if (!JniEnvironment.Types.IsSameObject (peer.PeerReference, value.PeerReference)) + if (!JniEnvironment.Types.IsSameObject (target.PeerReference, value.PeerReference)) continue; if (target.JniManagedPeerState.HasFlag (JniManagedPeerStates.Replaceable)) { peer.Dispose (); From 9fed5820ffaca137ec28cae05bab786fbdd08787 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 14 Jul 2025 13:41:44 -0500 Subject: [PATCH 21/32] Update BuildReleaseArm64SimpleDotNet.apkdesc --- .../BuildReleaseArm64SimpleDotNet.apkdesc | 47 ++++++++++--------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc index 0d09c5250f4..ab96a8a915e 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc @@ -5,73 +5,76 @@ "Size": 3036 }, "classes.dex": { - "Size": 22484 + "Size": 22488 }, "lib/arm64-v8a/lib__Microsoft.Android.Resource.Designer.dll.so": { "Size": 18288 }, "lib/arm64-v8a/lib_Java.Interop.dll.so": { - "Size": 86688 + "Size": 88728 }, "lib/arm64-v8a/lib_Mono.Android.dll.so": { - "Size": 117712 + "Size": 124072 }, "lib/arm64-v8a/lib_Mono.Android.Runtime.dll.so": { - "Size": 22384 + "Size": 23432 }, "lib/arm64-v8a/lib_System.Console.dll.so": { - "Size": 24392 + "Size": 24408 + }, + "lib/arm64-v8a/lib_System.Diagnostics.TraceSource.dll.so": { + "Size": 24056 }, "lib/arm64-v8a/lib_System.Linq.dll.so": { - "Size": 25336 + "Size": 25552 }, "lib/arm64-v8a/lib_System.Private.CoreLib.dll.so": { - "Size": 628216 + "Size": 695560 }, "lib/arm64-v8a/lib_System.Runtime.dll.so": { - "Size": 20056 + "Size": 20104 }, "lib/arm64-v8a/lib_System.Runtime.InteropServices.dll.so": { - "Size": 21480 + "Size": 21528 }, "lib/arm64-v8a/lib_UnnamedProject.dll.so": { "Size": 20024 }, "lib/arm64-v8a/libarc.bin.so": { - "Size": 18872 + "Size": 18984 }, "lib/arm64-v8a/libmono-component-marshal-ilgen.so": { - "Size": 36440 + "Size": 36616 }, "lib/arm64-v8a/libmonodroid.so": { - "Size": 1524752 + "Size": 1508312 }, "lib/arm64-v8a/libmonosgen-2.0.so": { - "Size": 3101112 + "Size": 3119744 }, "lib/arm64-v8a/libSystem.Globalization.Native.so": { - "Size": 71976 + "Size": 71952 }, "lib/arm64-v8a/libSystem.IO.Compression.Native.so": { - "Size": 758896 + "Size": 759304 }, "lib/arm64-v8a/libSystem.Native.so": { - "Size": 103520 + "Size": 104216 }, "lib/arm64-v8a/libSystem.Security.Cryptography.Native.Android.so": { - "Size": 165000 + "Size": 165240 }, "lib/arm64-v8a/libxamarin-app.so": { - "Size": 19536 + "Size": 19944 }, "META-INF/BNDLTOOL.RSA": { - "Size": 1221 + "Size": 1219 }, "META-INF/BNDLTOOL.SF": { - "Size": 3266 + "Size": 3393 }, "META-INF/MANIFEST.MF": { - "Size": 3139 + "Size": 3266 }, "res/drawable-hdpi-v4/icon.png": { "Size": 2178 @@ -98,5 +101,5 @@ "Size": 1904 } }, - "PackageSize": 3078677 + "PackageSize": 3156602 } \ No newline at end of file From 3a54084fc6c755d83387cde5e1573ea3865fc398 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 15 Jul 2025 08:45:39 +0200 Subject: [PATCH 22/32] Clean up registered reference tracking handles dictionary --- .../Microsoft.Android.Runtime/ManagedValueManager.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index d8949f01994..86c312bf93d 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -392,6 +392,10 @@ static void EnsureContextIsOurs (IntPtr context) public static void Free (ref HandleContext* context) { if (context != null) { + lock (referenceTrackingHandles) { + referenceTrackingHandles.Remove ((IntPtr)context); + } + NativeMemory.Free (context); context = null; } From 07283b8c6d27431a40f6021a57135160ee5235b0 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 15 Jul 2025 08:48:41 +0200 Subject: [PATCH 23/32] Avoid using Trace.Assert --- .../Microsoft.Android.Runtime/ManagedValueManager.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index 86c312bf93d..499b53c0c5f 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -415,7 +415,9 @@ static unsafe void BridgeProcessingStarted (MarkCrossReferencesArgs* mcr) [UnmanagedCallersOnly] static unsafe void BridgeProcessingFinished (MarkCrossReferencesArgs* mcr) { - Trace.Assert (mcr != null, "MarkCrossReferenceArgs should never be null."); + if (mcr == null) { + throw new ArgumentNullException (nameof (mcr), "MarkCrossReferencesArgs should never be null."); + } ReadOnlySpan handlesToFree = ProcessCollectedContexts (mcr); JavaMarshal.FinishCrossReferenceProcessing (mcr, handlesToFree); @@ -437,7 +439,9 @@ static unsafe ReadOnlySpan ProcessCollectedContexts (MarkCrossReferenc void ProcessContext (HandleContext* context) { - Trace.Assert (context != null, "Context should never be null."); + if (context == null) { + throw new ArgumentNullException (nameof (context), "HandleContext should never be null."); + } // Ignore contexts which were not collected if (!context->IsCollected) { From 98e0b0d208583565b8bdc864a6439c450f8f83ca Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 15 Jul 2025 08:50:58 +0200 Subject: [PATCH 24/32] Add debug assert for CollectedContexts invariant --- .../Microsoft.Android.Runtime/ManagedValueManager.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index 499b53c0c5f..b2dbb343054 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -62,6 +62,7 @@ public unsafe override void CollectPeers () ThrowIfDisposed (); while (CollectedContexts.TryDequeue (out IntPtr contextPtr)) { + Debug.Assert (contextPtr != IntPtr.Zero, "CollectedContexts should not contain null pointers."); HandleContext* context = (HandleContext*)contextPtr; lock (RegisteredInstances) { From 1721e7f25e902541c36d12f76f005659dbcf208f Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 15 Jul 2025 08:51:59 +0200 Subject: [PATCH 25/32] Reduce nesting --- .../ManagedValueManager.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index b2dbb343054..982aa41e65d 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -392,14 +392,16 @@ static void EnsureContextIsOurs (IntPtr context) public static void Free (ref HandleContext* context) { - if (context != null) { - lock (referenceTrackingHandles) { - referenceTrackingHandles.Remove ((IntPtr)context); - } + if (context == null) { + return; + } - NativeMemory.Free (context); - context = null; + lock (referenceTrackingHandles) { + referenceTrackingHandles.Remove ((IntPtr)context); } + + NativeMemory.Free (context); + context = null; } } From 93f1afd64e67ec3903fdd7dfdd1090a9432ebf96 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 15 Jul 2025 11:21:27 +0200 Subject: [PATCH 26/32] Fix input validation --- rebuild-android-and-run-testapp.sh | 7 ++++++ .../ManagedValueManager.cs | 23 +++++++++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) create mode 100755 rebuild-android-and-run-testapp.sh diff --git a/rebuild-android-and-run-testapp.sh b/rebuild-android-and-run-testapp.sh new file mode 100755 index 00000000000..112dd55d669 --- /dev/null +++ b/rebuild-android-and-run-testapp.sh @@ -0,0 +1,7 @@ +#!/bin/bash +set -x + +make CONFIGURATION=Debug prepare all +rm -rf ~/.nuget/packages/* +./dotnet-local.sh build Xamarin.Android.sln -c Debug -t:InstallMaui -p:MauiVersion=10.0.0-preview.5.25306.5 -p:MauiVersionBand=10.0.100-preview.5 +./dotnet-local.sh build -t:Run -c Debug -f net10.0-android TestApp -p:UseMonoRuntime=false diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index 982aa41e65d..5002cf4d858 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -356,11 +356,19 @@ public static GCHandle GetAssociatedGCHandle (HandleContext* context) } } - public static void EnsureAllContextsAreOurs (ReadOnlySpan contexts) + public static unsafe void EnsureAllContextsAreOurs (MarkCrossReferencesArgs* mcr) { lock (referenceTrackingHandles) { - foreach (var context in contexts) { - EnsureContextIsOurs (context); + for (nuint i = 0; i < mcr->ComponentCount; i++) { + StronglyConnectedComponent component = mcr->Components [i]; + EnsureAllContextsInComponentAreOurs (component); + } + } + + static void EnsureAllContextsInComponentAreOurs (StronglyConnectedComponent component) + { + for (nuint i = 0; i < component.Count; i++) { + EnsureContextIsOurs ((IntPtr)component.Contexts [i]); } } @@ -408,11 +416,12 @@ public static void Free (ref HandleContext* context) [UnmanagedCallersOnly] static unsafe void BridgeProcessingStarted (MarkCrossReferencesArgs* mcr) { - bridgeProcessingSemaphore.Wait (); + if (mcr == null) { + throw new ArgumentNullException (nameof (mcr), "MarkCrossReferencesArgs should never be null."); + } - int count = checked ((int)mcr->ComponentCount); - ReadOnlySpan contexts = new (mcr->Components, count); - HandleContext.EnsureAllContextsAreOurs (contexts); + HandleContext.EnsureAllContextsAreOurs (mcr); + bridgeProcessingSemaphore.Wait (); } [UnmanagedCallersOnly] From 0ea139e82e60229e1f4a6a437cc722b0d6cbc660 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 15 Jul 2025 08:24:15 -0500 Subject: [PATCH 27/32] Revert "Update BuildReleaseArm64SimpleDotNet.apkdesc" This reverts commit 9fed5820ffaca137ec28cae05bab786fbdd08787. --- .../BuildReleaseArm64SimpleDotNet.apkdesc | 47 +++++++++---------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc index ab96a8a915e..0d09c5250f4 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc @@ -5,76 +5,73 @@ "Size": 3036 }, "classes.dex": { - "Size": 22488 + "Size": 22484 }, "lib/arm64-v8a/lib__Microsoft.Android.Resource.Designer.dll.so": { "Size": 18288 }, "lib/arm64-v8a/lib_Java.Interop.dll.so": { - "Size": 88728 + "Size": 86688 }, "lib/arm64-v8a/lib_Mono.Android.dll.so": { - "Size": 124072 + "Size": 117712 }, "lib/arm64-v8a/lib_Mono.Android.Runtime.dll.so": { - "Size": 23432 + "Size": 22384 }, "lib/arm64-v8a/lib_System.Console.dll.so": { - "Size": 24408 - }, - "lib/arm64-v8a/lib_System.Diagnostics.TraceSource.dll.so": { - "Size": 24056 + "Size": 24392 }, "lib/arm64-v8a/lib_System.Linq.dll.so": { - "Size": 25552 + "Size": 25336 }, "lib/arm64-v8a/lib_System.Private.CoreLib.dll.so": { - "Size": 695560 + "Size": 628216 }, "lib/arm64-v8a/lib_System.Runtime.dll.so": { - "Size": 20104 + "Size": 20056 }, "lib/arm64-v8a/lib_System.Runtime.InteropServices.dll.so": { - "Size": 21528 + "Size": 21480 }, "lib/arm64-v8a/lib_UnnamedProject.dll.so": { "Size": 20024 }, "lib/arm64-v8a/libarc.bin.so": { - "Size": 18984 + "Size": 18872 }, "lib/arm64-v8a/libmono-component-marshal-ilgen.so": { - "Size": 36616 + "Size": 36440 }, "lib/arm64-v8a/libmonodroid.so": { - "Size": 1508312 + "Size": 1524752 }, "lib/arm64-v8a/libmonosgen-2.0.so": { - "Size": 3119744 + "Size": 3101112 }, "lib/arm64-v8a/libSystem.Globalization.Native.so": { - "Size": 71952 + "Size": 71976 }, "lib/arm64-v8a/libSystem.IO.Compression.Native.so": { - "Size": 759304 + "Size": 758896 }, "lib/arm64-v8a/libSystem.Native.so": { - "Size": 104216 + "Size": 103520 }, "lib/arm64-v8a/libSystem.Security.Cryptography.Native.Android.so": { - "Size": 165240 + "Size": 165000 }, "lib/arm64-v8a/libxamarin-app.so": { - "Size": 19944 + "Size": 19536 }, "META-INF/BNDLTOOL.RSA": { - "Size": 1219 + "Size": 1221 }, "META-INF/BNDLTOOL.SF": { - "Size": 3393 + "Size": 3266 }, "META-INF/MANIFEST.MF": { - "Size": 3266 + "Size": 3139 }, "res/drawable-hdpi-v4/icon.png": { "Size": 2178 @@ -101,5 +98,5 @@ "Size": 1904 } }, - "PackageSize": 3156602 + "PackageSize": 3078677 } \ No newline at end of file From ffa44b34b52cde419cc497b5f212fcdb6498f613 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 15 Jul 2025 17:59:40 +0200 Subject: [PATCH 28/32] Fix gref and gwref accounting --- .../ManagedValueManager.cs | 34 +++++++++++---- src/native/clr/host/bridge-processing.cc | 43 +++++++++---------- src/native/clr/host/internal-pinvokes.cc | 17 +------- .../clr/include/host/bridge-processing.hh | 2 +- src/native/clr/include/host/gc-bridge.hh | 3 +- 5 files changed, 51 insertions(+), 48 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index 5002cf4d858..e8a637919aa 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -343,19 +343,37 @@ unsafe struct HandleContext IntPtr controlBlock; public int PeerIdentityHashCode => identityHashCode; - public bool IsCollected => controlBlock == IntPtr.Zero; - - public static GCHandle GetAssociatedGCHandle (HandleContext* context) + public bool IsCollected { - lock (referenceTrackingHandles) { - if (!referenceTrackingHandles.TryGetValue ((IntPtr)context, out GCHandle handle)) { - throw new InvalidOperationException ("Unknown reference tracking handle."); - } + get + { + if (controlBlock == IntPtr.Zero) + throw new InvalidOperationException ("HandleContext control block is not initialized."); - return handle; + return ((JniObjectReferenceControlBlock*) controlBlock)->handle == IntPtr.Zero; } } + // This is an internal mirror of the Java.Interop.JniObjectReferenceControlBlock + private struct JniObjectReferenceControlBlock + { + public IntPtr handle; + public int handle_type; + public IntPtr weak_handle; + public int refs_added; + } + + public static GCHandle GetAssociatedGCHandle (HandleContext* context) + { + lock (referenceTrackingHandles) { + if (!referenceTrackingHandles.TryGetValue ((IntPtr) context, out GCHandle handle)) { + throw new InvalidOperationException ("Unknown reference tracking handle."); + } + + return handle; + } + } + public static unsafe void EnsureAllContextsAreOurs (MarkCrossReferencesArgs* mcr) { lock (referenceTrackingHandles) { diff --git a/src/native/clr/host/bridge-processing.cc b/src/native/clr/host/bridge-processing.cc index 73067abbcbb..a72e75d902a 100644 --- a/src/native/clr/host/bridge-processing.cc +++ b/src/native/clr/host/bridge-processing.cc @@ -220,18 +220,15 @@ void BridgeProcessing::take_global_ref (HandleContext &context) noexcept jobject handle = env->NewGlobalRef (weak); log_weak_to_gref (weak, handle); - if (handle != nullptr) { - log_weak_ref_survived (weak, handle); - - context.control_block->handle = handle; - context.control_block->handle_type = JNIGlobalRefType; - - env->DeleteWeakGlobalRef (weak); - } else { - // The native memory of the control block will be freed in managed code as well as the weak global ref - context.control_block = nullptr; + if (handle == nullptr) { log_weak_ref_collected (weak); } + + context.control_block->handle = handle; // by doing this, the weak reference won't be deleted AGAIN in managed code + context.control_block->handle_type = JNIGlobalRefType; + + log_weak_ref_delete (weak); + env->DeleteWeakGlobalRef (weak); } void BridgeProcessing::take_weak_global_ref (const HandleContext &context) noexcept @@ -343,6 +340,13 @@ void BridgeProcessing::log_missing_clear_references_method ([[maybe_unused]] jcl [[gnu::always_inline]] void BridgeProcessing::log_weak_to_gref (jobject weak, jobject handle) noexcept { + if (handle != nullptr) { + OSBridge::_monodroid_gref_log_new (weak, OSBridge::get_object_ref_type (env, weak), + handle, OSBridge::get_object_ref_type (env, handle), + "finalizer", gettid (), + " at [[clr-gc:take_global_ref]]", 0); + } + if (!Logger::gref_log ()) [[likely]] { return; } @@ -353,18 +357,6 @@ void BridgeProcessing::log_weak_to_gref (jobject weak, jobject handle) noexcept reinterpret_cast (handle)).data ()); } -[[gnu::always_inline]] -void BridgeProcessing::log_weak_ref_survived (jobject weak, jobject handle) noexcept -{ - OSBridge::_monodroid_gref_log_new (weak, OSBridge::get_object_ref_type (env, weak), - handle, OSBridge::get_object_ref_type (env, handle), - "finalizer", gettid (), - " at [[clr-gc:take_global_ref]]", 0); - - OSBridge::_monodroid_weak_gref_delete (weak, OSBridge::get_object_ref_type (env, weak), - "finalizer", gettid (), " at [[clr-gc:take_global_ref]]", 0); -} - [[gnu::always_inline]] void BridgeProcessing::log_weak_ref_collected (jobject weak) noexcept { @@ -401,6 +393,13 @@ void BridgeProcessing::log_gref_delete (jobject handle) noexcept "finalizer", gettid (), " at [[clr-gc:take_weak_global_ref]]", 0); } +[[gnu::always_inline]] +void BridgeProcessing::log_weak_ref_delete (jobject weak) noexcept +{ + OSBridge::_monodroid_weak_gref_delete (weak, OSBridge::get_object_ref_type (env, weak), + "finalizer", gettid (), " at [[clr-gc:take_global_ref]]", 0); +} + [[gnu::always_inline]] void BridgeProcessing::log_gc_summary () noexcept { diff --git a/src/native/clr/host/internal-pinvokes.cc b/src/native/clr/host/internal-pinvokes.cc index 9b0bc817ad0..5260a6bac6a 100644 --- a/src/native/clr/host/internal-pinvokes.cc +++ b/src/native/clr/host/internal-pinvokes.cc @@ -26,22 +26,7 @@ int _monodroid_gref_log_new (jobject curHandle, char curType, jobject newHandle, void _monodroid_gref_log_delete (jobject handle, char type, const char *threadName, int threadId, const char *from, int from_writable) noexcept { - // NOTE: disabled until GC bridge is in - // It currently causes a crash in Mono.Android_Tests: - // - // FATAL EXCEPTION: Instr: xamarin.android.runtimetests.NUnitInstrumentation - // Process: Mono.Android.NET_Tests, PID: 16194 - // android.runtime.JavaProxyThrowable: [System.ObjectDisposedException]: Cannot access disposed object with JniIdentityHashCode=166175665. - // Object name: 'Xamarin.Android.RuntimeTests.NUnitInstrumentation'. - // at Java.Interop.JniPeerMembers.AssertSelf + 0x2f(Unknown Source) - // at Java.Interop.JniPeerMembers+JniInstanceMethods.InvokeVirtualVoidMethod + 0x1(Unknown Source) - // at Android.App.Instrumentation.Finish + 0x46(Unknown Source) - // at Xamarin.Android.UnitTests.TestInstrumentation`1.OnStart + 0x6d(Unknown Source) - // at Android.App.Instrumentation.n_OnStart + 0x1f(Unknown Source) - // at crc643df67da7b13bb6b1.TestInstrumentation_1.n_onStart(Native Method) - // at crc643df67da7b13bb6b1.TestInstrumentation_1.onStart(TestInstrumentation_1.java:32) - // at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2606) - // OSBridge::_monodroid_gref_log_delete (handle, type, threadName, threadId, from, from_writable); + OSBridge::_monodroid_gref_log_delete (handle, type, threadName, threadId, from, from_writable); } const char* clr_typemap_managed_to_java (const char *typeName, const uint8_t *mvid) noexcept diff --git a/src/native/clr/include/host/bridge-processing.hh b/src/native/clr/include/host/bridge-processing.hh index 9e1ec5407bb..0fdb56d931e 100644 --- a/src/native/clr/include/host/bridge-processing.hh +++ b/src/native/clr/include/host/bridge-processing.hh @@ -54,10 +54,10 @@ private: void log_missing_add_references_method (jclass java_class) noexcept; void log_missing_clear_references_method (jclass java_class) noexcept; void log_weak_to_gref (jobject weak, jobject handle) noexcept; - void log_weak_ref_survived (jobject weak, jobject handle) noexcept; void log_weak_ref_collected (jobject weak) noexcept; void log_take_weak_global_ref (jobject handle) noexcept; void log_weak_gref_new (jobject handle, jobject weak) noexcept; void log_gref_delete (jobject handle) noexcept; + void log_weak_ref_delete (jobject weak) noexcept; void log_gc_summary () noexcept; }; diff --git a/src/native/clr/include/host/gc-bridge.hh b/src/native/clr/include/host/gc-bridge.hh index e3d24e1ab3a..6fa2448fd65 100644 --- a/src/native/clr/include/host/gc-bridge.hh +++ b/src/native/clr/include/host/gc-bridge.hh @@ -24,7 +24,8 @@ struct HandleContext bool is_collected () const noexcept { - return control_block == nullptr; + abort_unless (control_block != nullptr, "Control block must not be null"); + return control_block->handle == nullptr; } }; From cc9df5d502feaf4d4990e536452a06d2484037c6 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 15 Jul 2025 18:20:09 +0200 Subject: [PATCH 29/32] Update apkdesc --- .../BuildReleaseArm64SimpleDotNet.apkdesc | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc index 0d09c5250f4..59aacae52d3 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc @@ -5,67 +5,67 @@ "Size": 3036 }, "classes.dex": { - "Size": 22484 + "Size": 22488 }, "lib/arm64-v8a/lib__Microsoft.Android.Resource.Designer.dll.so": { "Size": 18288 }, "lib/arm64-v8a/lib_Java.Interop.dll.so": { - "Size": 86688 + "Size": 88472 }, "lib/arm64-v8a/lib_Mono.Android.dll.so": { - "Size": 117712 + "Size": 124040 }, "lib/arm64-v8a/lib_Mono.Android.Runtime.dll.so": { - "Size": 22384 + "Size": 23424 }, "lib/arm64-v8a/lib_System.Console.dll.so": { - "Size": 24392 + "Size": 24408 }, "lib/arm64-v8a/lib_System.Linq.dll.so": { - "Size": 25336 + "Size": 25488 }, "lib/arm64-v8a/lib_System.Private.CoreLib.dll.so": { - "Size": 628216 + "Size": 692528 }, "lib/arm64-v8a/lib_System.Runtime.dll.so": { - "Size": 20056 + "Size": 20104 }, "lib/arm64-v8a/lib_System.Runtime.InteropServices.dll.so": { - "Size": 21480 + "Size": 21528 }, "lib/arm64-v8a/lib_UnnamedProject.dll.so": { "Size": 20024 }, "lib/arm64-v8a/libarc.bin.so": { - "Size": 18872 + "Size": 18984 }, "lib/arm64-v8a/libmono-component-marshal-ilgen.so": { - "Size": 36440 + "Size": 36616 }, "lib/arm64-v8a/libmonodroid.so": { - "Size": 1524752 + "Size": 1508312 }, "lib/arm64-v8a/libmonosgen-2.0.so": { - "Size": 3101112 + "Size": 3119744 }, "lib/arm64-v8a/libSystem.Globalization.Native.so": { - "Size": 71976 + "Size": 71952 }, "lib/arm64-v8a/libSystem.IO.Compression.Native.so": { - "Size": 758896 + "Size": 759304 }, "lib/arm64-v8a/libSystem.Native.so": { - "Size": 103520 + "Size": 104216 }, "lib/arm64-v8a/libSystem.Security.Cryptography.Native.Android.so": { - "Size": 165000 + "Size": 165240 }, "lib/arm64-v8a/libxamarin-app.so": { - "Size": 19536 + "Size": 19800 }, "META-INF/BNDLTOOL.RSA": { - "Size": 1221 + "Size": 1223 }, "META-INF/BNDLTOOL.SF": { "Size": 3266 @@ -98,5 +98,5 @@ "Size": 1904 } }, - "PackageSize": 3078677 + "PackageSize": 3148309 } \ No newline at end of file From f453805c71d78950e6d92185b9ba2d2ea7d7ce1e Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 16 Jul 2025 09:09:52 +0200 Subject: [PATCH 30/32] Remove unnecessary script file --- rebuild-android-and-run-testapp.sh | 7 ------- 1 file changed, 7 deletions(-) delete mode 100755 rebuild-android-and-run-testapp.sh diff --git a/rebuild-android-and-run-testapp.sh b/rebuild-android-and-run-testapp.sh deleted file mode 100755 index 112dd55d669..00000000000 --- a/rebuild-android-and-run-testapp.sh +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/bash -set -x - -make CONFIGURATION=Debug prepare all -rm -rf ~/.nuget/packages/* -./dotnet-local.sh build Xamarin.Android.sln -c Debug -t:InstallMaui -p:MauiVersion=10.0.0-preview.5.25306.5 -p:MauiVersionBand=10.0.100-preview.5 -./dotnet-local.sh build -t:Run -c Debug -f net10.0-android TestApp -p:UseMonoRuntime=false From e2e203abb2a3d0f287019a292cff62188583d4da Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 16 Jul 2025 09:10:44 +0200 Subject: [PATCH 31/32] Fix formatting --- .../ManagedValueManager.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs index e8a637919aa..e84a518971b 100644 --- a/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs +++ b/src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs @@ -364,15 +364,15 @@ private struct JniObjectReferenceControlBlock } public static GCHandle GetAssociatedGCHandle (HandleContext* context) - { - lock (referenceTrackingHandles) { - if (!referenceTrackingHandles.TryGetValue ((IntPtr) context, out GCHandle handle)) { - throw new InvalidOperationException ("Unknown reference tracking handle."); - } - - return handle; + { + lock (referenceTrackingHandles) { + if (!referenceTrackingHandles.TryGetValue ((IntPtr) context, out GCHandle handle)) { + throw new InvalidOperationException ("Unknown reference tracking handle."); } + + return handle; } + } public static unsafe void EnsureAllContextsAreOurs (MarkCrossReferencesArgs* mcr) { From 6a7782f09006731aaebfeb9a445efafac93d21c4 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 16 Jul 2025 09:12:12 +0200 Subject: [PATCH 32/32] Align naming of initialize_on_onload --- src/native/clr/host/gc-bridge.cc | 2 +- src/native/clr/host/host.cc | 2 +- src/native/clr/include/host/gc-bridge.hh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/native/clr/host/gc-bridge.cc b/src/native/clr/host/gc-bridge.cc index d5eecdae4e2..6bf3a387e96 100644 --- a/src/native/clr/host/gc-bridge.cc +++ b/src/native/clr/host/gc-bridge.cc @@ -7,7 +7,7 @@ using namespace xamarin::android; -void GCBridge::initialize_on_load (JNIEnv *env) noexcept +void GCBridge::initialize_on_onload (JNIEnv *env) noexcept { abort_if_invalid_pointer_argument (env, "env"); diff --git a/src/native/clr/host/host.cc b/src/native/clr/host/host.cc index 00c7893a9dd..ea8ccbc5215 100644 --- a/src/native/clr/host/host.cc +++ b/src/native/clr/host/host.cc @@ -572,7 +572,7 @@ auto Host::Java_JNI_OnLoad (JavaVM *vm, [[maybe_unused]] void *reserved) noexcep JNIEnv *env = nullptr; vm->GetEnv ((void**)&env, JNI_VERSION_1_6); OSBridge::initialize_on_onload (vm, env); - GCBridge::initialize_on_load (env); + GCBridge::initialize_on_onload (env); AndroidSystem::init_max_gref_count (); return JNI_VERSION_1_6; diff --git a/src/native/clr/include/host/gc-bridge.hh b/src/native/clr/include/host/gc-bridge.hh index 6fa2448fd65..80c0ac68132 100644 --- a/src/native/clr/include/host/gc-bridge.hh +++ b/src/native/clr/include/host/gc-bridge.hh @@ -57,7 +57,7 @@ namespace xamarin::android { class GCBridge { public: - static void initialize_on_load (JNIEnv *env) noexcept; + static void initialize_on_onload (JNIEnv *env) noexcept; static void initialize_on_runtime_init (JNIEnv *env, jclass runtimeClass) noexcept; static BridgeProcessingFtn initialize_callback (