From 7f6f2373221f6e4b67bd900b114ecd07a859126c Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Fri, 9 May 2025 20:38:09 -0400 Subject: [PATCH 1/7] [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 Originally from: https://github.com/dotnet/java-interop/pull/1334 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 ); 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 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.. Implemented the thunk! --- .../Java.Interop/IJavaPeerable.cs | 2 + .../Java.Interop/JavaException.cs | 29 ++- src/Java.Interop/Java.Interop/JavaObject.cs | 31 ++- .../JniObjectReferenceControlBlock.cs | 27 +++ src/Java.Interop/PublicAPI.Unshipped.txt | 1 + .../Java.Interop/MonoRuntimeValueManager.cs | 12 + .../java-interop-gc-bridge-mono.cc | 209 ++++++++++++------ src/java-interop/java-interop-gc-bridge.h | 24 ++ 8 files changed, 246 insertions(+), 89 deletions(-) create mode 100644 src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs 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/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 fd3d68d3e..d0ea9bc62 100644 --- a/src/Java.Interop/Java.Interop/JavaObject.cs +++ b/src/Java.Interop/Java.Interop/JavaObject.cs @@ -25,13 +25,7 @@ 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 + unsafe JniObjectReferenceControlBlock* jniObjectReferenceControlBlock; #endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS protected static readonly JniObjectReference* InvalidJniObjectReference = null; @@ -41,13 +35,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 +90,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); @@ -118,6 +121,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 +176,9 @@ void IJavaPeerable.SetPeerReference (JniObjectReference reference) { SetPeerReference (ref reference, JniObjectReferenceOptions.Copy); } + + IntPtr IJavaPeerable.JniObjectReferenceControlBlock => + (IntPtr) jniObjectReferenceControlBlock; } } diff --git a/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs b/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs new file mode 100644 index 000000000..348e08cbd --- /dev/null +++ b/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs @@ -0,0 +1,27 @@ +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 JniObjectReferenceControlBlock* Alloc () + { + return (JniObjectReferenceControlBlock*) NativeMemory.AllocZeroed (1, (uint) Size); + } + + public static unsafe void Free (ref JniObjectReferenceControlBlock* value) + { + if (value == null) { + return; + } + NativeMemory.Free (value); + value = null; + } +} 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 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 aca306c60..fd7e0337a 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 { @@ -72,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 @@ -366,13 +372,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 +783,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 +803,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 +822,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 +833,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 +849,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 +858,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 +877,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 +903,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 +926,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 +985,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 +1049,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 +1208,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); @@ -1279,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) { @@ -1312,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); @@ -1328,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 dcd4ba126460e4981e5482a3de0d049632844932 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 9 Jun 2025 08:44:52 -0500 Subject: [PATCH 2/7] Update MonoRuntimeValueManager.cs --- .../Java.Interop/MonoRuntimeValueManager.cs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/Java.Runtime.Environment/Java.Interop/MonoRuntimeValueManager.cs b/src/Java.Runtime.Environment/Java.Interop/MonoRuntimeValueManager.cs index 77a517ae0..4cae1dc48 100644 --- a/src/Java.Runtime.Environment/Java.Interop/MonoRuntimeValueManager.cs +++ b/src/Java.Runtime.Environment/Java.Interop/MonoRuntimeValueManager.cs @@ -415,18 +415,6 @@ 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 { From e887d20290c552d68cd83757e4e19f113eaa09b1 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 9 Jun 2025 09:28:31 -0500 Subject: [PATCH 3/7] Update java-interop-gc-bridge-mono.cc --- src/java-interop/java-interop-gc-bridge-mono.cc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/java-interop/java-interop-gc-bridge-mono.cc b/src/java-interop/java-interop-gc-bridge-mono.cc index fd7e0337a..cebe8ece9 100644 --- a/src/java-interop/java-interop-gc-bridge-mono.cc +++ b/src/java-interop/java-interop-gc-bridge-mono.cc @@ -1391,13 +1391,9 @@ java_interop_gc_bridge_set_mark_cross_references (JavaInteropGCBridge *bridge, J int java_interop_gc_bridge_release_mark_cross_references_resources (JavaInteropGCBridge *bridge, Srij_MarkCrossReferences *crossReferences) { - if (bridge == NULL) - return -1; - - if (crossReferences == NULL) + if (bridge == NULL || crossReferences == NULL) return -1; // leak it… - return 0; } From 06a6a1e00a20dc4762bae41724bc289c4a98eef2 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 9 Jun 2025 14:01:02 -0500 Subject: [PATCH 4/7] [NonSerialized] --- src/Java.Interop/Java.Interop/JavaObject.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Java.Interop/Java.Interop/JavaObject.cs b/src/Java.Interop/Java.Interop/JavaObject.cs index d0ea9bc62..f782b7565 100644 --- a/src/Java.Interop/Java.Interop/JavaObject.cs +++ b/src/Java.Interop/Java.Interop/JavaObject.cs @@ -25,7 +25,7 @@ unsafe public class JavaObject : IJavaPeerable [NonSerialized] JniObjectReference reference; #endif // FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES #if FEATURE_JNIOBJECTREFERENCE_INTPTRS - unsafe JniObjectReferenceControlBlock* jniObjectReferenceControlBlock; + [NonSerialized] unsafe JniObjectReferenceControlBlock* jniObjectReferenceControlBlock; #endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS protected static readonly JniObjectReference* InvalidJniObjectReference = null; From a29fe5e45800e573b9f42377af49c1e1acc37a51 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 11 Jun 2025 09:55:58 -0500 Subject: [PATCH 5/7] `JniObjectReferenceControlBlock.Alloc()` takes `JniObjectReference` We think this could be a potential race condition --- src/Java.Interop/Java.Interop/JavaException.cs | 7 ++++--- src/Java.Interop/Java.Interop/JavaObject.cs | 7 ++++--- .../Java.Interop/JniObjectReferenceControlBlock.cs | 7 +++++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/Java.Interop/Java.Interop/JavaException.cs b/src/Java.Interop/Java.Interop/JavaException.cs index 43cffa228..0b2010ddf 100644 --- a/src/Java.Interop/Java.Interop/JavaException.cs +++ b/src/Java.Interop/Java.Interop/JavaException.cs @@ -138,10 +138,11 @@ protected void SetPeerReference (ref JniObjectReference reference, JniObjectRefe var c = jniObjectReferenceControlBlock; if (c == null) { c = jniObjectReferenceControlBlock = - Java.Interop.JniObjectReferenceControlBlock.Alloc (); + Java.Interop.JniObjectReferenceControlBlock.Alloc (reference); + } else { + c->handle = reference.Handle; + c->handle_type = (int) reference.Type; } - c->handle = reference.Handle; - c->handle_type = (int) reference.Type; #endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS JniObjectReference.Dispose (ref reference, options); diff --git a/src/Java.Interop/Java.Interop/JavaObject.cs b/src/Java.Interop/Java.Interop/JavaObject.cs index f782b7565..3391e205a 100644 --- a/src/Java.Interop/Java.Interop/JavaObject.cs +++ b/src/Java.Interop/Java.Interop/JavaObject.cs @@ -93,10 +93,11 @@ protected void SetPeerReference (ref JniObjectReference reference, JniObjectRefe var c = jniObjectReferenceControlBlock; if (c == null) { c = jniObjectReferenceControlBlock = - Java.Interop.JniObjectReferenceControlBlock.Alloc (); + Java.Interop.JniObjectReferenceControlBlock.Alloc (reference); + } else { + c->handle = reference.Handle; + c->handle_type = (int) reference.Type; } - c->handle = reference.Handle; - c->handle_type = (int) reference.Type; #endif // FEATURE_JNIOBJECTREFERENCE_INTPTRS JniObjectReference.Dispose (ref reference, options); diff --git a/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs b/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs index 348e08cbd..3f2d6f8ec 100644 --- a/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs +++ b/src/Java.Interop/Java.Interop/JniObjectReferenceControlBlock.cs @@ -11,9 +11,12 @@ internal struct JniObjectReferenceControlBlock { public static readonly int Size = Marshal.SizeOf(); - public static unsafe JniObjectReferenceControlBlock* Alloc () + public static unsafe JniObjectReferenceControlBlock* Alloc (JniObjectReference reference) { - return (JniObjectReferenceControlBlock*) NativeMemory.AllocZeroed (1, (uint) Size); + var value = (JniObjectReferenceControlBlock*) NativeMemory.AllocZeroed (1, (uint) Size); + value->handle = reference.Handle; + value->handle_type = (int) reference.Type; + return value; } public static unsafe void Free (ref JniObjectReferenceControlBlock* value) From 384bbfd46c6a78aad176eab7c51cc0d4946034dc Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 13 May 2025 07:17:54 -0400 Subject: [PATCH 6/7] 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 0b2010ddf..23280e05c 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)); @@ -171,9 +171,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) @@ -267,11 +264,13 @@ protected void SetJavaStackTrace (JniObjectReference peerReferenceOverride = def void IJavaPeerable.Disposed () { + JniManagedPeerState |= Disposed; Dispose (disposing: true); } void IJavaPeerable.Finalized () { + JniManagedPeerState |= Disposed; Dispose (disposing: false); } @@ -288,6 +287,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 3391e205a..04a14861f 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; @@ -122,9 +122,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) @@ -155,11 +152,13 @@ public override unsafe int GetHashCode () void IJavaPeerable.Disposed () { + managedPeerState |= Disposed; Dispose (disposing: true); } void IJavaPeerable.Finalized () { + managedPeerState |= Disposed; Dispose (disposing: false); } @@ -176,6 +175,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 f8b62a6806470b61b1f88ea79efd50be549ffea2 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 11 Jun 2025 15:53:39 -0500 Subject: [PATCH 7/7] Add assertion messages --- tests/Java.Interop-Tests/Java.Interop/JavaObjectTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Java.Interop-Tests/Java.Interop/JavaObjectTest.cs b/tests/Java.Interop-Tests/Java.Interop/JavaObjectTest.cs index d90d5846f..33dac36fe 100644 --- a/tests/Java.Interop-Tests/Java.Interop/JavaObjectTest.cs +++ b/tests/Java.Interop-Tests/Java.Interop/JavaObjectTest.cs @@ -50,12 +50,12 @@ public void UnregisterFromRuntime () using (o = new JavaObject ()) { l = o.PeerReference.NewLocalRef (); Assert.AreEqual (JniObjectReferenceType.Global, o.PeerReference.Type); - Assert.AreEqual (registeredCount+1, JniRuntime.CurrentRuntime.ValueManager.GetSurfacedPeers ().Count); + Assert.AreEqual (registeredCount+1, JniRuntime.CurrentRuntime.ValueManager.GetSurfacedPeers ().Count, "registeredCount+1 should match!"); Assert.IsNotNull (JniRuntime.CurrentRuntime.ValueManager.PeekValue (l)); Assert.AreNotSame (l, o.PeerReference); Assert.AreSame (o, JniRuntime.CurrentRuntime.ValueManager.PeekValue (l)); } - Assert.AreEqual (registeredCount, JniRuntime.CurrentRuntime.ValueManager.GetSurfacedPeers ().Count); + Assert.AreEqual (registeredCount, JniRuntime.CurrentRuntime.ValueManager.GetSurfacedPeers ().Count, "registeredCount should match!"); Assert.IsNull (JniRuntime.CurrentRuntime.ValueManager.PeekValue (l)); JniObjectReference.Dispose (ref l); Assert.Throws (() => o.UnregisterFromRuntime ());