From e1bbaefa3e816450b023a1ed7415836e3da39cb2 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Fri, 9 May 2025 20:38:09 -0400 Subject: [PATCH 1/8] [Java.Interop] JNI handles are now in a "control block" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/dotnet/runtime/pull/114184 Context: https://github.com/dotnet/android/pull/10125 Context: https://github.com/dotnet/android/pull/10125#issuecomment-2864434118 Part of adding a GC bridge to CoreCLR are the new APIs: namespace System.Runtime.InteropServices.Java; public struct ComponentCrossReference { public nint SourceGroupIndex, DestinationGroupIndex; } public unsafe struct StronglyConnectedComponent { public nint Count; public IntPtr* Context; } public static partial class JavaMarshal { public static unsafe void Initialize( delegate* unmanaged< System.IntPtr, // sccsLen StronglyConnectedComponent*, // sccs System.IntPtr, // ccrsLen ComponentCrossReference*, // ccrs void> markCrossReferences); public static GCHandle CreateReferenceTrackingHandle(object obj, IntPtr context); public static IntPtr GetContext(GCHandle obj); } Of note is the "data flow" of `context`: * `JavaMarshal.CreateReferenceTrackingHandle()` has a "`context`" parameter. * The `context` parameter to `JavaMarshal.CreateReferenceTrackingHandle()` is the return value of `JavaMarshal.GetContext()` * The `context` parameter to `JavaMarshal.CreateReferenceTrackingHandle()` is stored within `StronglyConnectedComponent.Context`. * The `markCrossReferences` parameter of `JavaMarshal.Initialize()` is called by the GC bridge and given a native array of `StronglyConnectedComponent` instances, which contains `Context`. The short of it is that the proposed GC bridge doesn't contain direct access to the `IJavaPeerable` instances in play. Instead, it has access to "context" which must contain the JNI Object Reference information that the `markCrossReferences` callback needs access to. Furthermore, the `context` pointer value *cannot change*, i.e. it needs to be a native pointer value a'la **malloc**(3), ***not*** a value which can be moved by the GC. (The *contents* can change; the pointer value cannot.)) While we're still prototyping this, what we currently believe we need is the JNI object reference, JNI object reference type, and (maybe?) the JNI Weak Global Reference value and "refs added" values. Update `IJavaPeerable` to add a `JniObjectReferenceControlBlock` property which can be used as the `context` parameter: partial interface IJavaPeerable { IntPtr JniObjectReferenceControlBlock => 0; } This supports usage of: IJavaPeerable value = … GCHandle handle = JavaMarshal.CreateReferenceTrackingHandle( value, value.JniObjectReferenceControlBlock ); --- .../Java.Interop/IJavaPeerable.cs | 2 ++ src/Java.Interop/Java.Interop/JavaObject.cs | 34 +++++++++++++------ src/Java.Interop/PublicAPI.Unshipped.txt | 1 + 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/Java.Interop/Java.Interop/IJavaPeerable.cs b/src/Java.Interop/Java.Interop/IJavaPeerable.cs index 612744b10..a1888e7e8 100644 --- a/src/Java.Interop/Java.Interop/IJavaPeerable.cs +++ b/src/Java.Interop/Java.Interop/IJavaPeerable.cs @@ -37,6 +37,8 @@ public interface IJavaPeerable : IDisposable void Disposed (); /// void Finalized (); + + IntPtr JniObjectReferenceControlBlock => IntPtr.Zero; } } diff --git a/src/Java.Interop/Java.Interop/JavaObject.cs b/src/Java.Interop/Java.Interop/JavaObject.cs index fd3d68d3e..3c1c2b46d 100644 --- a/src/Java.Interop/Java.Interop/JavaObject.cs +++ b/src/Java.Interop/Java.Interop/JavaObject.cs @@ -25,13 +25,9 @@ unsafe public class JavaObject : IJavaPeerable [NonSerialized] JniObjectReference reference; #endif // FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES #if FEATURE_JNIOBJECTREFERENCE_INTPTRS - [NonSerialized] IntPtr handle; - [NonSerialized] JniObjectReferenceType handle_type; - #pragma warning disable 0169 - // Used by JavaInteropGCBridge - [NonSerialized] IntPtr weak_handle; - [NonSerialized] int refs_added; - #pragma warning restore 0169 + IntPtr jniObjectReferenceControlBlock; + unsafe JniObjectReferenceControlBlock* JniObjectReferenceControlBlock => + (JniObjectReferenceControlBlock*) jniObjectReferenceControlBlock; #endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS protected static readonly JniObjectReference* InvalidJniObjectReference = null; @@ -41,13 +37,17 @@ unsafe public class JavaObject : IJavaPeerable JniEnvironment.Runtime.ValueManager.FinalizePeer (this); } - public JniObjectReference PeerReference { + public unsafe JniObjectReference PeerReference { get { #if FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES return reference; #endif // FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES #if FEATURE_JNIOBJECTREFERENCE_INTPTRS - return new JniObjectReference (handle, handle_type); + var c = JniObjectReferenceControlBlock; + if (c == null) { + return default; + } + return new JniObjectReference (c->handle, (JniObjectReferenceType) c->handle_type); #endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS } } @@ -92,8 +92,14 @@ protected void SetPeerReference (ref JniObjectReference reference, JniObjectRefe this.reference = reference; #endif // FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES #if FEATURE_JNIOBJECTREFERENCE_INTPTRS - this.handle = reference.Handle; - this.handle_type = reference.Type; + var c = JniObjectReferenceControlBlock; + if (c == null) { + jniObjectReferenceControlBlock = + Java.Interop.JniObjectReferenceControlBlock.Alloc (); + c = JniObjectReferenceControlBlock; + } + c->handle = reference.Handle; + c->handle_type = (int) reference.Type; #endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS JniObjectReference.Dispose (ref reference, options); @@ -118,6 +124,9 @@ public virtual void DisposeUnlessReferenced () protected virtual void Dispose (bool disposing) { +#if FEATURE_JNIOBJECTREFERENCE_INTPTRS + Java.Interop.JniObjectReferenceControlBlock.Free (ref jniObjectReferenceControlBlock); +#endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS } public override bool Equals (object? obj) @@ -170,6 +179,9 @@ void IJavaPeerable.SetPeerReference (JniObjectReference reference) { SetPeerReference (ref reference, JniObjectReferenceOptions.Copy); } + + IntPtr IJavaPeerable.JniObjectReferenceControlBlock => + jniObjectReferenceControlBlock; } } diff --git a/src/Java.Interop/PublicAPI.Unshipped.txt b/src/Java.Interop/PublicAPI.Unshipped.txt index 90cde5ae6..90fda8ff8 100644 --- a/src/Java.Interop/PublicAPI.Unshipped.txt +++ b/src/Java.Interop/PublicAPI.Unshipped.txt @@ -11,3 +11,4 @@ Java.Interop.JniRuntime.JniTypeManager.GetInvokerType(System.Type! type) -> Syst Java.Interop.JniRuntime.JniValueManager.GetPeer(Java.Interop.JniObjectReference reference, System.Type? targetType = null) -> Java.Interop.IJavaPeerable? Java.Interop.JniTypeSignatureAttribute.InvokerType.get -> System.Type? Java.Interop.JniTypeSignatureAttribute.InvokerType.set -> void +Java.Interop.IJavaPeerable.JniObjectReferenceControlBlock.get -> nint From 6aa7216003d70b7e56d5253f32b56f3cedf1a141 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Mon, 12 May 2025 07:28:47 -0400 Subject: [PATCH 2/8] Doh! --- .../JniObjectReferenceControlBlock.cs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs diff --git a/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs b/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs new file mode 100644 index 000000000..eb7c91b7c --- /dev/null +++ b/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs @@ -0,0 +1,25 @@ +using System; +using System.Runtime.InteropServices; + +namespace Java.Interop; + +internal struct JniObjectReferenceControlBlock { + public IntPtr handle; + public int handle_type; + public IntPtr weak_handle; + public int refs_added; + + public static readonly int Size = Marshal.SizeOf(); + + public static unsafe IntPtr Alloc () => + (IntPtr) NativeMemory.AllocZeroed (1, (uint) Size); + + public static unsafe void Free (ref IntPtr value) + { + if (value == 0) { + return; + } + NativeMemory.Free ((void*) value); + value = IntPtr.Zero; + } +} From 530527b0a3f4a5a476e04ed1a3064e3a8935b8d4 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Mon, 12 May 2025 07:36:12 -0400 Subject: [PATCH 3/8] Add a JniObjectReferenceControlBlock to JavaException --- .../Java.Interop/JavaException.cs | 29 ++++++++++++------- src/Java.Interop/Java.Interop/JavaObject.cs | 13 ++++----- .../JniObjectReferenceControlBlock.cs | 14 +++++---- 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/Java.Interop/Java.Interop/JavaException.cs b/src/Java.Interop/Java.Interop/JavaException.cs index b07735b5a..43cffa228 100644 --- a/src/Java.Interop/Java.Interop/JavaException.cs +++ b/src/Java.Interop/Java.Interop/JavaException.cs @@ -18,13 +18,7 @@ unsafe public class JavaException : Exception, IJavaPeerable JniObjectReference reference; #endif // FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES #if FEATURE_JNIOBJECTREFERENCE_INTPTRS - IntPtr handle; - JniObjectReferenceType handle_type; - #pragma warning disable 0169 - // Used by JavaInteropGCBridge - IntPtr weak_handle; - int refs_added; - #pragma warning restore 0169 + unsafe JniObjectReferenceControlBlock* jniObjectReferenceControlBlock; #endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS protected static readonly JniObjectReference* InvalidJniObjectReference = null; @@ -106,7 +100,11 @@ public JniObjectReference PeerReference { return reference; #endif // FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES #if FEATURE_JNIOBJECTREFERENCE_INTPTRS - return new JniObjectReference (handle, handle_type); + var c = jniObjectReferenceControlBlock; + if (c == null) { + return default; + } + return new JniObjectReference (c->handle, (JniObjectReferenceType) c->handle_type); #endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS } } @@ -137,8 +135,13 @@ protected void SetPeerReference (ref JniObjectReference reference, JniObjectRefe this.reference = reference; #endif // FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES #if FEATURE_JNIOBJECTREFERENCE_INTPTRS - this.handle = reference.Handle; - this.handle_type = reference.Type; + var c = jniObjectReferenceControlBlock; + if (c == null) { + c = jniObjectReferenceControlBlock = + Java.Interop.JniObjectReferenceControlBlock.Alloc (); + } + c->handle = reference.Handle; + c->handle_type = (int) reference.Type; #endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS JniObjectReference.Dispose (ref reference, options); @@ -167,6 +170,9 @@ protected virtual void Dispose (bool disposing) if (inner != null) { inner.Dispose (); } +#if FEATURE_JNIOBJECTREFERENCE_INTPTRS + Java.Interop.JniObjectReferenceControlBlock.Free (ref jniObjectReferenceControlBlock); +#endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS } public override bool Equals (object? obj) @@ -282,6 +288,9 @@ void IJavaPeerable.SetPeerReference (JniObjectReference reference) { SetPeerReference (ref reference, JniObjectReferenceOptions.Copy); } + + IntPtr IJavaPeerable.JniObjectReferenceControlBlock => + (IntPtr) jniObjectReferenceControlBlock; } } diff --git a/src/Java.Interop/Java.Interop/JavaObject.cs b/src/Java.Interop/Java.Interop/JavaObject.cs index 3c1c2b46d..d0ea9bc62 100644 --- a/src/Java.Interop/Java.Interop/JavaObject.cs +++ b/src/Java.Interop/Java.Interop/JavaObject.cs @@ -25,9 +25,7 @@ unsafe public class JavaObject : IJavaPeerable [NonSerialized] JniObjectReference reference; #endif // FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES #if FEATURE_JNIOBJECTREFERENCE_INTPTRS - IntPtr jniObjectReferenceControlBlock; - unsafe JniObjectReferenceControlBlock* JniObjectReferenceControlBlock => - (JniObjectReferenceControlBlock*) jniObjectReferenceControlBlock; + unsafe JniObjectReferenceControlBlock* jniObjectReferenceControlBlock; #endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS protected static readonly JniObjectReference* InvalidJniObjectReference = null; @@ -43,7 +41,7 @@ public unsafe JniObjectReference PeerReference { return reference; #endif // FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES #if FEATURE_JNIOBJECTREFERENCE_INTPTRS - var c = JniObjectReferenceControlBlock; + var c = jniObjectReferenceControlBlock; if (c == null) { return default; } @@ -92,11 +90,10 @@ protected void SetPeerReference (ref JniObjectReference reference, JniObjectRefe this.reference = reference; #endif // FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES #if FEATURE_JNIOBJECTREFERENCE_INTPTRS - var c = JniObjectReferenceControlBlock; + var c = jniObjectReferenceControlBlock; if (c == null) { - jniObjectReferenceControlBlock = + c = jniObjectReferenceControlBlock = Java.Interop.JniObjectReferenceControlBlock.Alloc (); - c = JniObjectReferenceControlBlock; } c->handle = reference.Handle; c->handle_type = (int) reference.Type; @@ -181,7 +178,7 @@ void IJavaPeerable.SetPeerReference (JniObjectReference reference) } IntPtr IJavaPeerable.JniObjectReferenceControlBlock => - jniObjectReferenceControlBlock; + (IntPtr) jniObjectReferenceControlBlock; } } diff --git a/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs b/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs index eb7c91b7c..01871cafe 100644 --- a/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs +++ b/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs @@ -11,15 +11,17 @@ internal struct JniObjectReferenceControlBlock { public static readonly int Size = Marshal.SizeOf(); - public static unsafe IntPtr Alloc () => - (IntPtr) NativeMemory.AllocZeroed (1, (uint) Size); + public static unsafe JniObjectReferenceControlBlock* Alloc () + { + return (JniObjectReferenceControlBlock*)NativeMemory.AllocZeroed (1, (uint) Size); + } - public static unsafe void Free (ref IntPtr value) + public static unsafe void Free (ref JniObjectReferenceControlBlock* value) { - if (value == 0) { + if (value == null) { return; } - NativeMemory.Free ((void*) value); - value = IntPtr.Zero; + NativeMemory.Free (value); + value = null; } } From 645153b5edce044416f50d877fa0942ed06fac41 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Mon, 12 May 2025 11:57:19 -0400 Subject: [PATCH 4/8] Update MonoVM GC bridge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It works! Build: dotnet build -c Release -p:DotNetTargetFramework=net8.0 -t:Prepare dotnet build -c Release -p:DotNetTargetFramework=net8.0 dotnet publish --self-contained -p:UseMonoRuntime=true -p:DotNetTargetFramework=net8.0 \ -p:UseAppHost=true -p:ErrorOnDuplicatePublishOutputFiles=false \ samples/Hello-Java.Base/Hello-Java.Base.csproj -r osx-x64 Run: JAVA_INTEROP_GREF_LOG=g.txt ./samples/Hello-Java.Base/bin/Release/osx-x64/publish/Hello-Java.Base `g.txt` contains: … *take_weak obj=0x10560dc70; handle=0x7f7f42f15148 +w+ grefc 8 gwrefc 1 obj-handle 0x7f7f42f15148/G -> new-handle 0x7f7f43008401/W from thread '(null)'(1) take_weak_global_ref_jni -g- grefc 7 gwrefc 1 handle 0x7f7f42f15148/G from thread '(null)'(1) take_weak_global_ref_jni *try_take_global obj=0x10560dc70 -> wref=0x7f7f43008401 handle=0x0 -w- grefc 7 gwrefc 0 handle 0x7f7f43008401/W from thread '(null)'(1) take_global_ref_jni Finalizing PeerReference=0x0/G IdentityHashCode=0x70dea4e Instance=0x59b98e2b Instance.Type=Hello.MyJLO --- .../JniObjectReferenceControlBlock.cs | 2 +- .../java-interop-gc-bridge-mono.cc | 138 +++++++++--------- 2 files changed, 72 insertions(+), 68 deletions(-) diff --git a/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs b/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs index 01871cafe..348e08cbd 100644 --- a/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs +++ b/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs @@ -13,7 +13,7 @@ internal struct JniObjectReferenceControlBlock { public static unsafe JniObjectReferenceControlBlock* Alloc () { - return (JniObjectReferenceControlBlock*)NativeMemory.AllocZeroed (1, (uint) Size); + return (JniObjectReferenceControlBlock*) NativeMemory.AllocZeroed (1, (uint) Size); } public static unsafe void Free (ref JniObjectReferenceControlBlock* value) diff --git a/src/java-interop/java-interop-gc-bridge-mono.cc b/src/java-interop/java-interop-gc-bridge-mono.cc index aca306c60..05be2dee2 100644 --- a/src/java-interop/java-interop-gc-bridge-mono.cc +++ b/src/java-interop/java-interop-gc-bridge-mono.cc @@ -30,12 +30,16 @@ using namespace xamarin::android; typedef struct MonoJavaGCBridgeInfo { MonoClass *klass; - MonoClassField *handle; - MonoClassField *handle_type; - MonoClassField *refs_added; - MonoClassField *weak_handle; + MonoClassField *jniObjectReferenceControlBlock; } MonoJavaGCBridgeInfo; +typedef struct JniObjectReferenceControlBlock { + jobject handle; + int handle_type; + jobject weak_handle; + int refs_added; +} JniObjectReferenceControlBlock; + #define NUM_GC_BRIDGE_TYPES (4) struct JavaInteropGCBridge { @@ -366,13 +370,10 @@ java_interop_gc_bridge_register_bridgeable_type ( MonoJavaGCBridgeInfo *info = &bridge->mono_java_gc_bridge_info [i]; info->klass = mono_class_from_mono_type (type); - info->handle = mono_class_get_field_from_name (info->klass, const_cast ("handle")); - info->handle_type = mono_class_get_field_from_name (info->klass, const_cast ("handle_type")); - info->refs_added = mono_class_get_field_from_name (info->klass, const_cast ("refs_added")); - info->weak_handle = mono_class_get_field_from_name (info->klass, const_cast ("weak_handle")); - if (info->klass == NULL || info->handle == NULL || info->handle_type == NULL || - info->refs_added == NULL || info->weak_handle == NULL) + info->jniObjectReferenceControlBlock = mono_class_get_field_from_name (info->klass, const_cast ("jniObjectReferenceControlBlock")); + + if (info->klass == NULL || info->jniObjectReferenceControlBlock == NULL) return -1; bridge->num_bridge_types++; return 0; @@ -780,6 +781,18 @@ get_gc_bridge_info_for_object (JavaInteropGCBridge *bridge, MonoObject *object) return get_gc_bridge_info_for_class (bridge, mono_object_get_class (object)); } +static JniObjectReferenceControlBlock* +get_gc_control_block_for_object (JavaInteropGCBridge *bridge, MonoObject *obj) +{ + MonoJavaGCBridgeInfo *bridge_info = get_gc_bridge_info_for_object (bridge, obj); + if (bridge_info == NULL) + return NULL; + + JniObjectReferenceControlBlock *control_block; + mono_field_get_value (obj, bridge_info->jniObjectReferenceControlBlock, &control_block); + return control_block; +} + typedef mono_bool (*MonodroidGCTakeRefFunc) (JavaInteropGCBridge *bridge, JNIEnv *env, MonoObject *obj, const char *thread_name, int64_t thread_id); static MonodroidGCTakeRefFunc take_global_ref; @@ -788,12 +801,11 @@ static MonodroidGCTakeRefFunc take_weak_global_ref; static mono_bool take_global_ref_java (JavaInteropGCBridge *bridge, JNIEnv *env, MonoObject *obj, const char *thread_name, int64_t thread_id) { - MonoJavaGCBridgeInfo *bridge_info = get_gc_bridge_info_for_object (bridge, obj); - if (bridge_info == NULL) + JniObjectReferenceControlBlock *control_block = get_gc_control_block_for_object (bridge, obj); + if (control_block == NULL) return 0; - jobject weak; - mono_field_get_value (obj, bridge_info->weak_handle, &weak); + jobject weak = control_block->weak_handle; jobject handle = env->CallObjectMethod (weak, bridge->WeakReference_get); log_gref (bridge, "*try_take_global_2_1 obj=%p -> wref=%p handle=%p\n", obj, weak, handle); @@ -808,12 +820,10 @@ take_global_ref_java (JavaInteropGCBridge *bridge, JNIEnv *env, MonoObject *obj, java_interop_gc_bridge_weak_gref_log_delete (bridge, weak, get_object_ref_type (env, weak), thread_name, thread_id, "take_global_ref_java"); env->DeleteGlobalRef (weak); weak = NULL; - mono_field_set_value (obj, bridge_info->weak_handle, &weak); - mono_field_set_value (obj, bridge_info->handle, &handle); - - int type = JNIGlobalRefType; - mono_field_set_value (obj, bridge_info->handle_type, &type); + control_block->weak_handle = weak; + control_block->handle = handle; + control_block->handle_type = JNIGlobalRefType; return handle != NULL; } @@ -821,12 +831,11 @@ take_global_ref_java (JavaInteropGCBridge *bridge, JNIEnv *env, MonoObject *obj, static mono_bool take_weak_global_ref_java (JavaInteropGCBridge *bridge, JNIEnv *env, MonoObject *obj, const char *thread_name, int64_t thread_id) { - MonoJavaGCBridgeInfo *bridge_info = get_gc_bridge_info_for_object (bridge, obj); - if (bridge_info == NULL) + JniObjectReferenceControlBlock *control_block = get_gc_control_block_for_object (bridge, obj); + if (control_block == NULL) return 0; - jobject handle; - mono_field_get_value (obj, bridge_info->handle, &handle); + jobject handle = control_block->handle; jobject weaklocal = env->NewObject (bridge->WeakReference_class, bridge->WeakReference_init, handle); jobject weakglobal = env->NewGlobalRef (weaklocal); @@ -838,8 +847,8 @@ take_weak_global_ref_java (JavaInteropGCBridge *bridge, JNIEnv *env, MonoObject java_interop_gc_bridge_gref_log_delete (bridge, handle, get_object_ref_type (env, handle), thread_name, thread_id, "take_weak_global_ref_2_1_compat"); env->DeleteGlobalRef (handle); - - mono_field_set_value (obj, bridge_info->weak_handle, &weakglobal); + control_block->handle = NULL; + control_block->weak_handle = weakglobal; return 1; } @@ -847,13 +856,11 @@ take_weak_global_ref_java (JavaInteropGCBridge *bridge, JNIEnv *env, MonoObject static mono_bool take_global_ref_jni (JavaInteropGCBridge *bridge, JNIEnv *env, MonoObject *obj, const char *thread_name, int64_t thread_id) { - MonoJavaGCBridgeInfo *bridge_info = get_gc_bridge_info_for_object (bridge, obj); - if (bridge_info == NULL) + JniObjectReferenceControlBlock *control_block = get_gc_control_block_for_object (bridge, obj); + if (control_block == NULL) return 0; - jobject weak; - mono_field_get_value (obj, bridge_info->handle, &weak); - + jobject weak = control_block->handle; jobject handle = env->NewGlobalRef (weak); log_gref (bridge, "*try_take_global obj=%p -> wref=%p handle=%p\n", obj, weak, handle); @@ -868,22 +875,20 @@ take_global_ref_jni (JavaInteropGCBridge *bridge, JNIEnv *env, MonoObject *obj, thread_name, thread_id, "take_global_ref_jni"); env->DeleteWeakGlobalRef (weak); - mono_field_set_value (obj, bridge_info->handle, &handle); + control_block->handle = handle; + control_block->handle_type = JNIGlobalRefType; - int type = JNIGlobalRefType; - mono_field_set_value (obj, bridge_info->handle_type, &type); return handle != NULL; } static mono_bool take_weak_global_ref_jni (JavaInteropGCBridge *bridge, JNIEnv *env, MonoObject *obj, const char *thread_name, int64_t thread_id) { - MonoJavaGCBridgeInfo *bridge_info = get_gc_bridge_info_for_object (bridge, obj); - if (bridge_info == NULL) + JniObjectReferenceControlBlock *control_block = get_gc_control_block_for_object (bridge, obj); + if (control_block == NULL) return 0; - jobject handle; - mono_field_get_value (obj, bridge_info->handle, &handle); + jobject handle = control_block->handle; log_gref (bridge, "*take_weak obj=%p; handle=%p\n", obj, handle); @@ -896,10 +901,8 @@ take_weak_global_ref_jni (JavaInteropGCBridge *bridge, JNIEnv *env, MonoObject * thread_name, thread_id, "take_weak_global_ref_jni"); env->DeleteGlobalRef (handle); - mono_field_set_value (obj, bridge_info->handle, &weak); - - int type = JNIWeakGlobalRefType; - mono_field_set_value (obj, bridge_info->handle_type, &type); + control_block->handle = weak; + control_block->handle_type = JNIWeakGlobalRefType; return 1; } @@ -921,17 +924,18 @@ get_add_reference_method (JavaInteropGCBridge *bridge, JNIEnv *env, jobject obj, } static mono_bool -add_reference (JavaInteropGCBridge *bridge, JNIEnv *env, MonoObject *obj, MonoJavaGCBridgeInfo *bridge_info, MonoObject *reffed_obj) +add_reference (JavaInteropGCBridge *bridge, JNIEnv *env, MonoObject *obj, JniObjectReferenceControlBlock *control_block, MonoObject *reffed_obj) { MonoClass *klass = mono_object_get_class (obj); - jobject handle; - mono_field_get_value (obj, bridge_info->handle, &handle); + jobject handle = control_block->handle; jmethodID add_method_id = get_add_reference_method (bridge, env, handle, klass); if (add_method_id) { - jobject reffed_handle; - mono_field_get_value (reffed_obj, bridge_info->handle, &reffed_handle); + JniObjectReferenceControlBlock *reffed_control_block = get_gc_control_block_for_object (bridge, reffed_obj); + if (reffed_control_block == NULL) + return 0; + jobject reffed_handle = reffed_control_block->handle; env->CallVoidMethod (handle, add_method_id, reffed_handle); #if DEBUG if (bridge->gref_log_level > 1) @@ -979,28 +983,30 @@ gc_prepare_for_java_collection (JavaInteropGCBridge *bridge, JNIEnv *env, int nu /* add java refs for items on the list which reference each other */ for (int i = 0; i < num_sccs; i++) { MonoGCBridgeSCC *scc = sccs [i]; - MonoJavaGCBridgeInfo *bridge_info = NULL; + + JniObjectReferenceControlBlock *control_block = NULL; + /* start at the second item, ref j from j-1 */ for (int j = 1; j < scc->num_objs; j++) { - bridge_info = get_gc_bridge_info_for_object (bridge, scc->objs [j-1]); - if (bridge_info != NULL && add_reference (bridge, env, scc->objs [j-1], bridge_info, scc->objs [j])) { - mono_field_set_value (scc->objs [j-1], bridge_info->refs_added, &ref_val); + control_block = get_gc_control_block_for_object (bridge, scc->objs [j-1]); + if (control_block != NULL && add_reference (bridge, env, scc->objs [j-1], control_block, scc->objs [j])) { + control_block->refs_added = ref_val; } } /* ref the first from the last */ if (scc->num_objs > 1) { - bridge_info = get_gc_bridge_info_for_object (bridge, scc->objs [scc->num_objs-1]); - if (bridge_info != NULL && add_reference (bridge, env, scc->objs [scc->num_objs-1], bridge_info, scc->objs [0])) { - mono_field_set_value (scc->objs [scc->num_objs-1], bridge_info->refs_added, &ref_val); + control_block = get_gc_control_block_for_object (bridge, scc->objs [scc->num_objs-1]); + if (control_block != NULL && add_reference (bridge, env, scc->objs [scc->num_objs-1], control_block, scc->objs [0])) { + control_block->refs_added = ref_val; } } } /* add the cross scc refs */ for (int i = 0; i < num_xrefs; i++) { - MonoJavaGCBridgeInfo *bridge_info = get_gc_bridge_info_for_object (bridge, sccs [xrefs [i].src_scc_index]->objs [0]); - if (bridge_info != NULL && add_reference (bridge, env, sccs [xrefs [i].src_scc_index]->objs [0], bridge_info, sccs [xrefs [i].dst_scc_index]->objs [0])) { - mono_field_set_value (sccs [xrefs [i].src_scc_index]->objs [0], bridge_info->refs_added, &ref_val); + JniObjectReferenceControlBlock *control_block = get_gc_control_block_for_object (bridge, sccs [xrefs [i].src_scc_index]->objs [0]); + if (control_block != NULL && add_reference (bridge, env, sccs [xrefs [i].src_scc_index]->objs [0], control_block, sccs [xrefs [i].dst_scc_index]->objs [0])) { + control_block->refs_added = ref_val; } } @@ -1041,19 +1047,18 @@ gc_cleanup_after_java_collection (JavaInteropGCBridge *bridge, JNIEnv *env, int sccs [i]->is_alive = 0; for (int j = 0; j < sccs [i]->num_objs; j++) { MonoObject *obj = sccs [i]->objs [j]; - MonoJavaGCBridgeInfo *bridge_info = get_gc_bridge_info_for_object (bridge, obj); - if (bridge_info == NULL) + + JniObjectReferenceControlBlock *control_block = get_gc_control_block_for_object (bridge, obj); + if (control_block == NULL) continue; - jobject jref; - mono_field_get_value (obj, bridge_info->handle, &jref); + jobject jref = control_block->handle; if (jref) { alive++; if (j > 0) assert (sccs [i]->is_alive); sccs [i]->is_alive = 1; - int refs_added; - mono_field_get_value (obj, bridge_info->refs_added, &refs_added); + int refs_added = control_block->refs_added; if (refs_added) { jmethodID clear_method_id = get_clear_references_method (bridge, env, jref); if (clear_method_id) { @@ -1201,13 +1206,12 @@ gc_bridge_class_kind (MonoClass *klass) static mono_bool gc_is_bridge_object (MonoObject *object) { - void *handle; - MonoJavaGCBridgeInfo *bridge_info = get_gc_bridge_info_for_object (mono_bridge, object); - if (bridge_info == NULL) - return 0; + JniObjectReferenceControlBlock *control_block = get_gc_control_block_for_object (mono_bridge, object); + if (control_block == NULL) + return 0;; - mono_field_get_value (object, bridge_info->handle, &handle); + void *handle = control_block->handle; if (handle == NULL) { #if DEBUG MonoClass *mclass = mono_object_get_class (object); From e42be6c571f705a6bec39ca23cd10a446c8e0677 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 13 May 2025 07:17:54 -0400 Subject: [PATCH 5/8] Fix GREF leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A "funny" thing happened after 628aa39d: a teardown assertion fired! % dotnet test bin/TestRelease-net8.0/Java.Interop-Tests.dll … # jonp: LoadJvmLibrary(/Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.15-6/x64/Contents/Home/lib/libjli.dylib)=140707427245328 # jonp: JNI_CreateJavaVM=7113469040; JNI_GetCreatedJavaVMs=7113469120 # jonp: executing JNI_CreateJavaVM=1a7feec70 # jonp: r=0 javavm=1a9dc1830 jnienv=7fe37d0422b0 TearDown failed for test fixture Java.InteropTests.JavaObjectArray_object_ContractTest JNI global references: grefStartCount=324; gref=340 Expected: True But was: False TearDown : NUnit.Framework.AssertionException : JNI global references: grefStartCount=324; gref=340 Expected: True But was: False StackTrace: at Java.InteropTests.JavaObjectArray_object_ContractTest.EndCheckGlobalRefCount() in /Users/runner/work/1/s/tests/Java.Interop-Tests/Java.Interop/JavaObjectArrayTest.cs:line 158 --TearDown at NUnit.Framework.Assert.ReportFailure(String message) at NUnit.Framework.Assert.ReportFailure(ConstraintResult result, String message, Object[] args) at NUnit.Framework.Assert.That[TActual](TActual actual, IResolveConstraint expression, String message, Object[] args) at NUnit.Framework.Assert.IsTrue(Boolean condition, String message, Object[] args) at Java.InteropTests.JavaObjectArray_object_ContractTest.EndCheckGlobalRefCount() in /Users/runner/work/1/s/tests/Java.Interop-Tests/Java.Interop/JavaObjectArrayTest.cs:line 158 at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor) at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr) Skipped References_CreatedReference_GlobalRef [3 ms] Skipped References_CreatedReference_LocalRef [< 1 ms] Skipped DoesTheJmethodNeedToMatchDeclaringType [5 ms] # jonp: LoadJvmLibrary(/Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.15-6/x64/Contents/Home/lib/libjli.dylib)=140707427245328 # jonp: JNI_CreateJavaVM=7113469040; JNI_GetCreatedJavaVMs=7113469120 # jonp: executing JNI_CreateJavaVM=1a7feec70 # jonp: r=-5 javavm=0 jnienv=0 Unfortunately, teardown assertions "don't count", i.e. CI was green, even though the above is copied from the CI run. The cause of the problem? Having `JavaObject.Dispose()` call `JniObjectReferenceControlBlock.Free()`. The underlying problem is that `JniRuntime.JniValueManager.DisposePeer()` uses `JavaObject.PeerReference` *after* calling `JavaObject.Disposed()` (which calls `JavaObject.Dispose(bool)`), yet `JavaObject.Dispose(bool)` released the `JniObjectReferenceControlBlock` that backs `PeerReference`, meaning we "lost" (leaked!) the GREF! There are two possible solutions: 1. Update `JniRuntime.JniValueManager.DisposePeer()` to use `JavaObject.PeerReference` *before* calling `JavaObject.Disposed()` 2. Update `JavaObject.Dispose(bool)` to not release the `JniObjectReferenceControlBlock` that backs `PeerReference`. I avoided (1) because I didn't want to have to audit and update dotnet/android and all other potential callsites. Which brings us to (2): if not in `Dispose(bool)`, then where? The last thing that `JniRuntime.JniValueManager.DisposePeer()` and `JniRuntime.JniValueManager.FinalizePeer()` do is call `JavaObject.SetPeerReference(default)`. *This*, then, is where cleanup needs to happen. Update `JavaObject` and `JavaException` to use the `JniManagedPeerStates` field to track "have I been disposed?", and then update `SetPeerReference()` to call `JniObjectReferenceControlBlock.Free()` if the state is "disposed". This fixes the leak. --- .../Java.Interop/JavaException.cs | 12 ++++++---- src/Java.Interop/Java.Interop/JavaObject.cs | 13 ++++++---- .../Java.Interop/JniManagedPeerStates.cs | 8 +++++++ .../Java.Interop/JavaObjectArrayTest.cs | 24 +++++++++++++++++++ 4 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/Java.Interop/Java.Interop/JavaException.cs b/src/Java.Interop/Java.Interop/JavaException.cs index 43cffa228..765afc4ea 100644 --- a/src/Java.Interop/Java.Interop/JavaException.cs +++ b/src/Java.Interop/Java.Interop/JavaException.cs @@ -5,7 +5,7 @@ namespace Java.Interop { [JniTypeSignature (JniTypeName, GenerateJavaPeer=false)] - unsafe public class JavaException : Exception, IJavaPeerable + unsafe public partial class JavaException : Exception, IJavaPeerable { internal const string JniTypeName = "java/lang/Throwable"; readonly static JniPeerMembers _members = new JniPeerMembers (JniTypeName, typeof (JavaException)); @@ -170,9 +170,6 @@ protected virtual void Dispose (bool disposing) if (inner != null) { inner.Dispose (); } -#if FEATURE_JNIOBJECTREFERENCE_INTPTRS - Java.Interop.JniObjectReferenceControlBlock.Free (ref jniObjectReferenceControlBlock); -#endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS } public override bool Equals (object? obj) @@ -266,11 +263,13 @@ protected void SetJavaStackTrace (JniObjectReference peerReferenceOverride = def void IJavaPeerable.Disposed () { + JniManagedPeerState |= Disposed; Dispose (disposing: true); } void IJavaPeerable.Finalized () { + JniManagedPeerState |= Disposed; Dispose (disposing: false); } @@ -287,6 +286,11 @@ void IJavaPeerable.SetJniManagedPeerState (JniManagedPeerStates value) void IJavaPeerable.SetPeerReference (JniObjectReference reference) { SetPeerReference (ref reference, JniObjectReferenceOptions.Copy); +#if FEATURE_JNIOBJECTREFERENCE_INTPTRS + if (!reference.IsValid && JniManagedPeerState.HasFlag (Disposed)) { + Java.Interop.JniObjectReferenceControlBlock.Free (ref jniObjectReferenceControlBlock); + } +#endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS } IntPtr IJavaPeerable.JniObjectReferenceControlBlock => diff --git a/src/Java.Interop/Java.Interop/JavaObject.cs b/src/Java.Interop/Java.Interop/JavaObject.cs index d0ea9bc62..fc7e00e10 100644 --- a/src/Java.Interop/Java.Interop/JavaObject.cs +++ b/src/Java.Interop/Java.Interop/JavaObject.cs @@ -8,7 +8,7 @@ namespace Java.Interop { [JniTypeSignature ("java/lang/Object", GenerateJavaPeer=false)] [Serializable] - unsafe public class JavaObject : IJavaPeerable + unsafe public partial class JavaObject : IJavaPeerable { internal const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors; @@ -121,9 +121,6 @@ public virtual void DisposeUnlessReferenced () protected virtual void Dispose (bool disposing) { -#if FEATURE_JNIOBJECTREFERENCE_INTPTRS - Java.Interop.JniObjectReferenceControlBlock.Free (ref jniObjectReferenceControlBlock); -#endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS } public override bool Equals (object? obj) @@ -154,11 +151,13 @@ public override unsafe int GetHashCode () void IJavaPeerable.Disposed () { + managedPeerState |= Disposed; Dispose (disposing: true); } void IJavaPeerable.Finalized () { + managedPeerState |= Disposed; Dispose (disposing: false); } @@ -175,6 +174,12 @@ void IJavaPeerable.SetJniManagedPeerState (JniManagedPeerStates value) void IJavaPeerable.SetPeerReference (JniObjectReference reference) { SetPeerReference (ref reference, JniObjectReferenceOptions.Copy); + +#if FEATURE_JNIOBJECTREFERENCE_INTPTRS + if (!reference.IsValid && managedPeerState.HasFlag (Disposed)) { + Java.Interop.JniObjectReferenceControlBlock.Free (ref jniObjectReferenceControlBlock); + } +#endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS } IntPtr IJavaPeerable.JniObjectReferenceControlBlock => diff --git a/src/Java.Interop/Java.Interop/JniManagedPeerStates.cs b/src/Java.Interop/Java.Interop/JniManagedPeerStates.cs index 765fb0b2d..b4f1fba3c 100644 --- a/src/Java.Interop/Java.Interop/JniManagedPeerStates.cs +++ b/src/Java.Interop/Java.Interop/JniManagedPeerStates.cs @@ -11,4 +11,12 @@ public enum JniManagedPeerStates { /// Replaceable = (1 << 1), } + + partial class JavaObject { + const JniManagedPeerStates Disposed = (JniManagedPeerStates) (1 << 2); + } + + partial class JavaException { + const JniManagedPeerStates Disposed = (JniManagedPeerStates) (1 << 2); + } } diff --git a/tests/Java.Interop-Tests/Java.Interop/JavaObjectArrayTest.cs b/tests/Java.Interop-Tests/Java.Interop/JavaObjectArrayTest.cs index f02bbc06b..896c1c9a6 100644 --- a/tests/Java.Interop-Tests/Java.Interop/JavaObjectArrayTest.cs +++ b/tests/Java.Interop-Tests/Java.Interop/JavaObjectArrayTest.cs @@ -39,6 +39,30 @@ protected override bool SequenceEqual (IEnumerable a, IEnumerable b) { return JniMarshal.RecursiveEquals (a, b); } + + int beforeTestGrefCount; + + [SetUp] + public void LogBeginCurrentTestGrefCount () + { + beforeTestGrefCount = JniEnvironment.Runtime.GlobalReferenceCount; + JniEnvironment.Runtime.ObjectReferenceManager.WriteGlobalReferenceLine( + "{0}", + $"Begin {TestContext.CurrentContext.Test.FullName}; " + + $"GREFs={beforeTestGrefCount}; " + ); + } + + [TearDown] + public void LogEndCurrentTestToGrefCount () + { + int afterTestGrefCount = JniEnvironment.Runtime.GlobalReferenceCount; + JniEnvironment.Runtime.ObjectReferenceManager.WriteGlobalReferenceLine( + "{0}", + $"End {TestContext.CurrentContext.Test.FullName}; " + + $"GREFs={afterTestGrefCount}; diff={afterTestGrefCount - beforeTestGrefCount}" + ); + } } [TestFixture] From abb0e940ae3c7850382d92af41fe28911c9d884f Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 13 May 2025 11:19:32 -0400 Subject: [PATCH 6/8] Add JavaBridgedValueManager Context: https://github.com/dotnet/runtime/issues/115506 Context: https://github.com/dotnet/android/pull/10125 We have an *API*, but not (yet) usable *implementation*. "Import" the `ManagedValueManager` from dotnet/android#10125, renaming to `JavaBridgedValueManager`, and add the proposed bridge API from dotnet/runtime#115506 to verify that it all compiles. It compiles! Next step: does it *work*? If I squint right, the proposed API looks very very similar to the existing MonoVM GC bridge API. Can I implement the proposed API in terms of MonoVM, and then have C# code perform the bridge code instead of native code, when using MonoVM? Let's find out! --- .../Java.Interop/JavaBridgedValueManager.cs | 318 ++++++++++++++++++ 1 file changed, 318 insertions(+) create mode 100644 src/Java.Runtime.Environment/Java.Interop/JavaBridgedValueManager.cs diff --git a/src/Java.Runtime.Environment/Java.Interop/JavaBridgedValueManager.cs b/src/Java.Runtime.Environment/Java.Interop/JavaBridgedValueManager.cs new file mode 100644 index 000000000..064b6758e --- /dev/null +++ b/src/Java.Runtime.Environment/Java.Interop/JavaBridgedValueManager.cs @@ -0,0 +1,318 @@ +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Runtime.InteropServices.Java; +using System.Threading; + +using Java.Interop; + +namespace System.Runtime.InteropServices.Java { + // https://github.com/dotnet/runtime/issues/115506 + + public struct ComponentCrossReference { + public nint SourceGroupIndex; + public nint DestinationGroupIndex; + } + + public unsafe struct StronglyConnectedComponent { + public nint Count; + public System.IntPtr* Context; + } + + public unsafe struct MarkCrossReferences { + public nint ComponentsLen; + public StronglyConnectedComponent* Components; + public nint CrossReferencesLen; + public ComponentCrossReference* CrossReferences; + } + + static class JavaMarshal { + public static unsafe void Initialize(delegate* unmanaged markCrossReferences) => + throw null!; + + public static GCHandle CreateReferenceTrackingHandle(object obj, IntPtr context) => + throw null!; + + public static IntPtr GetContext(GCHandle obj) => + throw null!; + + public static unsafe void ReleaseMarkCrossReferenceResources(MarkCrossReferences* crossReferences) => + throw null!; + } +} + +namespace Java.Interop { + + class JavaBridgedValueManager : JniRuntime.JniValueManager + { + Dictionary>? RegisteredInstances = new (); + + internal unsafe JavaBridgedValueManager () + { + JavaMarshal.Initialize (&MarkCrossReferences); + } + + public override void WaitForGCBridgeProcessing () + { + } + + 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 { + if (peer.Target is IDisposable disposable) + disposable.Dispose (); + } + catch (Exception e) { + exceptions = exceptions ?? new List (); + exceptions.Add (e); + } + } + if (exceptions != null) + throw new AggregateException ("Exceptions while collecting peers.", exceptions); + } + + public override void AddPeer (IJavaPeerable value) + { + if (RegisteredInstances == null) + throw new ObjectDisposedException (nameof (ManagedValueManager)); + + var r = value.PeerReference; + if (!r.IsValid) + throw new ObjectDisposedException (value.GetType ().FullName); + + if (r.Type != JniObjectReferenceType.Global) { + value.SetPeerReference (r.NewGlobalRef ()); + JniObjectReference.Dispose (ref r, JniObjectReferenceOptions.CopyAndDispose); + } + int key = value.JniIdentityHashCode; + lock (RegisteredInstances) { + List? peers; + if (!RegisteredInstances.TryGetValue (key, out peers)) { + peers = new List () { + CreateReferenceTrackingHandle (value) + }; + RegisteredInstances.Add (key, peers); + return; + } + + for (int i = peers.Count - 1; i >= 0; i--) { + var p = peers [i]; + if (p.Target is not IJavaPeerable peer) + continue; + if (!JniEnvironment.Types.IsSameObject (peer.PeerReference, value.PeerReference)) + continue; + if (Replaceable (p)) { + peers [i] = CreateReferenceTrackingHandle (value); + } else { + WarnNotReplacing (key, value, peer); + } + return; + } + peers.Add (CreateReferenceTrackingHandle (value)); + } + } + + static bool Replaceable (GCHandle handle) + { + if (handle.Target is not IJavaPeerable peer) + return true; + return peer.JniManagedPeerState.HasFlag (JniManagedPeerStates.Replaceable); + } + + void WarnNotReplacing (int key, IJavaPeerable ignoreValue, IJavaPeerable keepValue) + { + Runtime.ObjectReferenceManager.WriteGlobalReferenceLine ( + "Warning: Not registering PeerReference={0} IdentityHashCode=0x{1} Instance={2} Instance.Type={3} Java.Type={4}; " + + "keeping previously registered PeerReference={5} Instance={6} Instance.Type={7} Java.Type={8}.", + ignoreValue.PeerReference.ToString (), + key.ToString ("x", CultureInfo.InvariantCulture), + RuntimeHelpers.GetHashCode (ignoreValue).ToString ("x", CultureInfo.InvariantCulture), + ignoreValue.GetType ().FullName, + JniEnvironment.Types.GetJniTypeNameFromInstance (ignoreValue.PeerReference), + keepValue.PeerReference.ToString (), + RuntimeHelpers.GetHashCode (keepValue).ToString ("x", CultureInfo.InvariantCulture), + keepValue.GetType ().FullName, + JniEnvironment.Types.GetJniTypeNameFromInstance (keepValue.PeerReference)); + } + + public override IJavaPeerable? PeekPeer (JniObjectReference reference) + { + if (RegisteredInstances == null) + throw new ObjectDisposedException (nameof (ManagedValueManager)); + + if (!reference.IsValid) + return null; + + int key = GetJniIdentityHashCode (reference); + + lock (RegisteredInstances) { + List? peers; + if (!RegisteredInstances.TryGetValue (key, out peers)) + return null; + + for (int i = peers.Count - 1; i >= 0; i--) { + var p = peers [i]; + if (p.Target is IJavaPeerable peer && JniEnvironment.Types.IsSameObject (reference, peer.PeerReference)) + return peer; + } + if (peers.Count == 0) + RegisteredInstances.Remove (key); + } + return null; + } + + public override void RemovePeer (IJavaPeerable value) + { + if (RegisteredInstances == null) + throw new ObjectDisposedException (nameof (ManagedValueManager)); + + if (value == null) + throw new ArgumentNullException (nameof (value)); + + int key = value.JniIdentityHashCode; + lock (RegisteredInstances) { + List? peers; + if (!RegisteredInstances.TryGetValue (key, out peers)) + return; + + for (int i = peers.Count - 1; i >= 0; i--) { + var p = peers [i]; + if (object.ReferenceEquals (value, p.Target)) { + peers.RemoveAt (i); + FreeHandle (p); + } + } + if (peers.Count == 0) + RegisteredInstances.Remove (key); + } + } + + public override void FinalizePeer (IJavaPeerable value) + { + var h = value.PeerReference; + var o = Runtime.ObjectReferenceManager; + // MUST NOT use SafeHandle.ReferenceType: local refs are tied to a JniEnvironment + // and the JniEnvironment's corresponding thread; it's a thread-local value. + // Accessing SafeHandle.ReferenceType won't kill anything (so far...), but + // instead it always returns JniReferenceType.Invalid. + if (!h.IsValid || h.Type == JniObjectReferenceType.Local) { + if (o.LogGlobalReferenceMessages) { + o.WriteGlobalReferenceLine ("Finalizing PeerReference={0} IdentityHashCode=0x{1} Instance=0x{2} Instance.Type={3}", + h.ToString (), + value.JniIdentityHashCode.ToString ("x", CultureInfo.InvariantCulture), + RuntimeHelpers.GetHashCode (value).ToString ("x", CultureInfo.InvariantCulture), + value.GetType ().ToString ()); + } + RemovePeer (value); + value.SetPeerReference (new JniObjectReference ()); + value.Finalized (); + return; + } + + RemovePeer (value); + if (o.LogGlobalReferenceMessages) { + o.WriteGlobalReferenceLine ("Finalizing PeerReference={0} IdentityHashCode=0x{1} Instance=0x{2} Instance.Type={3}", + h.ToString (), + value.JniIdentityHashCode.ToString ("x", CultureInfo.InvariantCulture), + RuntimeHelpers.GetHashCode (value).ToString ("x", CultureInfo.InvariantCulture), + value.GetType ().ToString ()); + } + value.SetPeerReference (new JniObjectReference ()); + JniObjectReference.Dispose (ref h); + value.Finalized (); + } + + public override void ActivatePeer (IJavaPeerable? self, JniObjectReference reference, ConstructorInfo cinfo, object?[]? argumentValues) + { + try { + ActivateViaReflection (reference, cinfo, argumentValues); + } catch (Exception e) { + var m = string.Format ( + CultureInfo.InvariantCulture, + "Could not activate {{ PeerReference={0} IdentityHashCode=0x{1} Java.Type={2} }} for managed type '{3}'.", + reference, + GetJniIdentityHashCode (reference).ToString ("x", CultureInfo.InvariantCulture), + JniEnvironment.Types.GetJniTypeNameFromInstance (reference), + cinfo.DeclaringType?.FullName); + Debug.WriteLine (m); + + throw new NotSupportedException (m, e); + } + } + + void ActivateViaReflection (JniObjectReference reference, ConstructorInfo cinfo, object?[]? argumentValues) + { + var declType = GetDeclaringType (cinfo); + + #pragma warning disable IL2072 + var self = (IJavaPeerable) System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject (declType); + #pragma warning restore IL2072 + self.SetPeerReference (reference); + + cinfo.Invoke (self, argumentValues); + + [UnconditionalSuppressMessage ("Trimming", "IL2073", Justification = "🤷‍♂️")] + [return: DynamicallyAccessedMembers (Constructors)] + Type GetDeclaringType (ConstructorInfo cinfo) => + cinfo.DeclaringType ?? throw new NotSupportedException ("Do not know the type to create!"); + } + + public override List GetSurfacedPeers () + { + if (RegisteredInstances == null) + throw new ObjectDisposedException (nameof (ManagedValueManager)); + + lock (RegisteredInstances) { + var peers = new List (RegisteredInstances.Count); + foreach (var e in RegisteredInstances) { + foreach (var p in e.Value) { + if (p.Target is not IJavaPeerable peer) + continue; + peers.Add (new JniSurfacedPeerInfo (e.Key, new WeakReference (peer))); + } + } + return peers; + } + } + + static GCHandle CreateReferenceTrackingHandle (IJavaPeerable value) => + JavaMarshal.CreateReferenceTrackingHandle (value, value.JniObjectReferenceControlBlock); + + static unsafe void FreeHandle (GCHandle handle) + { + IntPtr context = JavaMarshal.GetContext (handle); + NativeMemory.Free ((void*) context); + } + + [UnmanagedCallersOnly] + internal static unsafe void MarkCrossReferences (MarkCrossReferences* crossReferences) + { + // Java.Lang.JavaSystem.Gc (); + + JavaMarshal.ReleaseMarkCrossReferenceResources (crossReferences); + } + } +} From 7174669897efbf38fcf9a340d3127b65a4b8d352 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 13 May 2025 13:16:55 -0400 Subject: [PATCH 7/8] Implement MonoVM BC Bridge :: Java Marshaling API "thunk" Commit 9e9daf49006f48a89c882e4e5c3ee0584fd24e89 suggested that we could probably "thunk" the MonoVM API to implement the proposed Java Bridge API.. Implement the thunk! Next up: implementing `markCrossReferences` in C#! --- .../Java.Interop/JavaBridgedValueManager.cs | 1 + .../Java.Interop/MonoRuntimeValueManager.cs | 12 ++++ .../java-interop-gc-bridge-mono.cc | 71 ++++++++++++++++++- src/java-interop/java-interop-gc-bridge.h | 24 +++++++ 4 files changed, 107 insertions(+), 1 deletion(-) diff --git a/src/Java.Runtime.Environment/Java.Interop/JavaBridgedValueManager.cs b/src/Java.Runtime.Environment/Java.Interop/JavaBridgedValueManager.cs index 064b6758e..0c17d51bc 100644 --- a/src/Java.Runtime.Environment/Java.Interop/JavaBridgedValueManager.cs +++ b/src/Java.Runtime.Environment/Java.Interop/JavaBridgedValueManager.cs @@ -23,6 +23,7 @@ public struct ComponentCrossReference { } public unsafe struct StronglyConnectedComponent { + public int IsAlive; public nint Count; public System.IntPtr* Context; } diff --git a/src/Java.Runtime.Environment/Java.Interop/MonoRuntimeValueManager.cs b/src/Java.Runtime.Environment/Java.Interop/MonoRuntimeValueManager.cs index 4cae1dc48..77a517ae0 100644 --- a/src/Java.Runtime.Environment/Java.Interop/MonoRuntimeValueManager.cs +++ b/src/Java.Runtime.Environment/Java.Interop/MonoRuntimeValueManager.cs @@ -415,6 +415,18 @@ partial class JreNativeMethods { [DllImport (JavaInteropLib, CallingConvention=CallingConvention.Cdecl)] internal static extern void java_interop_gc_bridge_wait_for_bridge_processing (IntPtr bridge); + + [DllImport (JavaInteropLib, CallingConvention=CallingConvention.Cdecl)] + internal static extern unsafe int java_interop_gc_bridge_set_mark_cross_references ( + IntPtr bridge, + delegate* unmanaged[Cdecl] markCrossReferences + ); + + [DllImport (JavaInteropLib, CallingConvention=CallingConvention.Cdecl)] + internal static extern unsafe int java_interop_gc_bridge_release_mark_cross_references_resources ( + IntPtr bridge, + System.Runtime.InteropServices.Java.MarkCrossReferences* crossReferences + ); } sealed class OverrideStackTrace : Exception { diff --git a/src/java-interop/java-interop-gc-bridge-mono.cc b/src/java-interop/java-interop-gc-bridge-mono.cc index 05be2dee2..fd7e0337a 100644 --- a/src/java-interop/java-interop-gc-bridge-mono.cc +++ b/src/java-interop/java-interop-gc-bridge-mono.cc @@ -76,6 +76,8 @@ struct JavaInteropGCBridge { char *gref_path, *lref_path; int gref_log_level, lref_log_level; int gref_cleanup, lref_cleanup; + + JavaInteropMarkCrossReferencesCallback mark_cross_references; }; static jobject @@ -1283,6 +1285,46 @@ gc_cross_references (int num_sccs, MonoGCBridgeSCC **sccs, int num_xrefs, MonoGC free (thread_name); } +static void +managed_gc_cross_references (int num_sccs, MonoGCBridgeSCC **sccs, int num_xrefs, MonoGCBridgeXRef *xrefs) +{ + if (mono_bridge->mark_cross_references == NULL) { + assert (!"mono_bridge->mark_cross_references is NULL; WE SHOULD NOT BE EXECUTING"); + return; + } + int i; + + Srij_MarkCrossReferences cross_references = {}; + + cross_references.ComponentsLen = (void*) (intptr_t) num_sccs; + cross_references.Components = (Srij_StronglyConnectedComponent*) calloc (num_sccs, sizeof (Srij_StronglyConnectedComponent)); + for (i = 0; i < num_sccs; ++i) { + Srij_StronglyConnectedComponent *scc = &cross_references.Components [i]; + + scc->Count = (void*) (intptr_t) sccs [i]->num_objs; + scc->Context = (void**) calloc (sccs [i]->num_objs, sizeof (void*)); + for (int j = 0; j < sccs [i]->num_objs; ++j) { + MonoObject *obj = sccs [i]->objs [j]; + scc->Context [j] = get_gc_control_block_for_object (mono_bridge, obj); + } + } + + cross_references.CrossReferencesLen = (void*) (intptr_t) num_xrefs; + cross_references.CrossReferences = (Srij_ComponentCrossReference*) calloc (num_xrefs, sizeof (Srij_ComponentCrossReference)); + for (i = 0; i < num_xrefs; ++i) { + Srij_ComponentCrossReference *xref = &cross_references.CrossReferences [i]; + xref->SourceGroupIndex = (void*) (intptr_t) xrefs [i].src_scc_index; + xref->DestinationGroupIndex = (void*) (intptr_t) xrefs [i].dst_scc_index; + } + + mono_bridge->mark_cross_references (&cross_references); + + for (i = 0; i < num_sccs; ++i) { + Srij_StronglyConnectedComponent *scc = &cross_references.Components [i]; + sccs [i]->is_alive = scc->IsAlive; + } +} + int java_interop_gc_bridge_register_hooks (JavaInteropGCBridge *bridge, int weak_ref_kind) { @@ -1316,7 +1358,9 @@ java_interop_gc_bridge_register_hooks (JavaInteropGCBridge *bridge, int weak_ref bridge_cbs.bridge_version = SGEN_BRIDGE_VERSION; bridge_cbs.bridge_class_kind = gc_bridge_class_kind; bridge_cbs.is_bridge_object = gc_is_bridge_object; - bridge_cbs.cross_references = gc_cross_references; + bridge_cbs.cross_references = bridge->mark_cross_references + ? managed_gc_cross_references + : gc_cross_references; mono_gc_register_bridge_callbacks (&bridge_cbs); @@ -1332,3 +1376,28 @@ java_interop_gc_bridge_wait_for_bridge_processing (JavaInteropGCBridge *bridge) mono_gc_wait_for_bridge_processing (); return 0; } + +int +java_interop_gc_bridge_set_mark_cross_references (JavaInteropGCBridge *bridge, JavaInteropMarkCrossReferencesCallback markCrossReferences) +{ + if (bridge == NULL) + return -1; + + bridge->mark_cross_references = markCrossReferences; + + return 0; +} + +int +java_interop_gc_bridge_release_mark_cross_references_resources (JavaInteropGCBridge *bridge, Srij_MarkCrossReferences *crossReferences) +{ + if (bridge == NULL) + return -1; + + if (crossReferences == NULL) + return -1; + + // leak it… + + return 0; +} diff --git a/src/java-interop/java-interop-gc-bridge.h b/src/java-interop/java-interop-gc-bridge.h index 8c8444cb6..6715b33be 100644 --- a/src/java-interop/java-interop-gc-bridge.h +++ b/src/java-interop/java-interop-gc-bridge.h @@ -19,6 +19,26 @@ struct JavaInterop_System_RuntimeTypeHandle { void *value; }; +typedef struct Srij_ComponentCrossReference { + void *SourceGroupIndex; + void *DestinationGroupIndex; +} Srij_ComponentCrossReference; + +typedef struct Srij_StronglyConnectedComponent { + int IsAlive; + void *Count; + void **Context; +} Srij_StronglyConnectedComponent; + +typedef struct Srij_MarkCrossReferences { + void *ComponentsLen; + Srij_StronglyConnectedComponent *Components; + void* CrossReferencesLen; + Srij_ComponentCrossReference *CrossReferences; +} Srij_MarkCrossReferences; + +typedef void (*JavaInteropMarkCrossReferencesCallback) (Srij_MarkCrossReferences *crossReferences); + JAVA_INTEROP_API JavaInteropGCBridge *java_interop_gc_bridge_get_current (void); JAVA_INTEROP_API int java_interop_gc_bridge_set_current_once (JavaInteropGCBridge *bridge); @@ -32,6 +52,10 @@ JAVA_INTEROP_API int java_interop_gc_bridge_add_current_a JAVA_INTEROP_API int java_interop_gc_bridge_remove_current_app_domain (JavaInteropGCBridge *bridge); JAVA_INTEROP_API int java_interop_gc_bridge_set_bridge_processing_field (JavaInteropGCBridge *bridge, struct JavaInterop_System_RuntimeTypeHandle type_handle, char *field_name); + +JAVA_INTEROP_API int java_interop_gc_bridge_set_mark_cross_references (JavaInteropGCBridge *bridge, JavaInteropMarkCrossReferencesCallback markCrossReferences); +JAVA_INTEROP_API int java_interop_gc_bridge_release_mark_cross_references_resources (JavaInteropGCBridge *bridge, Srij_MarkCrossReferences *crossReferences); + JAVA_INTEROP_API int java_interop_gc_bridge_register_bridgeable_type (JavaInteropGCBridge *bridge, struct JavaInterop_System_RuntimeTypeHandle type_handle); JAVA_INTEROP_API int java_interop_gc_bridge_enable (JavaInteropGCBridge *bridge, int enable); From ad451f96f1ca15fcaee0fb0db1e7801b4adfb6f6 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 13 May 2025 14:25:41 -0400 Subject: [PATCH 8/8] Try using the managed GC bridge. dotnet publish --self-contained -p:UseMonoRuntime=true -p:DotNetTargetFramework=net8.0 -p:UseAppHost=true -p:ErrorOnDuplicatePublishOutputFiles=false samples/Hello-Java.Base/Hello-Java.Base.csproj -r osx-x64 && \ JAVA_INTEROP_GREF_LOG=g.txt ./samples/Hello-Java.Base/bin/Release/osx-x64/publish/Hello-Java.Base Does it work? No quite. What's present here "works", in that the managed `MarkCrossReferences` callback *is* invoked, which just keeps all instances alive. However, it *doesn't* fully work, for reasons I don't understand: if `MarkCrossReferences` calls managed code, e.g. `Console.WriteLine("here!")`, then it *hangs*. Looks like this prototype approach can't work. --- .../Java.Interop/MonoRuntimeValueManager.cs | 6 +++++- src/java-interop/java-interop-gc-bridge-mono.cc | 3 +++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Java.Runtime.Environment/Java.Interop/MonoRuntimeValueManager.cs b/src/Java.Runtime.Environment/Java.Interop/MonoRuntimeValueManager.cs index 77a517ae0..722e65365 100644 --- a/src/Java.Runtime.Environment/Java.Interop/MonoRuntimeValueManager.cs +++ b/src/Java.Runtime.Environment/Java.Interop/MonoRuntimeValueManager.cs @@ -14,7 +14,7 @@ enum GCBridgeUseWeakReferenceKind { Jni, } - class MonoRuntimeValueManager : JniRuntime.JniValueManager { + partial class MonoRuntimeValueManager : JniRuntime.JniValueManager { #pragma warning disable 0649 // This field is mutated by the java-interop native lib @@ -46,6 +46,10 @@ public override void OnSetRuntime (JniRuntime runtime) throw new NotSupportedException ("Could not register current AppDomain!"); if (JreNativeMethods.java_interop_gc_bridge_set_current_once (bridge) < 0) throw new NotSupportedException ("Could not set GC Bridge instance!"); + unsafe { + if (JreNativeMethods.java_interop_gc_bridge_set_mark_cross_references(bridge, &MarkCrossReferences) < 0) + throw new NotSupportedException("Could not set MarkCrossReferences!"); + } } catch (Exception) { JreNativeMethods.java_interop_gc_bridge_free (bridge); diff --git a/src/java-interop/java-interop-gc-bridge-mono.cc b/src/java-interop/java-interop-gc-bridge-mono.cc index fd7e0337a..759a5656d 100644 --- a/src/java-interop/java-interop-gc-bridge-mono.cc +++ b/src/java-interop/java-interop-gc-bridge-mono.cc @@ -1288,6 +1288,7 @@ gc_cross_references (int num_sccs, MonoGCBridgeSCC **sccs, int num_xrefs, MonoGC static void managed_gc_cross_references (int num_sccs, MonoGCBridgeSCC **sccs, int num_xrefs, MonoGCBridgeXRef *xrefs) { + fprintf (stderr, "# jonp: managed_gc_cross_references: %d sccs and %d xrefs\n", num_sccs, num_xrefs); if (mono_bridge->mark_cross_references == NULL) { assert (!"mono_bridge->mark_cross_references is NULL; WE SHOULD NOT BE EXECUTING"); return; @@ -1317,12 +1318,14 @@ managed_gc_cross_references (int num_sccs, MonoGCBridgeSCC **sccs, int num_xrefs xref->DestinationGroupIndex = (void*) (intptr_t) xrefs [i].dst_scc_index; } + fprintf (stderr, "# jonp: calling managed mark_cross_references\n"); mono_bridge->mark_cross_references (&cross_references); for (i = 0; i < num_sccs; ++i) { Srij_StronglyConnectedComponent *scc = &cross_references.Components [i]; sccs [i]->is_alive = scc->IsAlive; } + fprintf (stderr, "# jonp: calling done\n"); } int