From 9c4067a6e30f680cf483632b25cc05aef2e56799 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 10:14:10 +0200 Subject: [PATCH 01/37] [Java.Interop] Release method-cache redirect references Retain redirect ownership until method-cache publication succeeds, dispose unpublished candidates, and release cached redirects during teardown. Preserve reentrant lookup and transfer fallback ownership only after enumeration completes. Add deterministic publication, reentrancy, exception, and cache-lifecycle regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../JniPeerMembers.JniInstanceMethods.cs | 27 ++- .../JniPeerMembers.JniMethodInfoCache.cs | 28 +++ .../JniPeerMembers.JniStaticMethods.cs | 29 ++- .../JniRedirectCacheOwnershipTests.cs | 212 ++++++++++++++++++ 4 files changed, 273 insertions(+), 23 deletions(-) create mode 100644 external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs create mode 100644 external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs index f9e3092ddfa..1c590ffe534 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs @@ -47,7 +47,7 @@ internal JniType JniPeerType { internal void Dispose () { - Clear (ref instanceMethods); + Clear (ref instanceMethods, static value => value.StaticRedirect?.Dispose ()); Clear (ref subclassConstructors, static value => value.Dispose ()); if (jniPeerType != null) @@ -95,7 +95,7 @@ internal JniInstanceMethods GetConstructorsForType (Type declaringType) public JniMethodInfo GetMethodInfo (string encodedMember) { - return InstanceMethods.GetOrAdd (encodedMember, static (member, methods) => { + return GetOrAddMethodInfo (InstanceMethods, encodedMember, static (member, methods) => { ReadOnlySpan method, signature; JniPeerMembers.GetNameAndSignature (member, out method, out signature); return methods.GetMethodInfo (method, signature); @@ -111,15 +111,20 @@ JniMethodInfo GetMethodInfo (ReadOnlySpan method, ReadOnlySpan signa var methodName = newMethod.Value.TargetJniMethodName is string name ? name.AsSpan () : method; var methodSig = newMethod.Value.TargetJniMethodSignature is string sig ? sig.AsSpan () : signature; - using var t = new JniType (typeName); - if (newMethod.Value.TargetJniMethodInstanceToStatic && - t.TryGetStaticMethod (methodName, methodSig, out m)) { - m.ParameterCount = newMethod.Value.TargetJniMethodParameterCount; - m.StaticRedirect = new JniType (typeName); - return m; - } - if (t.TryGetInstanceMethod (methodName, methodSig, out m)) { - return m; + JniType? t = new JniType (typeName); + try { + if (newMethod.Value.TargetJniMethodInstanceToStatic && + t.TryGetStaticMethod (methodName, methodSig, out m)) { + m.ParameterCount = newMethod.Value.TargetJniMethodParameterCount; + m.StaticRedirect = t; + t = null; + return m; + } + if (t.TryGetInstanceMethod (methodName, methodSig, out m)) { + return m; + } + } finally { + t?.Dispose (); } Console.Error.WriteLine ($"warning: For declared method `{Members.JniPeerTypeName}.{method}.{signature}`, could not find requested method `{typeName}.{methodName}.{methodSig}`!"); } diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs new file mode 100644 index 00000000000..dbb69771c24 --- /dev/null +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs @@ -0,0 +1,28 @@ +#nullable enable + +using System; +using System.Collections.Concurrent; + +namespace Java.Interop { + + partial class JniPeerMembers { + + static JniMethodInfo GetOrAddMethodInfo (ConcurrentDictionary cache, string member, Func factory, TArg argument) + { + if (cache.TryGetValue (member, out var method)) + return method; + + // JNI lookup can reenter this cache. Construct before publication, but retain + // ownership of the redirect until this candidate actually wins. + var candidate = factory (member, argument); + try { + method = cache.GetOrAdd (member, candidate); + if (ReferenceEquals (method, candidate)) + candidate = null; + return method; + } finally { + candidate?.StaticRedirect?.Dispose (); + } + } + } +} diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs index a7f8ce9a096..9067dd47fdf 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs @@ -21,12 +21,12 @@ internal JniStaticMethods (JniPeerMembers members) internal void Dispose () { - Clear (ref staticMethods); + Clear (ref staticMethods, static value => value.StaticRedirect?.Dispose ()); } public JniMethodInfo GetMethodInfo (string encodedMember) { - return StaticMethods.GetOrAdd (encodedMember, static (member, methods) => { + return GetOrAddMethodInfo (StaticMethods, encodedMember, static (member, methods) => { ReadOnlySpan method, signature; JniPeerMembers.GetNameAndSignature (member, out method, out signature); return methods.GetMethodInfo (method, signature); @@ -72,23 +72,28 @@ JniType GetMethodDeclaringType (JniMethodInfo method) if (fallbackTypes == null) { return null; } - foreach (var ft in fallbackTypes) { - JniType? t = null; - try { + JniType? t = null; + try { + JniMethodInfo? m = null; + foreach (var ft in fallbackTypes) { if (!JniType.TryParse (ft, out t)) { continue; } - if (t.TryGetStaticMethod (method, signature, out var m)) { - m.StaticRedirect = t; - t = null; - return m; + if (t.TryGetStaticMethod (method, signature, out m)) { + break; } + t.Dispose (); + t = null; } - finally { - t?.Dispose (); + if (m != null) { + // Transfer ownership only after the fallback enumerator has been disposed. + m.StaticRedirect = t; + t = null; } + return m; + } finally { + t?.Dispose (); } - return null; } public unsafe void InvokeVoidMethod (string encodedMember, JniArgumentValue* parameters) diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs new file mode 100644 index 00000000000..ae49303ef2c --- /dev/null +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs @@ -0,0 +1,212 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; + +using Java.Interop; +using NUnit.Framework; + +namespace Java.InteropTests +{ + [TestFixture] + [Category ("NativeAOTIgnore")] + public class JniRedirectCacheOwnershipTests : JavaVMFixture + { + [TestCase (false)] + [TestCase (true)] + public void DisposingMethodCacheReleasesRedirect (bool isStatic) + { + var members = isStatic + ? new JniPeerMembers (IAndroidInterface.JniTypeName, typeof (IAndroidInterface)) + : new JniPeerMembers (JavaLangRemappingTestObject.JniTypeName, typeof (JavaLangRemappingTestObject)); + try { + for (int i = 0; i < 3; ++i) { + var method = GetRedirectedMethod (members, isStatic); + var redirect = method.StaticRedirect; + Assert.IsNotNull (redirect); + Assert.IsTrue (redirect.PeerReference.IsValid); + Assert.AreSame (method, GetRedirectedMethod (members, isStatic)); + + try { + AssertRedirectIsCallable (members, isStatic); + if (isStatic) + members.StaticMethods.Dispose (); + else + members.InstanceMethods.Dispose (); + Assert.IsFalse (redirect.PeerReference.IsValid, "The method cache owns the redirect's global reference."); + if (isStatic) + members.StaticMethods.Dispose (); + else + members.InstanceMethods.Dispose (); + } finally { + redirect.Dispose (); + } + } + } finally { + JniPeerMembers.Dispose (members); + } + } + + [TestCase (false)] + [TestCase (true)] + public void DisposingOrdinaryMethodCacheDoesNotDisposePeerType (bool isStatic) + { + var members = new JniPeerMembers (JavaLangRemappingTestRuntime.JniTypeName, typeof (JavaLangRemappingTestRuntime)); + try { + var peerType = members.JniPeerType; + var method = isStatic + ? members.StaticMethods.GetMethodInfo ("getRuntime.()Ljava/lang/Runtime;") + : members.InstanceMethods.GetMethodInfo ("hashCode.()I"); + Assert.IsNull (method.StaticRedirect); + + if (isStatic) + members.StaticMethods.Dispose (); + else + members.InstanceMethods.Dispose (); + Assert.IsTrue (peerType.PeerReference.IsValid); + } finally { + JniPeerMembers.Dispose (members); + } + } + + [Test] + public void ConcurrentPublicationDisposesOnlyLosingRedirects () + { + var lookup = GetMethodLookup (); + var cache = new ConcurrentDictionary (); + var candidates = new JniMethodInfo [2]; + var results = new JniMethodInfo [candidates.Length]; + using var ready = new Barrier (candidates.Length); + try { + Parallel.For (0, candidates.Length, i => { + results [i] = lookup (cache, "currentTimeMillis.()J", (member, index) => { + candidates [index] = CreateRedirect (); + if (!ready.SignalAndWait (TimeSpan.FromSeconds (30))) + throw new TimeoutException ("Both candidates must be created before publication."); + return candidates [index]; + }, i); + }); + + var winner = results [0]; + Assert.AreSame (winner, results [1]); + Assert.AreEqual (1, cache.Count); + foreach (var candidate in candidates) + Assert.AreEqual (ReferenceEquals (candidate, winner), candidate.StaticRedirect.PeerReference.IsValid); + AssertSystemRedirectIsCallable (winner); + Assert.AreSame (winner, lookup (cache, "currentTimeMillis.()J", + (member, state) => throw new InvalidOperationException ("A cache hit must not construct a candidate."), 0)); + } finally { + foreach (var candidate in candidates) + candidate?.StaticRedirect?.Dispose (); + } + } + + [TestCase (false)] + [TestCase (true)] + public void ReentrantPublicationPreservesWinner (bool returnWinner) + { + var lookup = GetMethodLookup (); + var cache = new ConcurrentDictionary (); + var outer = CreateRedirect (); + var inner = CreateRedirect (); + try { + var method = lookup (cache, "currentTimeMillis.()J", (member, state) => { + var winner = lookup (cache, member, (key, argument) => inner, state); + return returnWinner ? winner : outer; + }, 0); + + Assert.AreSame (inner, method); + Assert.AreEqual (returnWinner, outer.StaticRedirect.PeerReference.IsValid); + AssertSystemRedirectIsCallable (inner); + } finally { + outer.StaticRedirect.Dispose (); + inner.StaticRedirect.Dispose (); + } + } + + [Test] + public void PublicationFailureDisposesCandidate () + { + var lookup = GetMethodLookup (); + var comparer = new PublicationFailureComparer (); + var cache = new ConcurrentDictionary (comparer); + var candidate = CreateRedirect (); + try { + var error = Assert.Throws (() => + lookup (cache, "currentTimeMillis.()J", (member, state) => { + comparer.Fail = true; + return candidate; + }, 0)); + Assert.AreEqual ("Publication failed.", error.Message); + Assert.IsTrue (cache.IsEmpty); + Assert.IsFalse (candidate.StaticRedirect.PeerReference.IsValid); + } finally { + candidate.StaticRedirect.Dispose (); + } + } + + delegate JniMethodInfo MethodLookup (ConcurrentDictionary cache, string member, Func factory, int argument); + + [UnconditionalSuppressMessage ("AOT", "IL3050", Justification = "The private cache helper is invoked with a test-only factory in tests excluded from Native AOT.")] + static MethodLookup GetMethodLookup () + { + var method = typeof (JniPeerMembers).GetMethod ("GetOrAddMethodInfo", BindingFlags.NonPublic | BindingFlags.Static) + ?? throw new MissingMethodException (nameof (JniPeerMembers), "GetOrAddMethodInfo"); + return (MethodLookup) method.MakeGenericMethod (typeof (int)).CreateDelegate (typeof (MethodLookup)); + } + + static JniMethodInfo CreateRedirect () + { + var type = new JniType ("java/lang/System"); + try { + var method = type.GetStaticMethod ("currentTimeMillis", "()J"); + method.StaticRedirect = type; + type = null; + return method; + } finally { + type?.Dispose (); + } + } + + static unsafe void AssertSystemRedirectIsCallable (JniMethodInfo method) + { + Assert.IsTrue (method.StaticRedirect.PeerReference.IsValid); + Assert.Greater (JniEnvironment.StaticMethods.CallStaticLongMethod (method.StaticRedirect.PeerReference, method, null), 0); + } + + static unsafe void AssertRedirectIsCallable (JniPeerMembers members, bool isStatic) + { + if (isStatic) { + var value = members.StaticMethods.InvokeObjectMethod ("getClassName.()Ljava/lang/String;", null); + Assert.AreEqual ("DesugarAndroidInterface$-CC", JniEnvironment.Strings.ToString (ref value, JniObjectReferenceOptions.CopyAndDispose)); + } else { + using var value = new JavaLangRemappingTestObject (); + Assert.AreEqual (value.GetHashCode (), members.InstanceMethods.InvokeNonvirtualInt32Method ("remappedToStaticHashCode.()I", value, null)); + } + } + + static JniMethodInfo GetRedirectedMethod (JniPeerMembers members, bool isStatic) + { + return isStatic + ? members.StaticMethods.GetMethodInfo ("getClassName.()Ljava/lang/String;") + : members.InstanceMethods.GetMethodInfo ("remappedToStaticHashCode.()I"); + } + + sealed class PublicationFailureComparer : IEqualityComparer + { + public bool Fail; + + public bool Equals (string x, string y) => StringComparer.Ordinal.Equals (x, y); + + public int GetHashCode (string value) + { + if (Fail) + throw new InvalidOperationException ("Publication failed."); + return StringComparer.Ordinal.GetHashCode (value); + } + } + } +} From 588c8fc22093bb62823ceca1bb22bee906fba27d Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 10:13:58 +0200 Subject: [PATCH 02/37] [Java.Interop] Dispose subclass caches without initializing the owner Always dispose child member caches during explicit peer-member disposal, even when the owning class has not been initialized. Keep owner class disposal conditional and preserve lazy, repeatable cleanup. Add focused coverage for subclass-only construction, runtime untracking, repeated cache lifecycles, and uninitialized owner disposal. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/JniPeerMembers.cs | 4 +- .../JniPeerMembersDisposalTests.cs | 110 ++++++++++++++++++ 2 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniPeerMembersDisposalTests.cs diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.cs index 1b2242181d2..3cb5818202f 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.cs @@ -136,14 +136,14 @@ static void Clear (ref ConcurrentDictionary? diction protected virtual void Dispose (bool disposing) { - if (!disposing || jniPeerType == null) + if (!disposing) return; instanceMethods.Dispose (); instanceFields.Dispose (); staticMethods.Dispose (); staticFields.Dispose (); - jniPeerType.Dispose (); + jniPeerType?.Dispose (); jniPeerType = null; } diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniPeerMembersDisposalTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniPeerMembersDisposalTests.cs new file mode 100644 index 00000000000..ee051661c73 --- /dev/null +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniPeerMembersDisposalTests.cs @@ -0,0 +1,110 @@ +using System; +using System.Collections.Generic; +using System.Reflection; + +using Java.Interop; +using NUnit.Framework; + +namespace Java.InteropTests +{ + [TestFixture] + public class JniPeerMembersDisposalTests : JavaVMFixture + { + const BindingFlags PrivateInstance = BindingFlags.NonPublic | BindingFlags.Instance; + + [TestCase (false)] + [TestCase (true)] + public unsafe void Dispose_ReleasesSubclassCache (bool initializeOwner) + { + var members = new JniPeerMembers (CallNonvirtualBase.JniTypeName, typeof (CallNonvirtualBase)); + try { + for (int cycle = 0; cycle < 3; ++cycle) { + using var ownerType = initializeOwner ? members.JniPeerType : null; + var peer = members.InstanceMethods.StartCreateInstance ("()V", typeof (CallNonvirtualDerived), null); + try { + Assert.IsTrue (peer.IsValid); + } finally { + JniObjectReference.Dispose (ref peer); + } + + var constructors = members.InstanceMethods.GetConstructorsForType (typeof (CallNonvirtualDerived)); + using var subclassType = constructors.JniPeerType; + Assert.IsTrue (subclassType.PeerReference.IsValid); + Assert.AreEqual (JniObjectReferenceType.Global, subclassType.PeerReference.Type); + Assert.IsTrue (IsTracked (subclassType)); + Assert.AreSame (ownerType, GetOwnerType (members)); + + JniPeerMembers.Dispose (members); + + Assert.IsFalse (subclassType.PeerReference.IsValid, "Subclass class reference must be released before runtime shutdown."); + Assert.IsFalse (IsTracked (subclassType)); + if (ownerType != null) { + Assert.IsFalse (ownerType.PeerReference.IsValid); + Assert.IsFalse (IsTracked (ownerType)); + } + Assert.Throws (() => { + var type = constructors.JniPeerType; + }); + AssertUninitialized (members); + + JniPeerMembers.Dispose (members); + AssertUninitialized (members); + } + } finally { + JniPeerMembers.Dispose (members); + } + } + + [Test] + public void Dispose_UninitializedMembers_RemainsLazy () + { + var members = new JniPeerMembers (JavaObjectWithMissingJavaPeer.JniTypeName, typeof (JavaObjectWithMissingJavaPeer)); + try { + AssertUninitialized (members); + for (int i = 0; i < 3; ++i) { + // Resolving the owning class here would throw because it does not exist. + JniPeerMembers.Dispose (members); + AssertUninitialized (members); + } + } finally { + JniPeerMembers.Dispose (members); + } + } + + static JniType GetOwnerType (JniPeerMembers members) + { + return (JniType) GetFieldValue (typeof (JniPeerMembers).GetField ("jniPeerType", PrivateInstance), members); + } + + static bool IsTracked (JniType type) + { + var tracked = (Dictionary) GetFieldValue ( + typeof (JniRuntime).GetField ("TrackedInstances", PrivateInstance), JniEnvironment.Runtime); + lock (tracked) { + return tracked.ContainsValue (type); + } + } + + static void AssertUninitialized (JniPeerMembers members) + { + Assert.IsNull (GetOwnerType (members)); + Assert.IsNull (GetFieldValue ( + typeof (JniPeerMembers.JniInstanceMethods).GetField ("instanceMethods", PrivateInstance), members.InstanceMethods)); + Assert.IsNull (GetFieldValue ( + typeof (JniPeerMembers.JniInstanceMethods).GetField ("subclassConstructors", PrivateInstance), members.InstanceMethods)); + Assert.IsNull (GetFieldValue ( + typeof (JniPeerMembers.JniInstanceFields).GetField ("instanceFields", PrivateInstance), members.InstanceFields)); + Assert.IsNull (GetFieldValue ( + typeof (JniPeerMembers.JniStaticMethods).GetField ("staticMethods", PrivateInstance), members.StaticMethods)); + Assert.IsNull (GetFieldValue ( + typeof (JniPeerMembers.JniStaticFields).GetField ("staticFields", PrivateInstance), members.StaticFields)); + } + + static object GetFieldValue (FieldInfo field, object owner) + { + if (field == null) + throw new InvalidOperationException ("Expected private cache field was not found."); + return field.GetValue (owner); + } + } +} From cf8a268e109ad62084d87e5f9cab2bc791fd3e0b Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 10:13:59 +0200 Subject: [PATCH 03/37] Fix transferred JNI reference cleanup during activation Release transferred input references in finally blocks around wrapper activation and peer construction. Add focused ownership regression coverage for borrowed, local, and global references across success and failure paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/Mono.Android/Java.Lang/Object.cs | 21 +- src/Mono.Android/Java.Lang/Throwable.cs | 15 +- .../Java.Interop/TransferredReferenceTests.cs | 293 ++++++++++++++++++ .../Mono.Android.NET-Tests.csproj | 1 + 4 files changed, 316 insertions(+), 14 deletions(-) create mode 100644 tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/TransferredReferenceTests.cs diff --git a/src/Mono.Android/Java.Lang/Object.cs b/src/Mono.Android/Java.Lang/Object.cs index e065c63d27a..087ab4836e2 100644 --- a/src/Mono.Android/Java.Lang/Object.cs +++ b/src/Mono.Android/Java.Lang/Object.cs @@ -111,11 +111,14 @@ protected void SetHandle (IntPtr value, JniHandleOwnership transfer) { var reference = new JniObjectReference (value); var options = JNIEnv.ToJniObjectReferenceOptions (transfer); - JniEnvironment.Runtime.ValueManager.ConstructPeer ( - this, - ref reference, - value == IntPtr.Zero ? JniObjectReferenceOptions.None : options); - JNIEnv.DeleteRef (value, transfer); + try { + JniEnvironment.Runtime.ValueManager.ConstructPeer ( + this, + ref reference, + value == IntPtr.Zero ? JniObjectReferenceOptions.None : options); + } finally { + JNIEnv.DeleteRef (value, transfer); + } } internal static IJavaPeerable? PeekObject (IntPtr handle, Type? requiredType = null) @@ -166,9 +169,11 @@ protected void SetHandle (IntPtr value, JniHandleOwnership transfer) if (handle == IntPtr.Zero) return null; - var r = JniEnvironment.Runtime.ValueManager.GetPeer (new JniObjectReference (handle), type); - JNIEnv.DeleteRef (handle, transfer); - return r; + try { + return JniEnvironment.Runtime.ValueManager.GetPeer (new JniObjectReference (handle), type); + } finally { + JNIEnv.DeleteRef (handle, transfer); + } } [EditorBrowsable (EditorBrowsableState.Never)] diff --git a/src/Mono.Android/Java.Lang/Throwable.cs b/src/Mono.Android/Java.Lang/Throwable.cs index a6dcdb4422f..58188ba3a3d 100644 --- a/src/Mono.Android/Java.Lang/Throwable.cs +++ b/src/Mono.Android/Java.Lang/Throwable.cs @@ -100,13 +100,16 @@ protected void SetHandle (IntPtr value, JniHandleOwnership transfer) { var reference = new JniObjectReference (value); - Construct ( - ref reference, - value == IntPtr.Zero ? JniObjectReferenceOptions.None : JniObjectReferenceOptions.Copy); - if (value != IntPtr.Zero) { - SetJavaStackTrace (new JniObjectReference (value)); + try { + Construct ( + ref reference, + value == IntPtr.Zero ? JniObjectReferenceOptions.None : JniObjectReferenceOptions.Copy); + if (value != IntPtr.Zero) { + SetJavaStackTrace (new JniObjectReference (value)); + } + } finally { + JNIEnv.DeleteRef (value, transfer); } - JNIEnv.DeleteRef (value, transfer); } public static Throwable FromException (System.Exception e) diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/TransferredReferenceTests.cs b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/TransferredReferenceTests.cs new file mode 100644 index 00000000000..ac7ae7a2d67 --- /dev/null +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/TransferredReferenceTests.cs @@ -0,0 +1,293 @@ +using System; +using System.Reflection; + +using Android.Runtime; + +using Java.Interop; + +using NUnit.Framework; + +namespace Java.InteropTests +{ + [TestFixture] + [NonParallelizable] + [Category ("TransferredReferences")] + public class TransferredReferenceTests + { + [Test] + public void GetObject_Success ( + [Values (JniHandleOwnership.DoNotTransfer, JniHandleOwnership.TransferLocalRef, JniHandleOwnership.TransferGlobalRef)] JniHandleOwnership ownership, + [Values (false, true)] bool doNotRegister) + { + var transfer = WithRegistration (ownership, doNotRegister); + using var input = new InputReferenceTracker (JNIEnv.AllocObject ("java/lang/Object"), transfer); + Assert.IsNull (JniEnvironment.Runtime.ValueManager.PeekPeer (new JniObjectReference (input.Handle))); + + using var peer = Java.Lang.Object.GetObject (input.Handle, transfer); + input.AssertOwnership (transfer); + Assert.IsNotNull (peer); + Assert.IsTrue (peer.PeerReference.IsValid); + Assert.AreNotEqual (input.Handle, peer.Handle, "The peer must own a separate reference."); + } + + [Test] + public void GetObject_CachedPeer ( + [Values (JniHandleOwnership.DoNotTransfer, JniHandleOwnership.TransferLocalRef, JniHandleOwnership.TransferGlobalRef)] JniHandleOwnership ownership) + { + using var peer = new Java.Lang.Object (JNIEnv.CreateInstance ("java/lang/Object", "()V"), JniHandleOwnership.TransferLocalRef); + using var input = new InputReferenceTracker (JNIEnv.NewLocalRef (peer.Handle), ownership); + + Assert.AreSame (peer, Java.Lang.Object.GetObject (input.Handle, ownership)); + input.AssertOwnership (ownership); + Assert.IsTrue (peer.PeerReference.IsValid); + } + + [Test] + public void GetObject_MissingActivationConstructor ( + [Values (JniHandleOwnership.DoNotTransfer, JniHandleOwnership.TransferLocalRef, JniHandleOwnership.TransferGlobalRef)] JniHandleOwnership ownership, + [Values (false, true)] bool doNotRegister) + { + if (Microsoft.Android.Runtime.RuntimeFeature.TrimmableTypeMap) { + Assert.Ignore ("The trimmable typemap supports inherited activation constructors."); + } + + var transfer = WithRegistration (ownership, doNotRegister); + using var input = new InputReferenceTracker (JNIEnv.AllocObject (typeof (MissingTransferredReferencePeer)), transfer); + Assert.IsNull (JniEnvironment.Runtime.ValueManager.PeekPeer (new JniObjectReference (input.Handle))); + + var exception = Assert.Throws (() => + Java.Lang.Object.GetObject (input.Handle, transfer)); + Assert.IsInstanceOf (exception.InnerException); + input.AssertOwnership (transfer); + } + + [Test] + public void GetObject_ThrowingActivationConstructor ( + [Values (JniHandleOwnership.DoNotTransfer, JniHandleOwnership.TransferLocalRef, JniHandleOwnership.TransferGlobalRef)] JniHandleOwnership ownership, + [Values (false, true)] bool doNotRegister) + { + var transfer = WithRegistration (ownership, doNotRegister); + using var input = new InputReferenceTracker (JNIEnv.AllocObject (typeof (ThrowingTransferredReferencePeer)), transfer); + Assert.IsNull (JniEnvironment.Runtime.ValueManager.PeekPeer (new JniObjectReference (input.Handle))); + + var exception = Assert.Catch (() => + Java.Lang.Object.GetObject (input.Handle, transfer)); + if (exception is TargetInvocationException invocation) { + exception = invocation.InnerException; + } + Assert.IsInstanceOf (exception); + Assert.AreEqual (ThrowingTransferredReferencePeer.ExceptionMessage, exception.Message); + input.AssertOwnership (transfer); + } + + [Test] + public void ObjectSetHandle ( + [Values (JniHandleOwnership.DoNotTransfer, JniHandleOwnership.TransferLocalRef, JniHandleOwnership.TransferGlobalRef)] JniHandleOwnership ownership, + [Values (false, true)] bool doNotRegister, + [Values (false, true)] bool failCopy) + { + var transfer = WithRegistration (ownership, doNotRegister); + using var input = new InputReferenceTracker (JNIEnv.AllocObject ("java/lang/Object"), transfer) { + FailCopy = failCopy, + }; + using var peer = new SetHandleObject (); + if (failCopy) { + Assert.Throws (() => peer.Assign (input.Handle, transfer)); + Assert.IsFalse (peer.PeerReference.IsValid); + } else { + peer.Assign (input.Handle, transfer); + Assert.IsTrue (peer.PeerReference.IsValid); + var registered = JniEnvironment.Runtime.ValueManager.PeekPeer (peer.PeerReference); + if (doNotRegister) { + Assert.IsNull (registered); + } else { + Assert.AreSame (peer, registered); + } + } + input.AssertOwnership (transfer); + } + + [Test] + public void ThrowableSetHandle ( + [Values (JniHandleOwnership.DoNotTransfer, JniHandleOwnership.TransferLocalRef, JniHandleOwnership.TransferGlobalRef)] JniHandleOwnership ownership, + [Values (false, true)] bool doNotRegister, + [Values (false, true)] bool failCopy) + { + var transfer = WithRegistration (ownership, doNotRegister); + using var input = new InputReferenceTracker (JNIEnv.CreateInstance ("java/lang/Throwable", "()V"), transfer) { + FailCopy = failCopy, + }; + using var peer = new SetHandleThrowable (); + if (failCopy) { + Assert.Throws (() => peer.Assign (input.Handle, transfer)); + Assert.IsFalse (peer.PeerReference.IsValid); + } else { + peer.Assign (input.Handle, transfer); + Assert.IsTrue (peer.PeerReference.IsValid); + Assert.IsNotNull (peer.JavaStackTrace); + } + input.AssertOwnership (transfer); + } + + static JniHandleOwnership WithRegistration (JniHandleOwnership ownership, bool doNotRegister) + { + return ownership | (doNotRegister ? JniHandleOwnership.DoNotRegister : JniHandleOwnership.DoNotTransfer); + } + + // Observe only the input handle, forwarding all JNI operations to the real manager. + // This avoids global-count races and never asks JNI about an already deleted handle. + sealed class InputReferenceTracker : JniRuntime.JniObjectReferenceManager + { + static readonly PropertyInfo managerProperty = typeof (JniRuntime).GetProperty (nameof (JniRuntime.ObjectReferenceManager)) + ?? throw new InvalidOperationException ("Could not find the JNI object reference manager property."); + + readonly JniRuntime.JniObjectReferenceManager original; + readonly JniObjectReferenceType referenceType; + int deletions; + + public IntPtr Handle { get; } + public bool FailCopy { get; set; } + + public InputReferenceTracker (IntPtr local, JniHandleOwnership ownership) + { + OnSetRuntime (JniEnvironment.Runtime); + original = Runtime.ObjectReferenceManager; + referenceType = (ownership & JniHandleOwnership.TransferLocalRef) != 0 + ? JniObjectReferenceType.Local + : JniObjectReferenceType.Global; + if (referenceType == JniObjectReferenceType.Local) { + Handle = local; + } else { + try { + Handle = JNIEnv.NewGlobalRef (local); + } finally { + JNIEnv.DeleteLocalRef (local); + } + } + bool installed = false; + try { + managerProperty.SetValue (Runtime, this); + installed = true; + } finally { + if (!installed) { + DeleteInput (); + } + } + } + + public void AssertOwnership (JniHandleOwnership ownership) + { + bool transferred = (ownership & (JniHandleOwnership.TransferLocalRef | JniHandleOwnership.TransferGlobalRef)) != 0; + Assert.AreEqual (transferred ? 1 : 0, deletions, "Input reference deletion count."); + } + + protected override void Dispose (bool disposing) + { + try { + // Also clean up when a regression leaves the transferred input behind. + if (deletions == 0) { + DeleteInput (); + } + } finally { + managerProperty.SetValue (Runtime, original); + } + } + + void DeleteInput () + { + if (referenceType == JniObjectReferenceType.Local) { + JNIEnv.DeleteLocalRef (Handle); + } else { + JNIEnv.DeleteGlobalRef (Handle); + } + } + + void Deleting (JniObjectReference reference) + { + if (reference.Handle != Handle) { + return; + } + Assert.AreEqual (referenceType, reference.Type, "Wrong deletion API for the input reference."); + // Fail before entering JNI if a regression attempts a double delete. + Assert.AreEqual (0, deletions, "Input reference was deleted more than once."); + deletions++; + } + + public override int GlobalReferenceCount => original.GlobalReferenceCount; + public override int WeakGlobalReferenceCount => original.WeakGlobalReferenceCount; + public override bool LogGlobalReferenceMessages => original.LogGlobalReferenceMessages; + public override bool LogLocalReferenceMessages => original.LogLocalReferenceMessages; + + public override void WriteGlobalReferenceLine (string format, params object [] args) => original.WriteGlobalReferenceLine (format, args); + public override void WriteLocalReferenceLine (string format, params object [] args) => original.WriteLocalReferenceLine (format, args); + public override JniObjectReference CreateLocalReference (JniObjectReference reference, ref int count) => original.CreateLocalReference (reference, ref count); + public override void CreatedLocalReference (JniObjectReference reference, ref int count) => original.CreatedLocalReference (reference, ref count); + public override IntPtr ReleaseLocalReference (ref JniObjectReference reference, ref int count) => original.ReleaseLocalReference (ref reference, ref count); + public override JniObjectReference CreateWeakGlobalReference (JniObjectReference reference) => original.CreateWeakGlobalReference (reference); + public override void DeleteWeakGlobalReference (ref JniObjectReference reference) => original.DeleteWeakGlobalReference (ref reference); + + public override JniObjectReference CreateGlobalReference (JniObjectReference reference) + { + if (FailCopy && reference.Handle == Handle) { + throw new InvalidOperationException ("Injected input reference copy failure."); + } + return original.CreateGlobalReference (reference); + } + + public override void DeleteLocalReference (ref JniObjectReference reference, ref int count) + { + Deleting (reference); + original.DeleteLocalReference (ref reference, ref count); + } + + public override void DeleteGlobalReference (ref JniObjectReference reference) + { + Deleting (reference); + original.DeleteGlobalReference (ref reference); + } + } + + sealed class SetHandleObject : Java.Lang.Object + { + public SetHandleObject () : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer) + { + } + + public void Assign (IntPtr handle, JniHandleOwnership transfer) => SetHandle (handle, transfer); + } + + sealed class SetHandleThrowable : Java.Lang.Throwable + { + public SetHandleThrowable () : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer) + { + } + + public void Assign (IntPtr handle, JniHandleOwnership transfer) => SetHandle (handle, transfer); + } + } + + [Register ("net/dot/android/test/MissingTransferredReferencePeer")] + public sealed class MissingTransferredReferencePeer : Java.Lang.Object + { + public MissingTransferredReferencePeer () + { + } + } + + [Register ("net/dot/android/test/ThrowingTransferredReferencePeer")] + public sealed class ThrowingTransferredReferencePeer : Java.Lang.Object + { + public const string ExceptionMessage = "transferred reference activation failure"; + + public ThrowingTransferredReferencePeer () + { + } + + public ThrowingTransferredReferencePeer (IntPtr handle, JniHandleOwnership transfer) + : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer) + { + // Fail before creating a peer reference so this tests only input ownership. + throw new InvalidOperationException (ExceptionMessage); + } + } +} diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj b/tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj index 8c817a19434..c6a2629389c 100644 --- a/tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj @@ -147,6 +147,7 @@ + From e8c159603f4089a05ebad620fde32387ca159257 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 10:14:04 +0200 Subject: [PATCH 04/37] [runtime] Release temporary startup class reference Use a local java.lang.Class reference when caching getName during MonoVM and CoreCLR startup, then release it immediately. Keep the unused init field reserved to preserve the shared native/managed layout. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/Mono.Android/Android.Runtime/JNIEnvInit.cs | 2 +- src/native/clr/host/host.cc | 7 +++++-- src/native/common/include/managed-interface.hh | 2 +- src/native/mono/monodroid/monodroid-glue.cc | 6 ++++-- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/Mono.Android/Android.Runtime/JNIEnvInit.cs b/src/Mono.Android/Android.Runtime/JNIEnvInit.cs index 9e6c6fa599d..bfb61dab91d 100644 --- a/src/Mono.Android/Android.Runtime/JNIEnvInit.cs +++ b/src/Mono.Android/Android.Runtime/JNIEnvInit.cs @@ -22,7 +22,7 @@ internal struct JnienvInitializeArgs { public IntPtr env; public IntPtr grefLoader; public IntPtr Loader_loadClass; - public IntPtr grefClass; // TODO: remove, not needed anymore + public IntPtr grefClass; // Unused; reserved to preserve the shared native/managed layout. public uint logCategories; public int version; // TODO: remove, not needed anymore public int grefGcThreshold; diff --git a/src/native/clr/host/host.cc b/src/native/clr/host/host.cc index f82d71f37a2..76a376c83d5 100644 --- a/src/native/clr/host/host.cc +++ b/src/native/clr/host/host.cc @@ -492,8 +492,11 @@ void Host::Java_mono_android_Runtime_initInternal ( // GC threshold is 90% of the max GREF count init.grefGcThreshold = static_cast(AndroidSystem::get_gref_gc_threshold ()); - init.grefClass = RuntimeUtil::get_class_from_runtime_field (env, runtimeClass, "java_lang_Class"sv, true); - Class_getName = env->GetMethodID (init.grefClass, "getName", "()Ljava/lang/String;"); + + // java.lang.Class is a bootstrap class, so the cached method ID outlives this local reference. + jclass lrefClass = RuntimeUtil::get_class_from_runtime_field (env, runtimeClass, "java_lang_Class"sv, false); + Class_getName = env->GetMethodID (lrefClass, "getName", "()Ljava/lang/String;"); + env->DeleteLocalRef (lrefClass); jclass lrefLoaderClass = env->GetObjectClass (loader); init.Loader_loadClass = env->GetMethodID (lrefLoaderClass, "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;"); diff --git a/src/native/common/include/managed-interface.hh b/src/native/common/include/managed-interface.hh index ca29461dc05..421cff849e7 100644 --- a/src/native/common/include/managed-interface.hh +++ b/src/native/common/include/managed-interface.hh @@ -23,7 +23,7 @@ namespace xamarin::android { JNIEnv *env; jobject grefLoader; jmethodID Loader_loadClass; - jclass grefClass; + jclass grefClass; // Unused; reserved to preserve the shared native/managed layout. unsigned int logCategories; int version; int grefGcThreshold; diff --git a/src/native/mono/monodroid/monodroid-glue.cc b/src/native/mono/monodroid/monodroid-glue.cc index 4a5f9e38c40..a01862f9ff7 100644 --- a/src/native/mono/monodroid/monodroid-glue.cc +++ b/src/native/mono/monodroid/monodroid-glue.cc @@ -836,8 +836,10 @@ MonodroidRuntime::init_android_runtime (JNIEnv *env, jclass runtimeClass, jobjec log_info (LOG_GC, "GREF GC Threshold: {}", init.grefGcThreshold); - init.grefClass = RuntimeUtil::get_class_from_runtime_field (env, runtimeClass, "java_lang_Class", true); - Class_getName = env->GetMethodID (init.grefClass, "getName", "()Ljava/lang/String;"); + // java.lang.Class is a bootstrap class, so the cached method ID outlives this local reference. + jclass lrefClass = RuntimeUtil::get_class_from_runtime_field (env, runtimeClass, "java_lang_Class", false); + Class_getName = env->GetMethodID (lrefClass, "getName", "()Ljava/lang/String;"); + env->DeleteLocalRef (lrefClass); MonoAssembly *mono_android_assembly; From beff091de917afea8c125796738434d1e88ecd6a Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 10:14:25 +0200 Subject: [PATCH 05/37] Dispose unpublished subclass constructor caches Keep subclass construction outside cache locks to preserve recursive lookup. Transfer ownership only to the published candidate and dispose losers or candidates whose publication throws. Add deterministic host-JVM coverage for concurrent and recursive publication, exceptional cleanup, winner usability, and runtime untracking. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../JniPeerMembers.JniInstanceMethods.cs | 14 +- .../JniSubclassConstructorCacheTests.cs | 255 ++++++++++++++++++ 2 files changed, 268 insertions(+), 1 deletion(-) create mode 100644 external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniSubclassConstructorCacheTests.cs diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs index 1c590ffe534..213b7e4ebcb 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs @@ -68,6 +68,10 @@ internal JniInstanceMethods GetConstructorsForType (Type declaringType) if (declaringType == DeclaringType) return this; + var cache = SubclassConstructors; + if (cache.TryGetValue (declaringType, out var constructors)) + return constructors; + // Initialize before publication in case construction recursively accesses this cache: // System.ArgumentException: An item with the same key has already been added. Key: Java.Interop.JavaProxyThrowable // at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior) @@ -90,7 +94,15 @@ internal JniInstanceMethods GetConstructorsForType (Type declaringType) // at Java.Interop.JniPeerMembers.JniInstanceMethods..ctor(Type declaringType) in /Users/jon/Developer/src/xamarin/java.interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 27 // at Java.Interop.JniPeerMembers.JniInstanceMethods.GetConstructorsForType(Type declaringType) in /Users/jon/Developer/src/xamarin/java.interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 77 // at Java.Interop.JniPeerMembers.JniInstanceMethods.StartCreateInstance(String constructorSignature, Type declaringType, JniArgumentValue* parameters) in /Users/jon/Developer/src/xamarin/java.interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 146 - return SubclassConstructors.GetOrAdd (declaringType, static type => new JniInstanceMethods (type)); + var candidate = new JniInstanceMethods (declaringType); + try { + constructors = cache.GetOrAdd (declaringType, candidate); + return constructors; + } finally { + // Only the published candidate transfers ownership to the cache. + if (!ReferenceEquals (constructors, candidate)) + candidate.Dispose (); + } } public JniMethodInfo GetMethodInfo (string encodedMember) diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniSubclassConstructorCacheTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniSubclassConstructorCacheTests.cs new file mode 100644 index 00000000000..2c853bb522a --- /dev/null +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniSubclassConstructorCacheTests.cs @@ -0,0 +1,255 @@ +#if !__ANDROID__ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; + +using Java.Interop; +using NUnit.Framework; + +namespace Java.InteropTests +{ + [TestFixture] + [NonParallelizable] + public class JniSubclassConstructorCacheTests : JavaVMFixture + { + [Test] + public void ConcurrentCreationDisposesUnpublishedConstructors () + { + RunWithReferenceTracking ((members, references, runtime) => { + const int count = 4; + using var barrier = new Barrier (count); + references.OnCreate = () => Assert.IsTrue (barrier.SignalAndWait (TimeSpan.FromSeconds (30)), "Constructor creation did not overlap."); + + var calls = Enumerable.Range (0, count) + .Select (_ => Task.Factory.StartNew ( + () => members.InstanceMethods.GetConstructorsForType (typeof (MyString)), + CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default)) + .ToArray (); + var constructors = Task.WhenAll (calls).GetAwaiter ().GetResult (); + references.OnCreate = null; + + foreach (var constructor in constructors) + Assert.AreSame (constructors [0], constructor); + AssertWinnerAndCleanup (members, references, runtime, constructors [0], count); + }); + } + + [Test] + public void RecursiveCreationDisposesOuterConstructor () + { + RunWithReferenceTracking ((members, references, runtime) => { + JniPeerMembers.JniInstanceMethods recursive = null; + references.OnCreate = () => { + references.OnCreate = null; + recursive = members.InstanceMethods.GetConstructorsForType (typeof (MyString)); + }; + + var constructor = members.InstanceMethods.GetConstructorsForType (typeof (MyString)); + + Assert.AreSame (recursive, constructor, "The recursive lookup should publish the winner."); + AssertWinnerAndCleanup (members, references, runtime, constructor, 2); + }); + } + + [Test] + public void PublicationFailureDisposesConstructor () + { + RunWithReferenceTracking ((members, references, runtime) => { + var comparer = new ThrowingTypeComparer (); + var cache = new ConcurrentDictionary (comparer); + var field = typeof (JniPeerMembers.JniInstanceMethods).GetField ("subclassConstructors", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.IsNotNull (field); + field.SetValue (members.InstanceMethods, cache); + references.OnCreate = () => comparer.ThrowOnHash = true; + + Assert.Throws (() => members.InstanceMethods.GetConstructorsForType (typeof (MyString))); + references.OnCreate = null; + comparer.ThrowOnHash = false; + + Assert.AreEqual (1, references.Created.Count); + Assert.IsEmpty (cache); + AssertReleased (references, runtime); + + // Failure must leave the cache usable for a subsequent lookup. + var constructor = members.InstanceMethods.GetConstructorsForType (typeof (MyString)); + AssertWinnerAndCleanup (members, references, runtime, constructor, 2); + }); + } + + static unsafe void AssertWinnerAndCleanup (JniPeerMembers members, TrackingReferenceManager references, JniRuntime runtime, JniPeerMembers.JniInstanceMethods winner, int created) + { + var type = winner.JniPeerType; + var handle = type.PeerReference.Handle; + Assert.AreSame (winner, members.InstanceMethods.GetConstructorsForType (typeof (MyString))); + Assert.IsTrue (winner.GetConstructor ("()V").IsValid); + + var instance = members.InstanceMethods.NewObject ("()V", typeof (MyString), null); + try { + Assert.IsTrue (type.IsInstanceOfType (instance), "The winning constructor must remain usable."); + } finally { + JniObjectReference.Dispose (ref instance); + } + + Assert.Multiple (() => { + Assert.AreEqual (created, references.Created.Count, "Cache hits must not create more class globals."); + CollectionAssert.AreEquivalent (new [] { handle }, references.Created.Where (references.IsLive).Distinct ()); + var tracked = GetTrackedInstances (runtime); + lock (tracked) { + CollectionAssert.AreEquivalent (new [] { handle }, references.Created.Where (tracked.ContainsKey).Distinct ()); + Assert.AreSame (type, tracked [handle], "The winner must stay registered until disposal."); + } + + JniPeerMembers.Dispose (members); + Assert.IsFalse (type.PeerReference.IsValid); + AssertReleased (references, runtime); + }); + } + + static void AssertReleased (TrackingReferenceManager references, JniRuntime runtime) + { + Assert.IsFalse (references.Created.Any (references.IsLive), "Unpublished constructor class globals must be deleted."); + var tracked = GetTrackedInstances (runtime); + lock (tracked) + Assert.IsFalse (references.Created.Any (tracked.ContainsKey), "Constructor classes must not be retained until runtime shutdown."); + } + + static Dictionary GetTrackedInstances (JniRuntime runtime) + { + var field = typeof (JniRuntime).GetField ("TrackedInstances", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.IsNotNull (field); + return (Dictionary) field.GetValue (runtime); + } + + static void RunWithReferenceTracking (Action test) + { + var runtime = JniEnvironment.Runtime; + var original = runtime.ObjectReferenceManager; + var references = new TrackingReferenceManager (original); + references.OnSetRuntime (runtime); + var property = typeof (JniRuntime).GetProperty (nameof (JniRuntime.ObjectReferenceManager)); + Assert.IsNotNull (property); + var members = new JniPeerMembers ("java/lang/Object", typeof (JavaObject)); + try { + // Exclude class initialization and the parent's separately-owned class global. + Assert.IsTrue (members.JniPeerType.PeerReference.IsValid); + using (var type = new JniType (MyString.JniTypeName)) { + var instance = type.AllocObject (); + JniObjectReference.Dispose (ref instance); + } + // Decorate the fixture's manager without creating another runtime or changing JNI environments. + property.SetValue (runtime, references); + references.TrackCreation = true; + test (members, references, runtime); + } finally { + references.TrackCreation = false; + references.OnCreate = null; + try { + JniPeerMembers.Dispose (members); + // Also clean up leaked candidates when running this regression against broken code. + var tracked = GetTrackedInstances (runtime); + List remaining; + lock (tracked) + remaining = references.LiveHandles.Where (tracked.ContainsKey).Select (handle => tracked [handle]).ToList (); + foreach (var value in remaining) + value.Dispose (); + } finally { + property.SetValue (runtime, original); + } + } + } + + sealed class TrackingReferenceManager : JniRuntime.JniObjectReferenceManager + { + readonly JniRuntime.JniObjectReferenceManager inner; + readonly ConcurrentDictionary live = new ConcurrentDictionary (); + readonly AsyncLocal trackCreation = new AsyncLocal (); + + public readonly ConcurrentBag Created = new ConcurrentBag (); + public Action OnCreate; + + public TrackingReferenceManager (JniRuntime.JniObjectReferenceManager inner) + { + this.inner = inner; + } + + public bool TrackCreation { + get => trackCreation.Value; + set => trackCreation.Value = value; + } + + public ICollection LiveHandles => live.Keys; + public override int GlobalReferenceCount => inner.GlobalReferenceCount; + public override int WeakGlobalReferenceCount => inner.WeakGlobalReferenceCount; + public override bool LogLocalReferenceMessages => inner.LogLocalReferenceMessages; + public override bool LogGlobalReferenceMessages => inner.LogGlobalReferenceMessages; + + public bool IsLive (IntPtr handle) => live.ContainsKey (handle); + + public override JniObjectReference CreateGlobalReference (JniObjectReference reference) + { + var result = inner.CreateGlobalReference (reference); + if (TrackCreation) { + live.TryAdd (result.Handle, 0); + Created.Add (result.Handle); + try { + OnCreate?.Invoke (); + } catch { + DeleteGlobalReference (ref result); + throw; + } + } + return result; + } + + public override void DeleteGlobalReference (ref JniObjectReference reference) + { + var handle = reference.Handle; + inner.DeleteGlobalReference (ref reference); + live.TryRemove (handle, out _); + } + + public override JniObjectReference CreateLocalReference (JniObjectReference reference, ref int localReferenceCount) => + inner.CreateLocalReference (reference, ref localReferenceCount); + + public override void DeleteLocalReference (ref JniObjectReference reference, ref int localReferenceCount) => + inner.DeleteLocalReference (ref reference, ref localReferenceCount); + + public override void CreatedLocalReference (JniObjectReference reference, ref int localReferenceCount) => + inner.CreatedLocalReference (reference, ref localReferenceCount); + + public override IntPtr ReleaseLocalReference (ref JniObjectReference reference, ref int localReferenceCount) => + inner.ReleaseLocalReference (ref reference, ref localReferenceCount); + + public override JniObjectReference CreateWeakGlobalReference (JniObjectReference reference) => + inner.CreateWeakGlobalReference (reference); + + public override void DeleteWeakGlobalReference (ref JniObjectReference reference) => + inner.DeleteWeakGlobalReference (ref reference); + + public override void WriteLocalReferenceLine (string format, params object [] args) => + inner.WriteLocalReferenceLine (format, args); + + public override void WriteGlobalReferenceLine (string format, params object [] args) => + inner.WriteGlobalReferenceLine (format, args); + } + + sealed class ThrowingTypeComparer : IEqualityComparer + { + public bool ThrowOnHash; + + public bool Equals (Type x, Type y) => x == y; + + public int GetHashCode (Type type) + { + if (ThrowOnHash) + throw new InvalidOperationException ("Constructor publication failed."); + return type.GetHashCode (); + } + } + } +} +#endif From c17a27922e11ba0d076b46500e1e48933a316ade Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 10:14:50 +0200 Subject: [PATCH 06/37] [Java.Interop] Preserve standalone native registration ownership Dispose temporary class globals when standalone registration does not adopt them. Transfer ownership after marshalling and before JNI can publish callbacks, preserving earlier delegate batches and retaining partial registrations safely until disposal. Add focused regression coverage for empty and failed registration, borrowed references, existing owners, and callback lifetime across repeated and concurrent registration. Addresses finding 7 in dotnet/android#12760. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/JniEnvironment.Types.cs | 9 + .../src/Java.Interop/Java.Interop/JniType.cs | 26 +- .../Java.Interop/Java.Interop/ManagedPeer.cs | 41 +-- .../Java.Interop-Tests.csproj | 1 + .../Java.Interop/JavaVMFixture.cs | 9 + .../ManagedPeerRegistrationTests.cs | 274 ++++++++++++++++++ .../dot/jni/test/ManagedPeerRegistration.java | 6 + 7 files changed, 340 insertions(+), 26 deletions(-) create mode 100644 external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs create mode 100644 external/Java.Interop/tests/Java.Interop-Tests/java/net/dot/jni/test/ManagedPeerRegistration.java diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs index 2a543700398..f52f06a2acb 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs @@ -224,6 +224,12 @@ public static void RegisterNatives (JniObjectReference type, JniNativeMethodRegi [RequiresDynamicCode ("Native method registration via JniNativeMethodRegistration[] requires dynamic code generation. Use the blittable RegisterNatives(JniObjectReference, ReadOnlySpan) overload with statically-compiled function pointers for Native AOT compatibility.")] public static unsafe void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration [] methods, int numMethods) + { + RegisterNatives (type, methods, numMethods, null); + } + + [RequiresDynamicCode ("Native method registration via JniNativeMethodRegistration[] requires dynamic code generation. Use the blittable RegisterNatives(JniObjectReference, ReadOnlySpan) overload with statically-compiled function pointers for Native AOT compatibility.")] + internal static unsafe void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration [] methods, int numMethods, JniType? owner) { if ((numMethods < 0) || (numMethods > (methods?.Length ?? 0))) { @@ -243,6 +249,8 @@ public static unsafe void RegisterNatives (JniObjectReference type, JniNativeMet #endif // DEBUG if (numMethods == 0 || methods == null) { + if (methods != null) + owner?.KeepNativeMethodsAlive (methods); return; } @@ -273,6 +281,7 @@ public static unsafe void RegisterNatives (JniObjectReference type, JniNativeMet unmanagedStrings [i * 2 + 1] = sig; natives [i] = new JniNativeMethod ((byte*) name, (byte*) sig, Marshal.GetFunctionPointerForDelegate (m.Marshaler)); } + owner?.KeepNativeMethodsAlive (methods); RegisterNatives (type, natives); // Keep the Marshaler delegates alive at least until JNI has consumed the function pointers. GC.KeepAlive (methods); diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs index d23487846e3..7a34c4acba1 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs @@ -43,6 +43,8 @@ public static bool TryParse (string name, [NotNullWhen (true)] out JniType? type bool registered; JniObjectReference peerReference; + internal bool IsRegisteredWithRuntime => registered; + public JniObjectReference PeerReference { get {return peerReference;} } @@ -151,11 +153,14 @@ public bool IsInstanceOfType (JniObjectReference value) return JniEnvironment.Types.IsInstanceOf (value, PeerReference); } -#pragma warning disable 0414 - // This isn't used anywhere; it's just present so that the GC won't collect the referenced delegates. + // Retains delegates from every batch JNI may have partially registered. JniNativeMethodRegistration[]? methods; -#pragma warning restore 0414 + object? nativeRegistrationLock; + /// + /// Once JNI registration is attempted, the runtime retains this type and its delegates + /// until disposal, even if registration throws: JNI may have registered part of the batch. + /// [RequiresDynamicCode ("Native method registration via JniNativeMethodRegistration[] requires dynamic code generation. Use the blittable RegisterNatives(JniObjectReference, ReadOnlySpan) overload with statically-compiled function pointers for Native AOT compatibility.")] public void RegisterNativeMethods (params JniNativeMethodRegistration[] methods) { @@ -164,10 +169,17 @@ public void RegisterNativeMethods (params JniNativeMethodRegistration[] methods) if (methods == null) throw new ArgumentNullException (nameof (methods)); - JniEnvironment.Types.RegisterNatives (PeerReference, methods, checked ((int)methods.Length)); - // Prevents method delegates from being GC'd so long as this type remains - this.methods = methods; - RegisterWithRuntime (); + JniEnvironment.Types.RegisterNatives (PeerReference, methods, methods.Length, this); + } + + internal void KeepNativeMethodsAlive (JniNativeMethodRegistration[] registrations) + { + lock (LazyInitializer.EnsureInitialized (ref nativeRegistrationLock)) { + // JNI can partially publish a batch before failing. Earlier batches may still be callable too. + var retained = methods == null ? registrations : methods.Concat (registrations).ToArray (); + RegisterWithRuntime (); + methods = retained; + } } public void UnregisterNativeMethods () diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/ManagedPeer.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/ManagedPeer.cs index d026789a180..5d939ebbd71 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/ManagedPeer.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/ManagedPeer.cs @@ -282,30 +282,33 @@ static unsafe void RegisterNativeMembers ( try { var r_nativeClass = new JniObjectReference (n_nativeClass); -#pragma warning disable CA2000 +#pragma warning disable CA2000 // Disposed below unless native registration transfers ownership to the runtime. var nativeClass = new JniType (ref r_nativeClass, JniObjectReferenceOptions.Copy); #pragma warning restore CA2000 + try { + var methodsRef = new JniObjectReference (n_methods); - var methodsRef = new JniObjectReference (n_methods); - - var typeSig = new JniTypeSignature (nativeClass.Name); - var type = GetTypeFromSignature (JniEnvironment.Runtime.TypeManager, typeSig); + var typeSig = new JniTypeSignature (nativeClass.Name); + var type = GetTypeFromSignature (JniEnvironment.Runtime.TypeManager, typeSig); - int methodsLength = JniEnvironment.Strings.GetStringLength (methodsRef); - var methodsChars = JniEnvironment.Strings.GetStringChars (methodsRef, null); - var methods = new ReadOnlySpan(methodsChars, methodsLength); - try { - JniEnvironment.Runtime.TypeManager.RegisterNativeMembers (nativeClass, type, methods); - } - catch (Exception e) { - throw new NotSupportedException ( - $"Unable to register native members for Java type `{nativeClass.Name}` <=> managed type `{type?.AssemblyQualifiedName}`.", - e); - } - finally { - JniEnvironment.Strings.ReleaseStringChars (methodsRef, methodsChars); + int methodsLength = JniEnvironment.Strings.GetStringLength (methodsRef); + var methodsChars = JniEnvironment.Strings.GetStringChars (methodsRef, null); + var methods = new ReadOnlySpan(methodsChars, methodsLength); + try { + JniEnvironment.Runtime.TypeManager.RegisterNativeMembers (nativeClass, type, methods); + } + catch (Exception e) { + throw new NotSupportedException ( + $"Unable to register native members for Java type `{nativeClass.Name}` <=> managed type `{type?.AssemblyQualifiedName}`.", + e); + } + finally { + JniEnvironment.Strings.ReleaseStringChars (methodsRef, methodsChars); + } + } finally { + if (!nativeClass.IsRegisteredWithRuntime) + nativeClass.Dispose (); } - } catch (Exception e) { __r?.OnUserUnhandledException (ref envp, e); diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop-Tests.csproj b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop-Tests.csproj index 1d43a2ca427..b046594a5f5 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop-Tests.csproj +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop-Tests.csproj @@ -39,6 +39,7 @@ + diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs index 99004f98c2b..29cecf3dd42 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs @@ -51,8 +51,17 @@ class JavaVMFixtureTypeManager : JniRuntime.ReflectionJniTypeManager { [JavaObjectWithMissingJavaPeer.JniTypeName] = typeof (JavaObjectWithMissingJavaPeer), [MyDisposableObject.JniTypeName] = typeof (JavaDisposedObject), [MyJavaInterfaceImpl.JniTypeName] = typeof (MyJavaInterfaceImpl), + [ManagedPeerRegistrationTests.JniTypeName] = typeof (ManagedPeerRegistrationTests.Registration), }; + internal Action? NativeRegistrationObserver; + + public override void RegisterNativeMembers (JniType nativeClass, Type type, ReadOnlySpan methods) + { + NativeRegistrationObserver?.Invoke (nativeClass); + base.RegisterNativeMembers (nativeClass, type, methods); + } + public JavaVMFixtureTypeManager () { } diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs new file mode 100644 index 00000000000..a9cee010f06 --- /dev/null +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs @@ -0,0 +1,274 @@ +#nullable enable + +using System; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Threading; +using System.Threading.Tasks; + +using Java.Interop; + +using NUnit.Framework; + +namespace Java.InteropTests { + + [TestFixture] + [NonParallelizable] + [UnconditionalSuppressMessage ("AOT", "IL3050", Justification = "Tests exercise standalone delegate-based native registration.")] + public class ManagedPeerRegistrationTests : JavaVMFixture { + + internal const string JniTypeName = "net/dot/jni/test/ManagedPeerRegistration"; + static Action? addRegistrations; + JniType? registeredClass; + + public class Registration { + [JniAddNativeMethodRegistration] + static void Register (JniNativeMethodRegistrationArguments args) + { + addRegistrations?.Invoke (args); + } + } + + [UnmanagedFunctionPointer (CallingConvention.Winapi)] + delegate int GetValue (IntPtr env, IntPtr klass); + + sealed class NativeTarget { + public int Value (IntPtr env, IntPtr klass) => 42; + } + + [SetUp] + public void SetUp () + { + var manager = TypeManager ?? throw new InvalidOperationException ("The test type manager is not initialized."); + manager.NativeRegistrationObserver = ObserveRegistration; + } + + [TearDown] + public void TearDown () + { + if (TypeManager != null) + TypeManager.NativeRegistrationObserver = null; + addRegistrations = null; + registeredClass?.Dispose (); + registeredClass = null; + } + + [Test] + public void EmptyRegistration_DisposesClass () + { + using var existingOwner = new JniType (JniTypeName); + existingOwner.RegisterNativeMethods ( + new JniNativeMethodRegistration ("existing", "()I", new GetValue (new NativeTarget ().Value))); + Register (); + Assert.IsFalse (GetRegisteredClass ().PeerReference.IsValid); + Assert.AreEqual (42, Call (existingOwner, "existing")); + } + + [Test] + public void FailureBeforeAdoption_DisposesClass () + { + var expected = new InvalidOperationException ("Registration failed before adoption."); + addRegistrations = args => throw expected; + + var error = Assert.Throws (Register); + Assert.AreSame (expected, error?.InnerException); + Assert.IsFalse (GetRegisteredClass ().PeerReference.IsValid); + } + + [TestCase (false)] + [TestCase (true)] + public void MarshalingFailure_DisposesClass (bool genericDelegate) + { + using var existingOwner = new JniType (JniTypeName); + existingOwner.RegisterNativeMethods ( + new JniNativeMethodRegistration ("existing", "()I", new GetValue (new NativeTarget ().Value))); + addRegistrations = args => { + args.Registrations.Add (new JniNativeMethodRegistration ("value", "()I", new GetValue (new NativeTarget ().Value))); + args.Registrations.Add (genericDelegate + ? new JniNativeMethodRegistration ("missing", "()I", new Func (new NativeTarget ().Value)) + : default); + }; + + var error = Assert.Throws (Register); + Assert.That (error?.InnerException, Is.TypeOf ()); + Assert.IsFalse (GetRegisteredClass ().PeerReference.IsValid); + Assert.AreEqual (42, Call (existingOwner, "existing")); + } + + [TestCase (false)] + [TestCase (true)] + public void AdoptedRegistration_RemainsCallable (bool throwAfterAdoption) + { + var weakOwner = RegisterAndReleaseOwner (throwAfterAdoption); + Collect (); + Assert.IsTrue (weakOwner.TryGetTarget (out var owner), "The runtime must retain the registration owner."); + registeredClass = owner ?? throw new InvalidOperationException ("The registration owner was collected."); + Assert.IsTrue (registeredClass.PeerReference.IsValid); + Assert.AreEqual (42, Call (registeredClass, "value")); + } + + [MethodImpl (MethodImplOptions.NoInlining)] + WeakReference RegisterAndReleaseOwner (bool throwAfterAdoption) + { + addRegistrations = args => args.Registrations.Add ( + new JniNativeMethodRegistration ("value", "()I", new GetValue (new NativeTarget ().Value))); + + if (throwAfterAdoption) { + var manager = TypeManager ?? throw new InvalidOperationException ("The test type manager is not initialized."); + var expected = new InvalidOperationException ("Registration failed after adoption."); + manager.NativeRegistrationObserver = nativeClass => { + registeredClass = nativeClass; + nativeClass.RegisterNativeMethods ( + new JniNativeMethodRegistration ("value", "()I", new GetValue (new NativeTarget ().Value))); + throw expected; + }; + var error = Assert.Throws (Register); + Assert.AreSame (expected, error?.InnerException); + } else { + Register (); + } + + return ReleaseOwner (); + } + + [Test] + public void PartialRegistrationFailure_PreservesBothOwners () + { + using var existingOwner = new JniType (JniTypeName); + existingOwner.RegisterNativeMethods ( + new JniNativeMethodRegistration ("existing", "()I", new GetValue (new NativeTarget ().Value))); + var weakOwner = RegisterPartialAndReleaseOwner (existingOwner); + Collect (); + Assert.IsTrue (weakOwner.TryGetTarget (out var owner), "A partial registration must remain runtime-owned."); + registeredClass = owner ?? throw new InvalidOperationException ("The partial registration owner was collected."); + Assert.IsTrue (registeredClass.PeerReference.IsValid); + Assert.AreEqual (42, Call (existingOwner, "existing")); + Assert.AreEqual (42, Call (registeredClass, "value")); + } + + [MethodImpl (MethodImplOptions.NoInlining)] + WeakReference RegisterPartialAndReleaseOwner (JniType existingOwner) + { + addRegistrations = args => { + args.Registrations.Add (new JniNativeMethodRegistration ("value", "()I", new GetValue (new NativeTarget ().Value))); + args.Registrations.Add (new JniNativeMethodRegistration ("missing", "()I", new GetValue (new NativeTarget ().Value))); + }; + + var error = Assert.Throws (Register); + using var cause = error?.InnerException as JavaException; + Assert.IsNotNull (cause); + Assert.AreEqual (42, Call (existingOwner, "existing"), "Failure must not unregister another owner's methods."); + Assert.AreEqual (42, Call (GetRegisteredClass (), "value"), "JNI may register a method before rejecting the batch."); + return ReleaseOwner (); + } + + [TestCase (false)] + [TestCase (true)] + public void RepeatedRegistration_RetainsPreviousAndAttemptedDelegates (bool fail) + { + using var owner = new JniType (JniTypeName); + var targets = RegisterRepeatedly (owner, fail); + Collect (); + Assert.IsTrue (targets.Previous.IsAlive, "The previous registration must remain rooted."); + Assert.IsTrue (targets.Attempted.IsAlive, "The partial registration must remain rooted."); + Assert.AreEqual (42, Call (owner, "existing")); + Assert.AreEqual (42, Call (owner, "value")); + } + + [MethodImpl (MethodImplOptions.NoInlining)] + static (WeakReference Previous, WeakReference Attempted) RegisterRepeatedly (JniType owner, bool fail) + { + var previous = new NativeTarget (); + var attempted = new NativeTarget (); + owner.RegisterNativeMethods (new JniNativeMethodRegistration ("existing", "()I", new GetValue (previous.Value))); + if (fail) { + using var error = Assert.Throws (() => owner.RegisterNativeMethods ( + new JniNativeMethodRegistration ("value", "()I", new GetValue (attempted.Value)), + new JniNativeMethodRegistration ("missing", "()I", new GetValue (attempted.Value)))); + } else { + owner.RegisterNativeMethods (new JniNativeMethodRegistration ("value", "()I", new GetValue (attempted.Value))); + } + return (new WeakReference (previous), new WeakReference (attempted)); + } + + [Test] + public void ConcurrentRegistration_RetainsDelegates () + { + using var owner = new JniType (JniTypeName); + using var barrier = new Barrier (2); + var first = Task.Factory.StartNew (() => RegisterConcurrently (owner, "existing", barrier), + CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default); + var second = Task.Factory.StartNew (() => RegisterConcurrently (owner, "value", barrier), + CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default); + var targets = Task.WhenAll (first, second).GetAwaiter ().GetResult (); + Collect (); + Assert.IsTrue (targets [0].IsAlive); + Assert.IsTrue (targets [1].IsAlive); + Assert.AreEqual (42, Call (owner, "existing")); + Assert.AreEqual (42, Call (owner, "value")); + } + + [MethodImpl (MethodImplOptions.NoInlining)] + static WeakReference RegisterConcurrently (JniType owner, string name, Barrier barrier) + { + var target = new NativeTarget (); + Assert.IsTrue (barrier.SignalAndWait (TimeSpan.FromSeconds (30)), "Concurrent registrars did not reach the barrier."); + owner.RegisterNativeMethods (new JniNativeMethodRegistration (name, "()I", new GetValue (target.Value))); + return new WeakReference (target); + } + + WeakReference ReleaseOwner () + { + var owner = new WeakReference (GetRegisteredClass ()); + registeredClass = null; + addRegistrations = null; + var manager = TypeManager ?? throw new InvalidOperationException ("The test type manager is not initialized."); + manager.NativeRegistrationObserver = ObserveRegistration; + // Clear the reflection registrar's shared list so only the runtime can retain the delegates. + Register (); + return owner; + } + + void ObserveRegistration (JniType nativeClass) => registeredClass = nativeClass; + + static int Call (JniType owner, string name) + { + var method = owner.GetStaticMethod (name, "()I"); + return JniEnvironment.StaticMethods.CallStaticIntMethod (owner.PeerReference, method); + } + + static void Collect () + { + GC.Collect (); + GC.WaitForPendingFinalizers (); + GC.Collect (); + } + + JniType GetRegisteredClass () => + registeredClass ?? throw new InvalidOperationException ("The standalone registrar was not invoked."); + + static unsafe void Register () + { + using var managedPeer = new JniType ("net/dot/jni/ManagedPeer"); + using var nativeClass = new JniType (JniTypeName); + var register = managedPeer.GetStaticMethod ("registerNativeMembers", "(Ljava/lang/Class;Ljava/lang/String;)V"); + var classRef = nativeClass.PeerReference.NewLocalRef (); + var methods = JniEnvironment.Strings.NewString (""); + try { + var args = stackalloc JniArgumentValue [2]; + args [0] = new JniArgumentValue (classRef); + args [1] = new JniArgumentValue (methods); + JniEnvironment.StaticMethods.CallStaticVoidMethod (managedPeer.PeerReference, register, args); + } finally { + try { + Assert.AreEqual (JniObjectReferenceType.Local, JniEnvironment.References.GetObjectRefType (classRef)); + Assert.AreEqual (JniObjectReferenceType.Local, JniEnvironment.References.GetObjectRefType (methods)); + } finally { + JniObjectReference.Dispose (ref classRef); + JniObjectReference.Dispose (ref methods); + } + } + } + } +} diff --git a/external/Java.Interop/tests/Java.Interop-Tests/java/net/dot/jni/test/ManagedPeerRegistration.java b/external/Java.Interop/tests/Java.Interop-Tests/java/net/dot/jni/test/ManagedPeerRegistration.java new file mode 100644 index 00000000000..4e19abf85c8 --- /dev/null +++ b/external/Java.Interop/tests/Java.Interop-Tests/java/net/dot/jni/test/ManagedPeerRegistration.java @@ -0,0 +1,6 @@ +package net.dot.jni.test; + +public class ManagedPeerRegistration { + public static native int existing (); + public static native int value (); +} From 94a89919a0163f75a48f79f5c12a094d4328708f Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 10:15:17 +0200 Subject: [PATCH 07/37] [Mono.Android] Avoid retaining canceled Action callbacks Keep weak runnable values under weak Action keys, preserving every pending post without retaining native-canceled callbacks. Let native owner/token matching determine cancellation and GC determine canceled-peer lifetime rather than disposing potentially queued work. Clean up completed callbacks in finally without removing newer mappings, and serialize removal with terminal disposal. Cover native cancellation ownership, queue survival, repeated/reentrant posts, token/handler identity, exceptions, and cache synchronization. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../ClipDrawable.cs | 7 +- .../Android.Graphics.Drawables/Drawable.cs | 7 +- .../DrawableContainer.cs | 7 +- .../InsetDrawable.cs | 7 +- .../LayerDrawable.cs | 7 +- .../RotateDrawable.cs | 7 +- .../ScaleDrawable.cs | 7 +- src/Mono.Android/Android.OS/Handler.cs | 13 +- src/Mono.Android/Android.Views/View.cs | 13 +- src/Mono.Android/Java.Lang/Thread.cs | 80 ++++- .../Android.OS/CallbackLifetimeTests.cs | 295 ++++++++++++++++++ .../Java.Lang/RunnableCacheTests.cs | 113 +++++++ .../Mono.Android.NET-Tests.csproj | 2 + 13 files changed, 486 insertions(+), 79 deletions(-) create mode 100644 tests/Mono.Android-Tests/Mono.Android-Tests/Android.OS/CallbackLifetimeTests.cs create mode 100644 tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/RunnableCacheTests.cs diff --git a/src/Mono.Android/Android.Graphics.Drawables/ClipDrawable.cs b/src/Mono.Android/Android.Graphics.Drawables/ClipDrawable.cs index e9fa6869fd6..dd92d372c59 100644 --- a/src/Mono.Android/Android.Graphics.Drawables/ClipDrawable.cs +++ b/src/Mono.Android/Android.Graphics.Drawables/ClipDrawable.cs @@ -12,12 +12,7 @@ public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action wh public void UnscheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what) { - var runnable = Java.Lang.Thread.RunnableImplementor.Remove (what); - if (runnable == null) - return; - UnscheduleDrawable (who, runnable); - runnable.Dispose (); + Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleDrawable (who, runnable)); } } } - diff --git a/src/Mono.Android/Android.Graphics.Drawables/Drawable.cs b/src/Mono.Android/Android.Graphics.Drawables/Drawable.cs index 18eca3a1c60..f4e83ab9a56 100644 --- a/src/Mono.Android/Android.Graphics.Drawables/Drawable.cs +++ b/src/Mono.Android/Android.Graphics.Drawables/Drawable.cs @@ -12,12 +12,7 @@ public void ScheduleSelf (Action what, long when) public void UnscheduleSelf (Action what) { - var runnable = Java.Lang.Thread.RunnableImplementor.Remove (what); - if (runnable == null) - return; - UnscheduleSelf (runnable); - runnable.Dispose (); + Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleSelf (runnable)); } } } - diff --git a/src/Mono.Android/Android.Graphics.Drawables/DrawableContainer.cs b/src/Mono.Android/Android.Graphics.Drawables/DrawableContainer.cs index 623712785f3..1bbf66ee73d 100644 --- a/src/Mono.Android/Android.Graphics.Drawables/DrawableContainer.cs +++ b/src/Mono.Android/Android.Graphics.Drawables/DrawableContainer.cs @@ -12,12 +12,7 @@ public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action wh public void UnscheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what) { - var runnable = Java.Lang.Thread.RunnableImplementor.Remove (what); - if (runnable == null) - return; - UnscheduleDrawable (who, runnable); - runnable.Dispose (); + Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleDrawable (who, runnable)); } } } - diff --git a/src/Mono.Android/Android.Graphics.Drawables/InsetDrawable.cs b/src/Mono.Android/Android.Graphics.Drawables/InsetDrawable.cs index ec5b2cc3a7b..52e1c714926 100644 --- a/src/Mono.Android/Android.Graphics.Drawables/InsetDrawable.cs +++ b/src/Mono.Android/Android.Graphics.Drawables/InsetDrawable.cs @@ -12,12 +12,7 @@ public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action wh public void UnscheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what) { - var runnable = Java.Lang.Thread.RunnableImplementor.Remove (what); - if (runnable == null) - return; - UnscheduleDrawable (who, runnable); - runnable.Dispose (); + Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleDrawable (who, runnable)); } } } - diff --git a/src/Mono.Android/Android.Graphics.Drawables/LayerDrawable.cs b/src/Mono.Android/Android.Graphics.Drawables/LayerDrawable.cs index 50256dbfd0b..3807229ef5b 100644 --- a/src/Mono.Android/Android.Graphics.Drawables/LayerDrawable.cs +++ b/src/Mono.Android/Android.Graphics.Drawables/LayerDrawable.cs @@ -12,12 +12,7 @@ public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action wh public void UnscheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what) { - var runnable = Java.Lang.Thread.RunnableImplementor.Remove (what); - if (runnable == null) - return; - UnscheduleDrawable (who, runnable); - runnable.Dispose (); + Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleDrawable (who, runnable)); } } } - diff --git a/src/Mono.Android/Android.Graphics.Drawables/RotateDrawable.cs b/src/Mono.Android/Android.Graphics.Drawables/RotateDrawable.cs index 6d9812e7aed..f28ee041c9b 100644 --- a/src/Mono.Android/Android.Graphics.Drawables/RotateDrawable.cs +++ b/src/Mono.Android/Android.Graphics.Drawables/RotateDrawable.cs @@ -12,12 +12,7 @@ public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action wh public void UnscheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what) { - var runnable = Java.Lang.Thread.RunnableImplementor.Remove (what); - if (runnable == null) - return; - UnscheduleDrawable (who, runnable); - runnable.Dispose (); + Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleDrawable (who, runnable)); } } } - diff --git a/src/Mono.Android/Android.Graphics.Drawables/ScaleDrawable.cs b/src/Mono.Android/Android.Graphics.Drawables/ScaleDrawable.cs index 0011689854d..66d0981fef1 100644 --- a/src/Mono.Android/Android.Graphics.Drawables/ScaleDrawable.cs +++ b/src/Mono.Android/Android.Graphics.Drawables/ScaleDrawable.cs @@ -12,12 +12,7 @@ public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action wh public void UnscheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what) { - var runnable = Java.Lang.Thread.RunnableImplementor.Remove (what); - if (runnable == null) - return; - UnscheduleDrawable (who, runnable); - runnable.Dispose (); + Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleDrawable (who, runnable)); } } } - diff --git a/src/Mono.Android/Android.OS/Handler.cs b/src/Mono.Android/Android.OS/Handler.cs index 40634af756f..047cc1adc8c 100644 --- a/src/Mono.Android/Android.OS/Handler.cs +++ b/src/Mono.Android/Android.OS/Handler.cs @@ -63,20 +63,12 @@ public bool PostDelayed (Action action, long delayMillis) public void RemoveCallbacks (Action action) { - var runnable = Java.Lang.Thread.RunnableImplementor.Remove (action); - if (runnable == null) - return; - RemoveCallbacks (runnable); - runnable.Dispose (); + Java.Lang.Thread.RunnableImplementor.Remove (action, runnable => RemoveCallbacks (runnable)); } public void RemoveCallbacks (Action action, Java.Lang.Object token) { - var runnable = Java.Lang.Thread.RunnableImplementor.Remove (action); - if (runnable == null) - return; - RemoveCallbacks (runnable, token); - runnable.Dispose (); + Java.Lang.Thread.RunnableImplementor.Remove (action, runnable => RemoveCallbacks (runnable, token)); } } @@ -103,4 +95,3 @@ public bool HandleMessage (Message m) } } } - diff --git a/src/Mono.Android/Android.Views/View.cs b/src/Mono.Android/Android.Views/View.cs index b6c48d9d781..9226a33d4cb 100644 --- a/src/Mono.Android/Android.Views/View.cs +++ b/src/Mono.Android/Android.Views/View.cs @@ -66,11 +66,8 @@ public bool PostDelayed (Action action, long delayMillis) public bool RemoveCallbacks (Action action) { - var runnable = Java.Lang.Thread.RunnableImplementor.Remove (action); - if (runnable == null) - return false; - bool result = RemoveCallbacks (runnable); - runnable.Dispose (); + bool result = false; + Java.Lang.Thread.RunnableImplementor.Remove (action, runnable => result |= RemoveCallbacks (runnable)); return result; } @@ -81,11 +78,7 @@ public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action wh public void UnscheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what) { - var runnable = Java.Lang.Thread.RunnableImplementor.Remove (what); - if (runnable == null) - return; - UnscheduleDrawable (who, runnable); - runnable.Dispose (); + Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleDrawable (who, runnable)); } #if ANDROID_11 diff --git a/src/Mono.Android/Java.Lang/Thread.cs b/src/Mono.Android/Java.Lang/Thread.cs index 787bebf4759..0d0ed4a0c18 100644 --- a/src/Mono.Android/Java.Lang/Thread.cs +++ b/src/Mono.Android/Java.Lang/Thread.cs @@ -26,32 +26,80 @@ public RunnableImplementor (Action handler, bool removable) Handler = handler; this.removable = removable; - if (removable) - lock (instances) - instances.AddOrUpdate (handler, this); + if (removable) { + lock (instances) { + var runnables = instances.GetOrCreateValue (handler); + Prune (runnables); + runnables.Add (new WeakReference (this, trackResurrection: true)); + } + } } public void Run () { - if (Handler != null) - Handler (); - if (removable) - lock (instances) - if (Handler != null) - instances.Remove (Handler); - Dispose (); + try { + Handler?.Invoke (); + } finally { + Dispose (); + } } - static ConditionalWeakTable instances = new (); + public new void Dispose () + { + lock (this) + base.Dispose (); + } - public static RunnableImplementor Remove (Action handler) + protected override void Dispose (bool disposing) { - RunnableImplementor result; + if (removable && Handler != null) { + lock (instances) { + if (instances.TryGetValue (Handler, out var runnables)) { + Prune (runnables, this); + if (runnables.Count == 0) + instances.Remove (Handler); + } + } + } + base.Dispose (disposing); + } + + // Java owns queued callbacks. Neither a rooted Action nor native cancellation + // should keep a runnable alive through this lookup table. + static readonly ConditionalWeakTable>> instances = new (); + + static void Prune (List> runnables, RunnableImplementor? completed = null) + { + for (int i = runnables.Count - 1; i >= 0; i--) { + if (!runnables [i].TryGetTarget (out var runnable) || + ReferenceEquals (runnable, completed) || runnable.Handle == IntPtr.Zero) + runnables.RemoveAt (i); + } + } + + public static void Remove (Action handler, Action remove) + { + List pending = new (); lock (instances) { - instances.TryGetValue (handler, out result!); - instances.Remove (handler); + if (!instances.TryGetValue (handler, out var runnables)) + return; + Prune (runnables); + foreach (var reference in runnables) { + if (reference.TryGetTarget (out var runnable)) + pending.Add (runnable); + } + if (runnables.Count == 0) + instances.Remove (handler); + } + + foreach (var runnable in pending) { + lock (runnable) { + if (runnable.Handle != IntPtr.Zero) + remove (runnable); + } } - return result; + // Native removal may not match the handler, token or drawable. Keep the + // weak mapping and let Java reachability determine when disposal is safe. } } diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Android.OS/CallbackLifetimeTests.cs b/tests/Mono.Android-Tests/Mono.Android-Tests/Android.OS/CallbackLifetimeTests.cs new file mode 100644 index 00000000000..e7f72e89b4a --- /dev/null +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Android.OS/CallbackLifetimeTests.cs @@ -0,0 +1,295 @@ +using System; +using System.Diagnostics; +using System.Runtime.CompilerServices; +using System.Runtime.ExceptionServices; +using System.Threading; + +using Android.OS; +using Android.Runtime; +using Java.Interop; + +using NUnit.Framework; + +using RunnableImplementor = Java.Lang.Thread.RunnableImplementor; + +namespace Xamarin.Android.RuntimeTests { + + [TestFixture] + [Category ("CallbackLifetime")] + public class CallbackLifetimeTests { + [TestCase (false)] + [TestCase (true)] + public void NativeCancellationReleasesRunnableWithRootedAction (bool removeAll) + { + using var queue = new CallbackQueue (); + using var token = new Java.Lang.String ("token"); + int calls = 0; + Action action = () => Interlocked.Increment (ref calls); + WeakReference weak = null; + JniObjectReference javaWeak = default; + + try { + OnFreshThread (() => { + var runnable = new RunnableImplementor (action, removable: true); + weak = new WeakReference (runnable, trackResurrection: true); + javaWeak = runnable.PeerReference.NewWeakGlobalRef (); + Assert.IsTrue (queue.Handler.PostAtTime (runnable, token, SystemClock.UptimeMillis ())); + }); + + CollectPeers (); + Assert.IsTrue (IsAlive (weak), "The Java queue must retain the original managed callback."); + Assert.IsFalse (JNIEnv.IsSameObject (javaWeak.Handle, IntPtr.Zero)); + + // This is the generated native binding, not the Action-specific removal helper. + queue.Handler.RemoveCallbacksAndMessages (removeAll ? null : token); + WaitForCollection (() => !IsAlive (weak) && JNIEnv.IsSameObject (javaWeak.Handle, IntPtr.Zero)); + queue.Drain (); + Assert.AreEqual (0, calls); + } finally { + JniObjectReference.Dispose (ref javaWeak); + GC.KeepAlive (action); + } + } + + [Test] + public void JavaQueueKeepsCallbackCallableAcrossCollection () + { + using var queue = new CallbackQueue (); + int calls = 0; + WeakReference weakAction = null; + OnFreshThread (() => { + Action action = () => Interlocked.Increment (ref calls); + weakAction = new WeakReference (action); + Assert.IsTrue (queue.Handler.Post (action)); + }); + + CollectPeers (); + Assert.IsTrue (IsAlive (weakAction), "The queue, not a managed Action root, owns the callback."); + queue.Drain (); + Assert.AreEqual (1, calls); + } + + [Test] + public void CacheDoesNotRootActionOrUnqueuedRunnable () + { + WeakReference weakAction = null; + WeakReference weakRunnable = null; + OnFreshThread (() => { + var target = new object (); + Action action = () => GC.KeepAlive (target); + var runnable = new RunnableImplementor (action, removable: true); + weakAction = new WeakReference (action); + weakRunnable = new WeakReference (runnable, trackResurrection: true); + }); + + WaitForCollection (() => !IsAlive (weakAction) && !IsAlive (weakRunnable)); + } + + [TestCase (false)] + [TestCase (true)] + public void RemoveCallbacksRemovesEveryPost (bool useToken) + { + using var queue = new CallbackQueue (); + using var token = new Java.Lang.String ("token"); + int calls = 0; + int otherCalls = 0; + Action action = () => calls++; + for (int i = 0; i < 3; i++) + Assert.IsTrue (queue.Handler.PostAtTime (action, token, SystemClock.UptimeMillis ())); + Assert.IsTrue (queue.Handler.Post (() => otherCalls++)); + + if (useToken) + queue.Handler.RemoveCallbacks (action, token); + else + queue.Handler.RemoveCallbacks (action); + queue.Drain (); + + Assert.AreEqual (0, calls); + Assert.AreEqual (1, otherCalls); + } + + [Test] + public void WrongTokenDoesNotDisposeOrForgetQueuedCallback () + { + using var queue = new CallbackQueue (); + using var token = new Java.Lang.String ("token"); + using var otherToken = new Java.Lang.String ("other token"); + int calls = 0; + Action action = () => calls++; + using var runnable = new RunnableImplementor (action, removable: true); + Assert.IsTrue (queue.Handler.PostAtTime (runnable, token, SystemClock.UptimeMillis ())); + + queue.Handler.RemoveCallbacks (action, otherToken); + Assert.AreNotEqual (IntPtr.Zero, runnable.Handle, "A token mismatch must not dispose queued work."); + queue.Handler.RemoveCallbacks (action, token); + queue.Drain (); + Assert.AreEqual (0, calls, "A token mismatch must leave the callback removable."); + } + + [TestCase (false)] + [TestCase (true)] + public void RemovingOneTokenPreservesOtherTokenCallbacks (bool removeOther) + { + using var queue = new CallbackQueue (); + using var token = new Java.Lang.String ("token"); + using var otherToken = new Java.Lang.String ("other token"); + int calls = 0; + Action action = () => calls++; + Assert.IsTrue (queue.Handler.PostAtTime (action, token, SystemClock.UptimeMillis ())); + Assert.IsTrue (queue.Handler.PostAtTime (action, otherToken, SystemClock.UptimeMillis ())); + + queue.Handler.RemoveCallbacks (action, token); + if (removeOther) + queue.Handler.RemoveCallbacks (action, otherToken); + queue.Drain (); + Assert.AreEqual (removeOther ? 0 : 1, calls); + } + + [TestCase (false)] + [TestCase (true)] + public void RemovingFromOneHandlerPreservesOtherHandlerCallbacks (bool removeOther) + { + using var queue = new CallbackQueue (); + using var other = new Handler (queue.Handler.Looper); + int calls = 0; + Action action = () => calls++; + Assert.IsTrue (queue.Handler.Post (action)); + Assert.IsTrue (other.Post (action)); + + queue.Handler.RemoveCallbacks (action); + if (removeOther) + other.RemoveCallbacks (action); + queue.Drain (); + Assert.AreEqual (removeOther ? 0 : 1, calls); + } + + [Test] + public void OlderCompletionDoesNotForgetNewerPost () + { + using var queue = new CallbackQueue (); + int calls = 0; + Action action = () => calls++; + using var remove = new Java.Lang.Runnable (() => queue.Handler.RemoveCallbacks (action)); + Assert.IsTrue (queue.Handler.Post (action)); + Assert.IsTrue (queue.Handler.Post (remove)); + Assert.IsTrue (queue.Handler.Post (action)); + + queue.Drain (); + Assert.AreEqual (1, calls); + } + + [Test] + public void SelfRepostingCallbackRemainsRemovable () + { + using var queue = new CallbackQueue (); + int calls = 0; + Action action = null; + using var remove = new Java.Lang.Runnable (() => queue.Handler.RemoveCallbacks (action)); + action = () => { + if (++calls == 1) { + queue.Handler.Post (remove); + queue.Handler.Post (action); + } + }; + Assert.IsTrue (queue.Handler.Post (action)); + + queue.Drain (); + // The first callback queues its removal and repost after the first drain marker. + queue.Drain (); + Assert.AreEqual (1, calls); + } + + static void CollectPeers () + { + int generation = JNIEnv.BridgeProcessingGeneration; + var timeout = Stopwatch.StartNew (); + do { + GC.Collect (); + GC.WaitForPendingFinalizers (); + JNIEnv.WaitForBridgeProcessing (); + Java.Lang.JavaSystem.Gc (); + JniEnvironment.Runtime.ValueManager.CollectPeers (); + JNIEnv.WaitForBridgeProcessing (); + if (Microsoft.Android.Runtime.RuntimeFeature.IsMonoRuntime || + JNIEnv.BridgeProcessingGeneration != generation) + return; + Thread.Sleep (10); + } while (timeout.ElapsedMilliseconds < 5000); + Assert.Fail ("No JNI bridge-processing cycle completed."); + } + + static void WaitForCollection (Func collected) + { + var timeout = Stopwatch.StartNew (); + do { + CollectPeers (); + if (collected ()) + return; + Thread.Sleep (10); + } while (timeout.ElapsedMilliseconds < 10000); + Assert.Fail ("The callback ownership chain was not released."); + } + + [MethodImpl (MethodImplOptions.NoInlining)] + static bool IsAlive (WeakReference weak) where T : class + { + bool alive = false; + OnFreshThread (() => alive = weak.TryGetTarget (out _)); + return alive; + } + + static void OnFreshThread (Action action) + { + // Do not leave callback references on a conservatively scanned Mono test stack. + Exception error = null; + var thread = new Thread (() => { + try { + action (); + } catch (Exception e) { + error = e; + } + }); + thread.Start (); + Assert.IsTrue (thread.Join (TimeSpan.FromSeconds (10)), "The callback operation did not finish."); + if (error != null) + ExceptionDispatchInfo.Capture (error).Throw (); + } + + sealed class CallbackQueue : IDisposable { + readonly HandlerThread thread = new HandlerThread ("CallbackLifetimeTests"); + readonly ManualResetEventSlim release = new ManualResetEventSlim (); + readonly Java.Lang.Runnable blocker; + + public Handler Handler { get; } + + public CallbackQueue () + { + thread.Start (); + Handler = new Handler (thread.Looper); + blocker = new Java.Lang.Runnable (() => release.Wait ()); + Assert.IsTrue (Handler.Post (blocker)); + } + + public void Drain () + { + using var done = new ManualResetEventSlim (); + using var marker = new Java.Lang.Runnable (() => done.Set ()); + Assert.IsTrue (Handler.Post (marker)); + release.Set (); + Assert.IsTrue (done.Wait (TimeSpan.FromSeconds (10)), "The callback queue did not drain."); + } + + public void Dispose () + { + Handler.RemoveCallbacksAndMessages (null); + release.Set (); + thread.Quit (); + thread.Join (); + blocker.Dispose (); + Handler.Dispose (); + thread.Dispose (); + release.Dispose (); + } + } + } +} diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/RunnableCacheTests.cs b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/RunnableCacheTests.cs new file mode 100644 index 00000000000..9db07709a02 --- /dev/null +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/RunnableCacheTests.cs @@ -0,0 +1,113 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +using NUnit.Framework; + +using RunnableImplementor = Java.Lang.Thread.RunnableImplementor; + +namespace Xamarin.Android.RuntimeTests { + + [TestFixture] + [Category ("CallbackLifetime")] + public class RunnableCacheTests { + [TestCase (false)] + [TestCase (true)] + public void HandledExceptionStillDisposesRunnable (bool removable) + { + var expected = new InvalidOperationException ("callback failure"); + Action action = () => throw expected; + using var runnable = new RunnableImplementor (action, removable); + + Assert.AreSame (expected, Assert.Throws (() => runnable.Run ())); + Assert.AreEqual (IntPtr.Zero, runnable.Handle, "Run must clean up even when its exception is handled."); + GC.KeepAlive (action); + } + + [Test] + public void DisposedCallbacksAreNotRemovalCandidates () + { + Action action = () => {}; + using var disposed = new RunnableImplementor (action, removable: true); + disposed.Dispose (); + using var pending = new RunnableImplementor (action, removable: true); + var candidates = new List (); + + RunnableImplementor.Remove (action, candidates.Add); + Assert.AreEqual (1, candidates.Count); + Assert.AreSame (pending, candidates [0]); + Assert.AreNotEqual (IntPtr.Zero, pending.Handle); + } + + [TestCase (false)] + [TestCase (true)] + public void TerminalCleanupOnlyRemovesItsOwnInstance (bool throws) + { + Action action = () => { + if (throws) + throw new InvalidOperationException (); + }; + using var first = new RunnableImplementor (action, removable: true); + using var second = new RunnableImplementor (action, removable: true); + if (throws) + Assert.Throws (() => first.Run ()); + else + first.Run (); + + var candidates = new List (); + RunnableImplementor.Remove (action, candidates.Add); + Assert.AreEqual (1, candidates.Count); + Assert.AreSame (second, candidates [0]); + Assert.AreEqual (IntPtr.Zero, first.Handle); + Assert.AreNotEqual (IntPtr.Zero, second.Handle); + } + + [Test] + public void RemovalUsesSnapshotWithoutHoldingCacheLock () + { + Action action = () => {}; + using var first = new RunnableImplementor (action, removable: true); + RunnableImplementor second = null; + int removals = 0; + try { + RunnableImplementor.Remove (action, runnable => { + Assert.AreSame (first, runnable); + removals++; + var post = Task.Run (() => second = new RunnableImplementor (action, removable: true)); + Assert.IsTrue (post.Wait (TimeSpan.FromSeconds (10)), "Removal must not hold the cache lock."); + }); + Assert.AreEqual (1, removals, "A reentrant post must not be added to an in-progress removal."); + var candidates = new List (); + RunnableImplementor.Remove (action, candidates.Add); + Assert.AreEqual (2, candidates.Count); + Assert.AreSame (first, candidates [0]); + Assert.AreSame (second, candidates [1]); + } finally { + second?.Dispose (); + } + } + + [Test] + public void CompletionDoesNotDisposeDuringRemoval () + { + using var running = new ManualResetEventSlim (); + Action action = () => running.Set (); + using var runnable = new RunnableImplementor (action, removable: true); + Task execution = null; + try { + RunnableImplementor.Remove (action, candidate => { + execution = Task.Run (() => runnable.Run ()); + Assert.IsTrue (running.Wait (TimeSpan.FromSeconds (10))); + Assert.IsFalse (execution.Wait (TimeSpan.FromMilliseconds (100)), + "Completion must wait until the native removal finishes using the peer."); + Assert.AreNotEqual (IntPtr.Zero, candidate.Handle); + }); + } finally { + if (execution != null) + Assert.IsTrue (execution.Wait (TimeSpan.FromSeconds (10))); + } + Assert.AreEqual (IntPtr.Zero, runnable.Handle); + } + } +} diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj b/tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj index c6a2629389c..b6b7ee73cd0 100644 --- a/tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj @@ -127,6 +127,7 @@ + @@ -153,6 +154,7 @@ + From 7e68e0e98d4336395f95f050b0f4118c875c5055 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 10:30:59 +0200 Subject: [PATCH 08/37] [tests] Keep standalone registration fixture off Android Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/ManagedPeerRegistrationTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs index a9cee010f06..99e7a63793d 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs @@ -1,4 +1,5 @@ #nullable enable +#if !__ANDROID__ using System; using System.Diagnostics.CodeAnalysis; @@ -29,6 +30,7 @@ static void Register (JniNativeMethodRegistrationArguments args) addRegistrations?.Invoke (args); } } + #endif // !__ANDROID__ [UnmanagedFunctionPointer (CallingConvention.Winapi)] delegate int GetValue (IntPtr env, IntPtr klass); From bea36b7db71b99ee6a7ec7491d578eaab6805cba Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 11:02:28 +0200 Subject: [PATCH 09/37] [tests] Make gref regressions reliable across test runtimes Bind constructor-race worker threads to the instrumented runtime, guard the complete standalone registration fixture on Android, and assert the shared activation-failure contract without assuming a Mono-specific inner exception. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/JniSubclassConstructorCacheTests.cs | 6 +++++- .../Java.Interop/ManagedPeerRegistrationTests.cs | 3 +-- .../Java.Interop/TransferredReferenceTests.cs | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniSubclassConstructorCacheTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniSubclassConstructorCacheTests.cs index 2c853bb522a..185b841c819 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniSubclassConstructorCacheTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniSubclassConstructorCacheTests.cs @@ -26,7 +26,11 @@ public void ConcurrentCreationDisposesUnpublishedConstructors () var calls = Enumerable.Range (0, count) .Select (_ => Task.Factory.StartNew ( - () => members.InstanceMethods.GetConstructorsForType (typeof (MyString)), + () => { + runtime.AttachCurrentThread (); + Assert.AreSame (runtime, JniEnvironment.Runtime); + return members.InstanceMethods.GetConstructorsForType (typeof (MyString)); + }, CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default)) .ToArray (); var constructors = Task.WhenAll (calls).GetAwaiter ().GetResult (); diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs index 99e7a63793d..3a14e0b5362 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs @@ -30,8 +30,6 @@ static void Register (JniNativeMethodRegistrationArguments args) addRegistrations?.Invoke (args); } } - #endif // !__ANDROID__ - [UnmanagedFunctionPointer (CallingConvention.Winapi)] delegate int GetValue (IntPtr env, IntPtr klass); @@ -274,3 +272,4 @@ static unsafe void Register () } } } +#endif // !__ANDROID__ diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/TransferredReferenceTests.cs b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/TransferredReferenceTests.cs index ac7ae7a2d67..2b3814fe701 100644 --- a/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/TransferredReferenceTests.cs +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/TransferredReferenceTests.cs @@ -57,7 +57,7 @@ public void GetObject_MissingActivationConstructor ( var exception = Assert.Throws (() => Java.Lang.Object.GetObject (input.Handle, transfer)); - Assert.IsInstanceOf (exception.InnerException); + StringAssert.Contains (typeof (MissingTransferredReferencePeer).FullName, exception.Message); input.AssertOwnership (transfer); } From 9cd00ba56ea2a7316346bea0d040b14fd5455a2c Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 12:41:09 +0200 Subject: [PATCH 10/37] [tests] Tolerate JDK-specific module constant pools Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../ExpectedTypeDeclaration.cs | 7 ++++--- .../ModuleInfoTests.cs | 3 +-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ExpectedTypeDeclaration.cs b/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ExpectedTypeDeclaration.cs index d45cf6d4bf9..ca25a1ffe2d 100644 --- a/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ExpectedTypeDeclaration.cs +++ b/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ExpectedTypeDeclaration.cs @@ -11,7 +11,7 @@ class ExpectedTypeDeclaration { public ushort MajorVersion; public ushort MinorVersion; - public int ConstantPoolCount; + public int? ConstantPoolCount; public ClassAccessFlags AccessFlags; public string FullName; public TypeInfo Superclass; @@ -26,7 +26,8 @@ public void Assert (ClassFile classDeclaration) { NAssert.AreEqual (MajorVersion, classDeclaration.MajorVersion, FullName + " Major Version"); NAssert.AreEqual (MinorVersion, classDeclaration.MinorVersion, FullName + " Minor Version"); - NAssert.AreEqual (ConstantPoolCount, classDeclaration.ConstantPool.Count, FullName + " ConstantPool Count"); + if (ConstantPoolCount.HasValue) + NAssert.AreEqual (ConstantPoolCount.Value, classDeclaration.ConstantPool.Count, FullName + " ConstantPool Count"); NAssert.AreEqual (AccessFlags, classDeclaration.AccessFlags, FullName + " AccessFlags"); NAssert.AreEqual (FullName, classDeclaration.ThisClass.Name.Value, FullName + " Name"); NAssert.AreEqual (Superclass?.BinaryName, classDeclaration?.SuperClass?.Name?.Value, FullName + " SuperClass Name"); @@ -58,6 +59,7 @@ public void Assert (ClassFile classDeclaration) } } + NAssert.AreEqual (Fields.Count, classDeclaration.Fields.Count, FullName + " Fields Count"); for (int i = 0; i < Fields.Count; ++i) { Fields [i].Assert (classDeclaration.Fields [i]); @@ -70,4 +72,3 @@ public void Assert (ClassFile classDeclaration) } } } - diff --git a/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ModuleInfoTests.cs b/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ModuleInfoTests.cs index 035cc708352..f9aea0b09e1 100644 --- a/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ModuleInfoTests.cs +++ b/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ModuleInfoTests.cs @@ -18,7 +18,6 @@ public void ClassFile () new ExpectedTypeDeclaration { MajorVersion = 0x37, MinorVersion = 0, - ConstantPoolCount = 12, AccessFlags = ClassAccessFlags.Module, FullName = "module-info", }.Assert (c); @@ -41,5 +40,5 @@ public void ClassFile () Assert.AreEqual ("com/xamarin", moduleAttr.Exports [0].Exports); } } -} +} From 32f9b47c86030894099a4512ba3c7d70a1b7677d Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 12:42:42 +0200 Subject: [PATCH 11/37] [tests] Clean up bytecode test formatting Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../ExpectedTypeDeclaration.cs | 1 - .../Xamarin.Android.Tools.Bytecode-Tests/ModuleInfoTests.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ExpectedTypeDeclaration.cs b/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ExpectedTypeDeclaration.cs index ca25a1ffe2d..b8f51492712 100644 --- a/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ExpectedTypeDeclaration.cs +++ b/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ExpectedTypeDeclaration.cs @@ -59,7 +59,6 @@ public void Assert (ClassFile classDeclaration) } } - NAssert.AreEqual (Fields.Count, classDeclaration.Fields.Count, FullName + " Fields Count"); for (int i = 0; i < Fields.Count; ++i) { Fields [i].Assert (classDeclaration.Fields [i]); diff --git a/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ModuleInfoTests.cs b/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ModuleInfoTests.cs index f9aea0b09e1..9e7f6a59e16 100644 --- a/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ModuleInfoTests.cs +++ b/external/Java.Interop/tests/Xamarin.Android.Tools.Bytecode-Tests/ModuleInfoTests.cs @@ -40,5 +40,4 @@ public void ClassFile () Assert.AreEqual ("com/xamarin", moduleAttr.Exports [0].Exports); } } - } From 37f9bbff5f3b158100c865460854da88f8651b22 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 12:44:34 +0200 Subject: [PATCH 12/37] [Java.Interop] Keep empty native registration a no-op Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/Java.Interop/Java.Interop/JniEnvironment.Types.cs | 5 +---- .../Java.Interop/ManagedPeerRegistrationTests.cs | 8 ++++++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs index f52f06a2acb..c9300981b9e 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs @@ -248,11 +248,8 @@ internal static unsafe void RegisterNatives (JniObjectReference type, JniNativeM } #endif // DEBUG - if (numMethods == 0 || methods == null) { - if (methods != null) - owner?.KeepNativeMethodsAlive (methods); + if (numMethods == 0 || methods == null) return; - } // Marshal the non-blittable JniNativeMethodRegistration[] into blittable JniNativeMethod // values and dispatch to the blittable overload, instead of invoking the JNI diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs index 3a14e0b5362..dc48bba2cb3 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs @@ -54,6 +54,14 @@ public void TearDown () registeredClass = null; } + [Test] + public void EmptyJniTypeRegistration_DoesNotAdoptOwner () + { + using var owner = new JniType (JniTypeName); + owner.RegisterNativeMethods (); + Assert.IsFalse (owner.IsRegisteredWithRuntime); + } + [Test] public void EmptyRegistration_DisposesClass () { From addc53d6de8c42201cfe15cd79ae82168ac5ee6b Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 12:45:41 +0200 Subject: [PATCH 13/37] [Java.Interop] Preserve empty-registration formatting Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/Java.Interop/Java.Interop/JniEnvironment.Types.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs index c9300981b9e..ff567036475 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs @@ -248,8 +248,9 @@ internal static unsafe void RegisterNatives (JniObjectReference type, JniNativeM } #endif // DEBUG - if (numMethods == 0 || methods == null) + if (numMethods == 0 || methods == null) { return; + } // Marshal the non-blittable JniNativeMethodRegistration[] into blittable JniNativeMethod // values and dispatch to the blittable overload, instead of invoking the JNI From efb73940dce9f4cd1de37d399bdc37f11faeb1a0 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 12:49:00 +0200 Subject: [PATCH 14/37] [Java.Interop] Explain method cache publication Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/JniPeerMembers.JniMethodInfoCache.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs index dbb69771c24..e30f2284f4c 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs @@ -12,8 +12,9 @@ static JniMethodInfo GetOrAddMethodInfo (ConcurrentDictionary Date: Fri, 11 Sep 2026 13:01:50 +0200 Subject: [PATCH 15/37] [Mono.Android] Avoid callback removal closure Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/Mono.Android/Android.Views/View.cs | 7 ++++--- src/Mono.Android/Java.Lang/Thread.cs | 14 +++++++++++-- .../Java.Lang/RunnableCacheTests.cs | 20 +++++++++++++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/Mono.Android/Android.Views/View.cs b/src/Mono.Android/Android.Views/View.cs index 9226a33d4cb..5348ad2a258 100644 --- a/src/Mono.Android/Android.Views/View.cs +++ b/src/Mono.Android/Android.Views/View.cs @@ -66,9 +66,10 @@ public bool PostDelayed (Action action, long delayMillis) public bool RemoveCallbacks (Action action) { - bool result = false; - Java.Lang.Thread.RunnableImplementor.Remove (action, runnable => result |= RemoveCallbacks (runnable)); - return result; + return Java.Lang.Thread.RunnableImplementor.Remove ( + action, + this, + static (view, runnable) => view.RemoveCallbacks (runnable)); } public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what, long when) diff --git a/src/Mono.Android/Java.Lang/Thread.cs b/src/Mono.Android/Java.Lang/Thread.cs index 0d0ed4a0c18..ff070b18417 100644 --- a/src/Mono.Android/Java.Lang/Thread.cs +++ b/src/Mono.Android/Java.Lang/Thread.cs @@ -78,11 +78,19 @@ static void Prune (List> runnables, RunnableI } public static void Remove (Action handler, Action remove) + { + Remove (handler, remove, static (callback, runnable) => { + callback (runnable); + return false; + }); + } + + public static bool Remove (Action handler, TState state, Func remove) { List pending = new (); lock (instances) { if (!instances.TryGetValue (handler, out var runnables)) - return; + return false; Prune (runnables); foreach (var reference in runnables) { if (reference.TryGetTarget (out var runnable)) @@ -92,14 +100,16 @@ public static void Remove (Action handler, Action remove) instances.Remove (handler); } + bool result = false; foreach (var runnable in pending) { lock (runnable) { if (runnable.Handle != IntPtr.Zero) - remove (runnable); + result |= remove (state, runnable); } } // Native removal may not match the handler, token or drawable. Keep the // weak mapping and let Java reachability determine when disposal is safe. + return result; } } diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/RunnableCacheTests.cs b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/RunnableCacheTests.cs index 9db07709a02..de21313f7ee 100644 --- a/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/RunnableCacheTests.cs +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/RunnableCacheTests.cs @@ -63,6 +63,26 @@ public void TerminalCleanupOnlyRemovesItsOwnInstance (bool throws) Assert.AreNotEqual (IntPtr.Zero, second.Handle); } + [Test] + public void RemovalAggregatesCallbackResults () + { + Action action = () => {}; + using var first = new RunnableImplementor (action, removable: true); + using var second = new RunnableImplementor (action, removable: true); + var callbacks = new List (); + + bool result = RunnableImplementor.Remove ( + action, + callbacks, + static (items, runnable) => { + items.Add (runnable); + return items.Count == 2; + }); + + Assert.IsTrue (result); + Assert.AreEqual (2, callbacks.Count); + } + [Test] public void RemovalUsesSnapshotWithoutHoldingCacheLock () { From c77de9ad218e191aa85a94253aa33fac5eae65bf Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:04:34 +0200 Subject: [PATCH 16/37] [tests] Keep native registration coverage focused Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/JavaVMFixture.cs | 9 - .../ManagedPeerRegistrationTests.cs | 183 ++---------------- 2 files changed, 18 insertions(+), 174 deletions(-) diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs index 29cecf3dd42..99004f98c2b 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs @@ -51,17 +51,8 @@ class JavaVMFixtureTypeManager : JniRuntime.ReflectionJniTypeManager { [JavaObjectWithMissingJavaPeer.JniTypeName] = typeof (JavaObjectWithMissingJavaPeer), [MyDisposableObject.JniTypeName] = typeof (JavaDisposedObject), [MyJavaInterfaceImpl.JniTypeName] = typeof (MyJavaInterfaceImpl), - [ManagedPeerRegistrationTests.JniTypeName] = typeof (ManagedPeerRegistrationTests.Registration), }; - internal Action? NativeRegistrationObserver; - - public override void RegisterNativeMembers (JniType nativeClass, Type type, ReadOnlySpan methods) - { - NativeRegistrationObserver?.Invoke (nativeClass); - base.RegisterNativeMembers (nativeClass, type, methods); - } - public JavaVMFixtureTypeManager () { } diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs index dc48bba2cb3..b5bbf565467 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs @@ -19,17 +19,8 @@ namespace Java.InteropTests { [UnconditionalSuppressMessage ("AOT", "IL3050", Justification = "Tests exercise standalone delegate-based native registration.")] public class ManagedPeerRegistrationTests : JavaVMFixture { - internal const string JniTypeName = "net/dot/jni/test/ManagedPeerRegistration"; - static Action? addRegistrations; - JniType? registeredClass; + const string JniTypeName = "net/dot/jni/test/ManagedPeerRegistration"; - public class Registration { - [JniAddNativeMethodRegistration] - static void Register (JniNativeMethodRegistrationArguments args) - { - addRegistrations?.Invoke (args); - } - } [UnmanagedFunctionPointer (CallingConvention.Winapi)] delegate int GetValue (IntPtr env, IntPtr klass); @@ -37,138 +28,40 @@ sealed class NativeTarget { public int Value (IntPtr env, IntPtr klass) => 42; } - [SetUp] - public void SetUp () - { - var manager = TypeManager ?? throw new InvalidOperationException ("The test type manager is not initialized."); - manager.NativeRegistrationObserver = ObserveRegistration; - } - - [TearDown] - public void TearDown () - { - if (TypeManager != null) - TypeManager.NativeRegistrationObserver = null; - addRegistrations = null; - registeredClass?.Dispose (); - registeredClass = null; - } - [Test] - public void EmptyJniTypeRegistration_DoesNotAdoptOwner () + public void EmptyRegistration_DoesNotAdoptOwner () { using var owner = new JniType (JniTypeName); owner.RegisterNativeMethods (); Assert.IsFalse (owner.IsRegisteredWithRuntime); } - [Test] - public void EmptyRegistration_DisposesClass () - { - using var existingOwner = new JniType (JniTypeName); - existingOwner.RegisterNativeMethods ( - new JniNativeMethodRegistration ("existing", "()I", new GetValue (new NativeTarget ().Value))); - Register (); - Assert.IsFalse (GetRegisteredClass ().PeerReference.IsValid); - Assert.AreEqual (42, Call (existingOwner, "existing")); - } - - [Test] - public void FailureBeforeAdoption_DisposesClass () - { - var expected = new InvalidOperationException ("Registration failed before adoption."); - addRegistrations = args => throw expected; - - var error = Assert.Throws (Register); - Assert.AreSame (expected, error?.InnerException); - Assert.IsFalse (GetRegisteredClass ().PeerReference.IsValid); - } - - [TestCase (false)] - [TestCase (true)] - public void MarshalingFailure_DisposesClass (bool genericDelegate) - { - using var existingOwner = new JniType (JniTypeName); - existingOwner.RegisterNativeMethods ( - new JniNativeMethodRegistration ("existing", "()I", new GetValue (new NativeTarget ().Value))); - addRegistrations = args => { - args.Registrations.Add (new JniNativeMethodRegistration ("value", "()I", new GetValue (new NativeTarget ().Value))); - args.Registrations.Add (genericDelegate - ? new JniNativeMethodRegistration ("missing", "()I", new Func (new NativeTarget ().Value)) - : default); - }; - - var error = Assert.Throws (Register); - Assert.That (error?.InnerException, Is.TypeOf ()); - Assert.IsFalse (GetRegisteredClass ().PeerReference.IsValid); - Assert.AreEqual (42, Call (existingOwner, "existing")); - } - [TestCase (false)] [TestCase (true)] - public void AdoptedRegistration_RemainsCallable (bool throwAfterAdoption) + public void RegistrationAttempt_RetainsOwnerAndDelegate (bool fail) { - var weakOwner = RegisterAndReleaseOwner (throwAfterAdoption); + var retained = RegisterAndReleaseOwner (fail); Collect (); - Assert.IsTrue (weakOwner.TryGetTarget (out var owner), "The runtime must retain the registration owner."); - registeredClass = owner ?? throw new InvalidOperationException ("The registration owner was collected."); - Assert.IsTrue (registeredClass.PeerReference.IsValid); - Assert.AreEqual (42, Call (registeredClass, "value")); + Assert.IsTrue (retained.Owner.TryGetTarget (out var owner), "The runtime must retain the registration owner."); + Assert.IsTrue (retained.Target.IsAlive, "The runtime must retain the registered delegate."); + using var retainedOwner = owner ?? throw new InvalidOperationException ("The registration owner was collected."); + Assert.IsTrue (retainedOwner.PeerReference.IsValid); + Assert.AreEqual (42, Call (retainedOwner, "value")); } [MethodImpl (MethodImplOptions.NoInlining)] - WeakReference RegisterAndReleaseOwner (bool throwAfterAdoption) + static (WeakReference Owner, WeakReference Target) RegisterAndReleaseOwner (bool fail) { - addRegistrations = args => args.Registrations.Add ( - new JniNativeMethodRegistration ("value", "()I", new GetValue (new NativeTarget ().Value))); - - if (throwAfterAdoption) { - var manager = TypeManager ?? throw new InvalidOperationException ("The test type manager is not initialized."); - var expected = new InvalidOperationException ("Registration failed after adoption."); - manager.NativeRegistrationObserver = nativeClass => { - registeredClass = nativeClass; - nativeClass.RegisterNativeMethods ( - new JniNativeMethodRegistration ("value", "()I", new GetValue (new NativeTarget ().Value))); - throw expected; - }; - var error = Assert.Throws (Register); - Assert.AreSame (expected, error?.InnerException); + var owner = new JniType (JniTypeName); + var target = new NativeTarget (); + if (fail) { + using var error = Assert.Throws (() => owner.RegisterNativeMethods ( + new JniNativeMethodRegistration ("value", "()I", new GetValue (target.Value)), + new JniNativeMethodRegistration ("missing", "()I", new GetValue (target.Value)))); } else { - Register (); + owner.RegisterNativeMethods (new JniNativeMethodRegistration ("value", "()I", new GetValue (target.Value))); } - - return ReleaseOwner (); - } - - [Test] - public void PartialRegistrationFailure_PreservesBothOwners () - { - using var existingOwner = new JniType (JniTypeName); - existingOwner.RegisterNativeMethods ( - new JniNativeMethodRegistration ("existing", "()I", new GetValue (new NativeTarget ().Value))); - var weakOwner = RegisterPartialAndReleaseOwner (existingOwner); - Collect (); - Assert.IsTrue (weakOwner.TryGetTarget (out var owner), "A partial registration must remain runtime-owned."); - registeredClass = owner ?? throw new InvalidOperationException ("The partial registration owner was collected."); - Assert.IsTrue (registeredClass.PeerReference.IsValid); - Assert.AreEqual (42, Call (existingOwner, "existing")); - Assert.AreEqual (42, Call (registeredClass, "value")); - } - - [MethodImpl (MethodImplOptions.NoInlining)] - WeakReference RegisterPartialAndReleaseOwner (JniType existingOwner) - { - addRegistrations = args => { - args.Registrations.Add (new JniNativeMethodRegistration ("value", "()I", new GetValue (new NativeTarget ().Value))); - args.Registrations.Add (new JniNativeMethodRegistration ("missing", "()I", new GetValue (new NativeTarget ().Value))); - }; - - var error = Assert.Throws (Register); - using var cause = error?.InnerException as JavaException; - Assert.IsNotNull (cause); - Assert.AreEqual (42, Call (existingOwner, "existing"), "Failure must not unregister another owner's methods."); - Assert.AreEqual (42, Call (GetRegisteredClass (), "value"), "JNI may register a method before rejecting the batch."); - return ReleaseOwner (); + return (new WeakReference (owner), new WeakReference (target)); } [TestCase (false)] @@ -226,20 +119,6 @@ static WeakReference RegisterConcurrently (JniType owner, string name, Barrier b return new WeakReference (target); } - WeakReference ReleaseOwner () - { - var owner = new WeakReference (GetRegisteredClass ()); - registeredClass = null; - addRegistrations = null; - var manager = TypeManager ?? throw new InvalidOperationException ("The test type manager is not initialized."); - manager.NativeRegistrationObserver = ObserveRegistration; - // Clear the reflection registrar's shared list so only the runtime can retain the delegates. - Register (); - return owner; - } - - void ObserveRegistration (JniType nativeClass) => registeredClass = nativeClass; - static int Call (JniType owner, string name) { var method = owner.GetStaticMethod (name, "()I"); @@ -252,32 +131,6 @@ static void Collect () GC.WaitForPendingFinalizers (); GC.Collect (); } - - JniType GetRegisteredClass () => - registeredClass ?? throw new InvalidOperationException ("The standalone registrar was not invoked."); - - static unsafe void Register () - { - using var managedPeer = new JniType ("net/dot/jni/ManagedPeer"); - using var nativeClass = new JniType (JniTypeName); - var register = managedPeer.GetStaticMethod ("registerNativeMembers", "(Ljava/lang/Class;Ljava/lang/String;)V"); - var classRef = nativeClass.PeerReference.NewLocalRef (); - var methods = JniEnvironment.Strings.NewString (""); - try { - var args = stackalloc JniArgumentValue [2]; - args [0] = new JniArgumentValue (classRef); - args [1] = new JniArgumentValue (methods); - JniEnvironment.StaticMethods.CallStaticVoidMethod (managedPeer.PeerReference, register, args); - } finally { - try { - Assert.AreEqual (JniObjectReferenceType.Local, JniEnvironment.References.GetObjectRefType (classRef)); - Assert.AreEqual (JniObjectReferenceType.Local, JniEnvironment.References.GetObjectRefType (methods)); - } finally { - JniObjectReference.Dispose (ref classRef); - JniObjectReference.Dispose (ref methods); - } - } - } } } #endif // !__ANDROID__ From b35b5cbae8e7512dd46fed13f54a27f9febae135 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:13:11 +0200 Subject: [PATCH 17/37] [Java.Interop] Explain native registration roots Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/src/Java.Interop/Java.Interop/JniType.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs index 7a34c4acba1..4fc1f3c65e1 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs @@ -175,7 +175,11 @@ public void RegisterNativeMethods (params JniNativeMethodRegistration[] methods) internal void KeepNativeMethodsAlive (JniNativeMethodRegistration[] registrations) { lock (LazyInitializer.EnsureInitialized (ref nativeRegistrationLock)) { - // JNI can partially publish a batch before failing. Earlier batches may still be callable too. + // RegisterNatives stores unmanaged function pointers without retaining the + // managed delegates behind them. Root every attempted batch because JNI can + // publish part of a failing batch, and earlier batches remain callable after + // later registrations. Runtime tracking also keeps this JniType alive until + // disposal, when its native methods are unregistered. var retained = methods == null ? registrations : methods.Concat (registrations).ToArray (); RegisterWithRuntime (); methods = retained; From 30ced499b840c4414cd66065998506e221e1b951 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:15:24 +0200 Subject: [PATCH 18/37] [Java.Interop] Clarify registration retention naming Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/Java.Interop/Java.Interop/JniEnvironment.Types.cs | 2 +- external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs index ff567036475..a938b841306 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs @@ -279,7 +279,7 @@ internal static unsafe void RegisterNatives (JniObjectReference type, JniNativeM unmanagedStrings [i * 2 + 1] = sig; natives [i] = new JniNativeMethod ((byte*) name, (byte*) sig, Marshal.GetFunctionPointerForDelegate (m.Marshaler)); } - owner?.KeepNativeMethodsAlive (methods); + owner?.RetainNativeMethodRegistrations (methods); RegisterNatives (type, natives); // Keep the Marshaler delegates alive at least until JNI has consumed the function pointers. GC.KeepAlive (methods); diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs index 4fc1f3c65e1..f40ff78f053 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs @@ -172,7 +172,7 @@ public void RegisterNativeMethods (params JniNativeMethodRegistration[] methods) JniEnvironment.Types.RegisterNatives (PeerReference, methods, methods.Length, this); } - internal void KeepNativeMethodsAlive (JniNativeMethodRegistration[] registrations) + internal void RetainNativeMethodRegistrations (JniNativeMethodRegistration[] registrations) { lock (LazyInitializer.EnsureInitialized (ref nativeRegistrationLock)) { // RegisterNatives stores unmanaged function pointers without retaining the From b43909a17571dfcab30cf542e5701708b20cb85e Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:18:10 +0200 Subject: [PATCH 19/37] [Java.Interop] Encapsulate method cache ownership Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../JniPeerMembers.JniInstanceMethods.cs | 8 +-- .../JniPeerMembers.JniMethodInfoCache.cs | 71 ++++++++++++++----- .../JniPeerMembers.JniStaticMethods.cs | 8 +-- .../JniRedirectCacheOwnershipTests.cs | 36 +++------- 4 files changed, 72 insertions(+), 51 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs index 213b7e4ebcb..56a92209422 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs @@ -39,15 +39,15 @@ internal JniType JniPeerType { readonly Type DeclaringType; - ConcurrentDictionary? instanceMethods; + JniMethodInfoCache? instanceMethods; ConcurrentDictionary? subclassConstructors; - ConcurrentDictionary InstanceMethods => GetOrCreate (ref instanceMethods, 3); + JniMethodInfoCache InstanceMethods => JniMethodInfoCache.GetOrCreate (ref instanceMethods); ConcurrentDictionary SubclassConstructors => GetOrCreate (ref subclassConstructors, 1); internal void Dispose () { - Clear (ref instanceMethods, static value => value.StaticRedirect?.Dispose ()); + JniMethodInfoCache.Dispose (ref instanceMethods); Clear (ref subclassConstructors, static value => value.Dispose ()); if (jniPeerType != null) @@ -107,7 +107,7 @@ internal JniInstanceMethods GetConstructorsForType (Type declaringType) public JniMethodInfo GetMethodInfo (string encodedMember) { - return GetOrAddMethodInfo (InstanceMethods, encodedMember, static (member, methods) => { + return InstanceMethods.GetOrAdd (encodedMember, static (member, methods) => { ReadOnlySpan method, signature; JniPeerMembers.GetNameAndSignature (member, out method, out signature); return methods.GetMethodInfo (method, signature); diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs index e30f2284f4c..6d15a40b138 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs @@ -2,27 +2,66 @@ using System; using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Threading; namespace Java.Interop { partial class JniPeerMembers { - static JniMethodInfo GetOrAddMethodInfo (ConcurrentDictionary cache, string member, Func factory, TArg argument) - { - if (cache.TryGetValue (member, out var method)) - return method; - - // ConcurrentDictionary may invoke a GetOrAdd factory multiple times and discard - // losing values. Construct explicitly so an unpublished StaticRedirect owner can - // be disposed. JNI lookup can also reenter this cache, so do not lock construction. - var candidate = factory (member, argument); - try { - method = cache.GetOrAdd (member, candidate); - if (ReferenceEquals (method, candidate)) - candidate = null; - return method; - } finally { - candidate?.StaticRedirect?.Dispose (); + internal sealed class JniMethodInfoCache : IDisposable { + + readonly ConcurrentDictionary methods; + + public JniMethodInfoCache () + { + methods = new ConcurrentDictionary (1, 3); + } + + internal JniMethodInfoCache (IEqualityComparer comparer) + { + methods = new ConcurrentDictionary (comparer); + } + + internal static JniMethodInfoCache GetOrCreate (ref JniMethodInfoCache? cache) + { + var value = Volatile.Read (ref cache); + if (value != null) + return value; + + var candidate = new JniMethodInfoCache (); + return Interlocked.CompareExchange (ref cache, candidate, null) ?? candidate; + } + + internal static void Dispose (ref JniMethodInfoCache? cache) + { + Interlocked.Exchange (ref cache, null)?.Dispose (); + } + + public JniMethodInfo GetOrAdd (string member, Func factory, TArg argument) + { + if (methods.TryGetValue (member, out var method)) + return method; + + // ConcurrentDictionary may invoke a GetOrAdd factory multiple times and discard + // losing values. Construct explicitly so an unpublished StaticRedirect owner can + // be disposed. JNI lookup can also reenter this cache, so do not lock construction. + var candidate = factory (member, argument); + try { + method = methods.GetOrAdd (member, candidate); + if (ReferenceEquals (method, candidate)) + candidate = null; + return method; + } finally { + candidate?.StaticRedirect?.Dispose (); + } + } + + public void Dispose () + { + foreach (var method in methods.Values) + method.StaticRedirect?.Dispose (); + methods.Clear (); } } } diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs index 9067dd47fdf..7799f688100 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs @@ -15,18 +15,18 @@ internal JniStaticMethods (JniPeerMembers members) internal readonly JniPeerMembers Members; - ConcurrentDictionary? staticMethods; + JniMethodInfoCache? staticMethods; - ConcurrentDictionary StaticMethods => GetOrCreate (ref staticMethods, 3); + JniMethodInfoCache StaticMethods => JniMethodInfoCache.GetOrCreate (ref staticMethods); internal void Dispose () { - Clear (ref staticMethods, static value => value.StaticRedirect?.Dispose ()); + JniMethodInfoCache.Dispose (ref staticMethods); } public JniMethodInfo GetMethodInfo (string encodedMember) { - return GetOrAddMethodInfo (StaticMethods, encodedMember, static (member, methods) => { + return StaticMethods.GetOrAdd (encodedMember, static (member, methods) => { ReadOnlySpan method, signature; JniPeerMembers.GetNameAndSignature (member, out method, out signature); return methods.GetMethodInfo (method, signature); diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs index ae49303ef2c..90b7285fd08 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs @@ -1,8 +1,5 @@ using System; -using System.Collections.Concurrent; using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; -using System.Reflection; using System.Threading; using System.Threading.Tasks; @@ -75,28 +72,26 @@ public void DisposingOrdinaryMethodCacheDoesNotDisposePeerType (bool isStatic) [Test] public void ConcurrentPublicationDisposesOnlyLosingRedirects () { - var lookup = GetMethodLookup (); - var cache = new ConcurrentDictionary (); + using var cache = new JniPeerMembers.JniMethodInfoCache (); var candidates = new JniMethodInfo [2]; var results = new JniMethodInfo [candidates.Length]; using var ready = new Barrier (candidates.Length); try { Parallel.For (0, candidates.Length, i => { - results [i] = lookup (cache, "currentTimeMillis.()J", (member, index) => { + results [i] = cache.GetOrAdd ("currentTimeMillis.()J", (member, index) => { candidates [index] = CreateRedirect (); if (!ready.SignalAndWait (TimeSpan.FromSeconds (30))) throw new TimeoutException ("Both candidates must be created before publication."); return candidates [index]; }, i); }); - var winner = results [0]; Assert.AreSame (winner, results [1]); - Assert.AreEqual (1, cache.Count); + Assert.AreSame (winner, results [1]); foreach (var candidate in candidates) Assert.AreEqual (ReferenceEquals (candidate, winner), candidate.StaticRedirect.PeerReference.IsValid); AssertSystemRedirectIsCallable (winner); - Assert.AreSame (winner, lookup (cache, "currentTimeMillis.()J", + Assert.AreSame (winner, cache.GetOrAdd ("currentTimeMillis.()J", (member, state) => throw new InvalidOperationException ("A cache hit must not construct a candidate."), 0)); } finally { foreach (var candidate in candidates) @@ -108,13 +103,12 @@ public void ConcurrentPublicationDisposesOnlyLosingRedirects () [TestCase (true)] public void ReentrantPublicationPreservesWinner (bool returnWinner) { - var lookup = GetMethodLookup (); - var cache = new ConcurrentDictionary (); + using var cache = new JniPeerMembers.JniMethodInfoCache (); var outer = CreateRedirect (); var inner = CreateRedirect (); try { - var method = lookup (cache, "currentTimeMillis.()J", (member, state) => { - var winner = lookup (cache, member, (key, argument) => inner, state); + var method = cache.GetOrAdd ("currentTimeMillis.()J", (member, state) => { + var winner = cache.GetOrAdd (member, (key, argument) => inner, state); return returnWinner ? winner : outer; }, 0); @@ -130,34 +124,22 @@ public void ReentrantPublicationPreservesWinner (bool returnWinner) [Test] public void PublicationFailureDisposesCandidate () { - var lookup = GetMethodLookup (); var comparer = new PublicationFailureComparer (); - var cache = new ConcurrentDictionary (comparer); + using var cache = new JniPeerMembers.JniMethodInfoCache (comparer); var candidate = CreateRedirect (); try { var error = Assert.Throws (() => - lookup (cache, "currentTimeMillis.()J", (member, state) => { + cache.GetOrAdd ("currentTimeMillis.()J", (member, state) => { comparer.Fail = true; return candidate; }, 0)); Assert.AreEqual ("Publication failed.", error.Message); - Assert.IsTrue (cache.IsEmpty); Assert.IsFalse (candidate.StaticRedirect.PeerReference.IsValid); } finally { candidate.StaticRedirect.Dispose (); } } - delegate JniMethodInfo MethodLookup (ConcurrentDictionary cache, string member, Func factory, int argument); - - [UnconditionalSuppressMessage ("AOT", "IL3050", Justification = "The private cache helper is invoked with a test-only factory in tests excluded from Native AOT.")] - static MethodLookup GetMethodLookup () - { - var method = typeof (JniPeerMembers).GetMethod ("GetOrAddMethodInfo", BindingFlags.NonPublic | BindingFlags.Static) - ?? throw new MissingMethodException (nameof (JniPeerMembers), "GetOrAddMethodInfo"); - return (MethodLookup) method.MakeGenericMethod (typeof (int)).CreateDelegate (typeof (MethodLookup)); - } - static JniMethodInfo CreateRedirect () { var type = new JniType ("java/lang/System"); From 8d5d846b0d562c6cc233e7a1a3b6283b1d8e28d2 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:20:04 +0200 Subject: [PATCH 20/37] [Java.Interop] Configure method cache at call sites Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/JniPeerMembers.JniInstanceMethods.cs | 2 +- .../Java.Interop/JniPeerMembers.JniMethodInfoCache.cs | 8 ++++---- .../Java.Interop/JniPeerMembers.JniStaticMethods.cs | 2 +- .../Java.Interop/JniRedirectCacheOwnershipTests.cs | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs index 56a92209422..d4477b35e43 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs @@ -42,7 +42,7 @@ internal JniType JniPeerType { JniMethodInfoCache? instanceMethods; ConcurrentDictionary? subclassConstructors; - JniMethodInfoCache InstanceMethods => JniMethodInfoCache.GetOrCreate (ref instanceMethods); + JniMethodInfoCache InstanceMethods => JniMethodInfoCache.GetOrCreate (ref instanceMethods, 1, 3); ConcurrentDictionary SubclassConstructors => GetOrCreate (ref subclassConstructors, 1); internal void Dispose () diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs index 6d15a40b138..5204016ec86 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs @@ -13,9 +13,9 @@ internal sealed class JniMethodInfoCache : IDisposable { readonly ConcurrentDictionary methods; - public JniMethodInfoCache () + public JniMethodInfoCache (int concurrencyLevel, int capacity) { - methods = new ConcurrentDictionary (1, 3); + methods = new ConcurrentDictionary (concurrencyLevel, capacity); } internal JniMethodInfoCache (IEqualityComparer comparer) @@ -23,13 +23,13 @@ internal JniMethodInfoCache (IEqualityComparer comparer) methods = new ConcurrentDictionary (comparer); } - internal static JniMethodInfoCache GetOrCreate (ref JniMethodInfoCache? cache) + internal static JniMethodInfoCache GetOrCreate (ref JniMethodInfoCache? cache, int concurrencyLevel, int capacity) { var value = Volatile.Read (ref cache); if (value != null) return value; - var candidate = new JniMethodInfoCache (); + var candidate = new JniMethodInfoCache (concurrencyLevel, capacity); return Interlocked.CompareExchange (ref cache, candidate, null) ?? candidate; } diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs index 7799f688100..044d6f567d9 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs @@ -17,7 +17,7 @@ internal JniStaticMethods (JniPeerMembers members) JniMethodInfoCache? staticMethods; - JniMethodInfoCache StaticMethods => JniMethodInfoCache.GetOrCreate (ref staticMethods); + JniMethodInfoCache StaticMethods => JniMethodInfoCache.GetOrCreate (ref staticMethods, 1, 3); internal void Dispose () { diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs index 90b7285fd08..d83a5f5ca46 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs @@ -72,7 +72,7 @@ public void DisposingOrdinaryMethodCacheDoesNotDisposePeerType (bool isStatic) [Test] public void ConcurrentPublicationDisposesOnlyLosingRedirects () { - using var cache = new JniPeerMembers.JniMethodInfoCache (); + using var cache = new JniPeerMembers.JniMethodInfoCache (1, 3); var candidates = new JniMethodInfo [2]; var results = new JniMethodInfo [candidates.Length]; using var ready = new Barrier (candidates.Length); @@ -103,7 +103,7 @@ public void ConcurrentPublicationDisposesOnlyLosingRedirects () [TestCase (true)] public void ReentrantPublicationPreservesWinner (bool returnWinner) { - using var cache = new JniPeerMembers.JniMethodInfoCache (); + using var cache = new JniPeerMembers.JniMethodInfoCache (1, 3); var outer = CreateRedirect (); var inner = CreateRedirect (); try { From 0fad3351982ee91c99003d7ad051239e3dcc1790 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:20:39 +0200 Subject: [PATCH 21/37] [Java.Interop] Size custom method caches explicitly Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/JniPeerMembers.JniMethodInfoCache.cs | 4 ++-- .../Java.Interop/JniRedirectCacheOwnershipTests.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs index 5204016ec86..a9f0d041ec4 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs @@ -18,9 +18,9 @@ public JniMethodInfoCache (int concurrencyLevel, int capacity) methods = new ConcurrentDictionary (concurrencyLevel, capacity); } - internal JniMethodInfoCache (IEqualityComparer comparer) + internal JniMethodInfoCache (int concurrencyLevel, int capacity, IEqualityComparer comparer) { - methods = new ConcurrentDictionary (comparer); + methods = new ConcurrentDictionary (concurrencyLevel, capacity, comparer); } internal static JniMethodInfoCache GetOrCreate (ref JniMethodInfoCache? cache, int concurrencyLevel, int capacity) diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs index d83a5f5ca46..54b75f79055 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs @@ -125,7 +125,7 @@ public void ReentrantPublicationPreservesWinner (bool returnWinner) public void PublicationFailureDisposesCandidate () { var comparer = new PublicationFailureComparer (); - using var cache = new JniPeerMembers.JniMethodInfoCache (comparer); + using var cache = new JniPeerMembers.JniMethodInfoCache (1, 3, comparer); var candidate = CreateRedirect (); try { var error = Assert.Throws (() => From 6c492204f6694580ac56fcf3e94e413d86194a27 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:21:47 +0200 Subject: [PATCH 22/37] [Java.Interop] Dispose unused cache candidates Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/JniPeerMembers.JniMethodInfoCache.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs index a9f0d041ec4..85740f86212 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs @@ -30,7 +30,12 @@ internal static JniMethodInfoCache GetOrCreate (ref JniMethodInfoCache? cache, i return value; var candidate = new JniMethodInfoCache (concurrencyLevel, capacity); - return Interlocked.CompareExchange (ref cache, candidate, null) ?? candidate; + var existing = Interlocked.CompareExchange (ref cache, candidate, null); + if (existing == null) + return candidate; + + candidate.Dispose (); + return existing; } internal static void Dispose (ref JniMethodInfoCache? cache) From f2e6a46df43fb683aa5649251a68e56d08b180eb Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:24:52 +0200 Subject: [PATCH 23/37] [Java.Interop] Use Lock for native registrations Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs index f40ff78f053..6fa6e5ce0fb 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs @@ -155,7 +155,7 @@ public bool IsInstanceOfType (JniObjectReference value) // Retains delegates from every batch JNI may have partially registered. JniNativeMethodRegistration[]? methods; - object? nativeRegistrationLock; + Lock? nativeRegistrationLock; /// /// Once JNI registration is attempted, the runtime retains this type and its delegates From caf9a29c9a54db55c7d9b0352250717c482a7cf9 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:33:32 +0200 Subject: [PATCH 24/37] [Java.Interop] Reject repeated native registration Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/Java.Interop/Java.Interop/JniType.cs | 15 +++-- .../ManagedPeerRegistrationTests.cs | 56 +++---------------- 2 files changed, 15 insertions(+), 56 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs index 6fa6e5ce0fb..b98fdbf1982 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs @@ -5,7 +5,6 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Collections.Generic; -using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -153,7 +152,7 @@ public bool IsInstanceOfType (JniObjectReference value) return JniEnvironment.Types.IsInstanceOf (value, PeerReference); } - // Retains delegates from every batch JNI may have partially registered. + // Retains delegates from the batch JNI may have partially registered. JniNativeMethodRegistration[]? methods; Lock? nativeRegistrationLock; @@ -176,13 +175,13 @@ internal void RetainNativeMethodRegistrations (JniNativeMethodRegistration[] reg { lock (LazyInitializer.EnsureInitialized (ref nativeRegistrationLock)) { // RegisterNatives stores unmanaged function pointers without retaining the - // managed delegates behind them. Root every attempted batch because JNI can - // publish part of a failing batch, and earlier batches remain callable after - // later registrations. Runtime tracking also keeps this JniType alive until - // disposal, when its native methods are unregistered. - var retained = methods == null ? registrations : methods.Concat (registrations).ToArray (); + // managed delegates behind them. JNI can publish part of a failing batch, so + // the first attempt owns this JniType until disposal and cannot be retried. + if (methods != null) + throw new InvalidOperationException ("Native method registration has already been attempted for this JniType."); + RegisterWithRuntime (); - methods = retained; + methods = registrations; } } diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs index b5bbf565467..53b9aee288e 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs @@ -5,8 +5,6 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using System.Threading; -using System.Threading.Tasks; using Java.Interop; @@ -66,57 +64,19 @@ public void RegistrationAttempt_RetainsOwnerAndDelegate (bool fail) [TestCase (false)] [TestCase (true)] - public void RepeatedRegistration_RetainsPreviousAndAttemptedDelegates (bool fail) + public void RepeatedRegistration_Throws (bool firstAttemptFails) { using var owner = new JniType (JniTypeName); - var targets = RegisterRepeatedly (owner, fail); - Collect (); - Assert.IsTrue (targets.Previous.IsAlive, "The previous registration must remain rooted."); - Assert.IsTrue (targets.Attempted.IsAlive, "The partial registration must remain rooted."); - Assert.AreEqual (42, Call (owner, "existing")); - Assert.AreEqual (42, Call (owner, "value")); - } - - [MethodImpl (MethodImplOptions.NoInlining)] - static (WeakReference Previous, WeakReference Attempted) RegisterRepeatedly (JniType owner, bool fail) - { - var previous = new NativeTarget (); - var attempted = new NativeTarget (); - owner.RegisterNativeMethods (new JniNativeMethodRegistration ("existing", "()I", new GetValue (previous.Value))); - if (fail) { + var target = new NativeTarget (); + if (firstAttemptFails) { using var error = Assert.Throws (() => owner.RegisterNativeMethods ( - new JniNativeMethodRegistration ("value", "()I", new GetValue (attempted.Value)), - new JniNativeMethodRegistration ("missing", "()I", new GetValue (attempted.Value)))); + new JniNativeMethodRegistration ("value", "()I", new GetValue (target.Value)), + new JniNativeMethodRegistration ("missing", "()I", new GetValue (target.Value)))); } else { - owner.RegisterNativeMethods (new JniNativeMethodRegistration ("value", "()I", new GetValue (attempted.Value))); + owner.RegisterNativeMethods (new JniNativeMethodRegistration ("existing", "()I", new GetValue (target.Value))); } - return (new WeakReference (previous), new WeakReference (attempted)); - } - - [Test] - public void ConcurrentRegistration_RetainsDelegates () - { - using var owner = new JniType (JniTypeName); - using var barrier = new Barrier (2); - var first = Task.Factory.StartNew (() => RegisterConcurrently (owner, "existing", barrier), - CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default); - var second = Task.Factory.StartNew (() => RegisterConcurrently (owner, "value", barrier), - CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default); - var targets = Task.WhenAll (first, second).GetAwaiter ().GetResult (); - Collect (); - Assert.IsTrue (targets [0].IsAlive); - Assert.IsTrue (targets [1].IsAlive); - Assert.AreEqual (42, Call (owner, "existing")); - Assert.AreEqual (42, Call (owner, "value")); - } - - [MethodImpl (MethodImplOptions.NoInlining)] - static WeakReference RegisterConcurrently (JniType owner, string name, Barrier barrier) - { - var target = new NativeTarget (); - Assert.IsTrue (barrier.SignalAndWait (TimeSpan.FromSeconds (30)), "Concurrent registrars did not reach the barrier."); - owner.RegisterNativeMethods (new JniNativeMethodRegistration (name, "()I", new GetValue (target.Value))); - return new WeakReference (target); + Assert.Throws (() => + owner.RegisterNativeMethods (new JniNativeMethodRegistration ("value", "()I", new GetValue (target.Value)))); } static int Call (JniType owner, string name) From 405181100ba7beee52a1416c85399a1ed4e1a6ef Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:38:11 +0200 Subject: [PATCH 25/37] [Java.Interop] Test redirect cleanup through public API Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../JniPeerMembers.JniMethodInfoCache.cs | 8 +- .../JniRedirectCacheOwnershipTests.cs | 161 ++---------------- 2 files changed, 11 insertions(+), 158 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs index 85740f86212..93fd6bcb23f 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs @@ -2,14 +2,13 @@ using System; using System.Collections.Concurrent; -using System.Collections.Generic; using System.Threading; namespace Java.Interop { partial class JniPeerMembers { - internal sealed class JniMethodInfoCache : IDisposable { + private sealed class JniMethodInfoCache : IDisposable { readonly ConcurrentDictionary methods; @@ -18,11 +17,6 @@ public JniMethodInfoCache (int concurrencyLevel, int capacity) methods = new ConcurrentDictionary (concurrencyLevel, capacity); } - internal JniMethodInfoCache (int concurrencyLevel, int capacity, IEqualityComparer comparer) - { - methods = new ConcurrentDictionary (concurrencyLevel, capacity, comparer); - } - internal static JniMethodInfoCache GetOrCreate (ref JniMethodInfoCache? cache, int concurrencyLevel, int capacity) { var value = Volatile.Read (ref cache); diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs index 54b75f79055..a167bb9c2a9 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs @@ -1,8 +1,3 @@ -using System; -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; - using Java.Interop; using NUnit.Framework; @@ -14,151 +9,28 @@ public class JniRedirectCacheOwnershipTests : JavaVMFixture { [TestCase (false)] [TestCase (true)] - public void DisposingMethodCacheReleasesRedirect (bool isStatic) + public void DisposingPeerMembersReleasesRedirect (bool isStatic) { var members = isStatic ? new JniPeerMembers (IAndroidInterface.JniTypeName, typeof (IAndroidInterface)) : new JniPeerMembers (JavaLangRemappingTestObject.JniTypeName, typeof (JavaLangRemappingTestObject)); + JniType redirect = null; try { - for (int i = 0; i < 3; ++i) { - var method = GetRedirectedMethod (members, isStatic); - var redirect = method.StaticRedirect; - Assert.IsNotNull (redirect); - Assert.IsTrue (redirect.PeerReference.IsValid); - Assert.AreSame (method, GetRedirectedMethod (members, isStatic)); + var method = GetRedirectedMethod (members, isStatic); + redirect = method.StaticRedirect; + Assert.IsNotNull (redirect); + Assert.IsTrue (redirect.PeerReference.IsValid); + Assert.AreSame (method, GetRedirectedMethod (members, isStatic)); + AssertRedirectIsCallable (members, isStatic); - try { - AssertRedirectIsCallable (members, isStatic); - if (isStatic) - members.StaticMethods.Dispose (); - else - members.InstanceMethods.Dispose (); - Assert.IsFalse (redirect.PeerReference.IsValid, "The method cache owns the redirect's global reference."); - if (isStatic) - members.StaticMethods.Dispose (); - else - members.InstanceMethods.Dispose (); - } finally { - redirect.Dispose (); - } - } - } finally { JniPeerMembers.Dispose (members); - } - } - - [TestCase (false)] - [TestCase (true)] - public void DisposingOrdinaryMethodCacheDoesNotDisposePeerType (bool isStatic) - { - var members = new JniPeerMembers (JavaLangRemappingTestRuntime.JniTypeName, typeof (JavaLangRemappingTestRuntime)); - try { - var peerType = members.JniPeerType; - var method = isStatic - ? members.StaticMethods.GetMethodInfo ("getRuntime.()Ljava/lang/Runtime;") - : members.InstanceMethods.GetMethodInfo ("hashCode.()I"); - Assert.IsNull (method.StaticRedirect); - - if (isStatic) - members.StaticMethods.Dispose (); - else - members.InstanceMethods.Dispose (); - Assert.IsTrue (peerType.PeerReference.IsValid); + Assert.IsFalse (redirect.PeerReference.IsValid, "Disposing the peer members must release the redirect's global reference."); } finally { JniPeerMembers.Dispose (members); + redirect?.Dispose (); } } - [Test] - public void ConcurrentPublicationDisposesOnlyLosingRedirects () - { - using var cache = new JniPeerMembers.JniMethodInfoCache (1, 3); - var candidates = new JniMethodInfo [2]; - var results = new JniMethodInfo [candidates.Length]; - using var ready = new Barrier (candidates.Length); - try { - Parallel.For (0, candidates.Length, i => { - results [i] = cache.GetOrAdd ("currentTimeMillis.()J", (member, index) => { - candidates [index] = CreateRedirect (); - if (!ready.SignalAndWait (TimeSpan.FromSeconds (30))) - throw new TimeoutException ("Both candidates must be created before publication."); - return candidates [index]; - }, i); - }); - var winner = results [0]; - Assert.AreSame (winner, results [1]); - Assert.AreSame (winner, results [1]); - foreach (var candidate in candidates) - Assert.AreEqual (ReferenceEquals (candidate, winner), candidate.StaticRedirect.PeerReference.IsValid); - AssertSystemRedirectIsCallable (winner); - Assert.AreSame (winner, cache.GetOrAdd ("currentTimeMillis.()J", - (member, state) => throw new InvalidOperationException ("A cache hit must not construct a candidate."), 0)); - } finally { - foreach (var candidate in candidates) - candidate?.StaticRedirect?.Dispose (); - } - } - - [TestCase (false)] - [TestCase (true)] - public void ReentrantPublicationPreservesWinner (bool returnWinner) - { - using var cache = new JniPeerMembers.JniMethodInfoCache (1, 3); - var outer = CreateRedirect (); - var inner = CreateRedirect (); - try { - var method = cache.GetOrAdd ("currentTimeMillis.()J", (member, state) => { - var winner = cache.GetOrAdd (member, (key, argument) => inner, state); - return returnWinner ? winner : outer; - }, 0); - - Assert.AreSame (inner, method); - Assert.AreEqual (returnWinner, outer.StaticRedirect.PeerReference.IsValid); - AssertSystemRedirectIsCallable (inner); - } finally { - outer.StaticRedirect.Dispose (); - inner.StaticRedirect.Dispose (); - } - } - - [Test] - public void PublicationFailureDisposesCandidate () - { - var comparer = new PublicationFailureComparer (); - using var cache = new JniPeerMembers.JniMethodInfoCache (1, 3, comparer); - var candidate = CreateRedirect (); - try { - var error = Assert.Throws (() => - cache.GetOrAdd ("currentTimeMillis.()J", (member, state) => { - comparer.Fail = true; - return candidate; - }, 0)); - Assert.AreEqual ("Publication failed.", error.Message); - Assert.IsFalse (candidate.StaticRedirect.PeerReference.IsValid); - } finally { - candidate.StaticRedirect.Dispose (); - } - } - - static JniMethodInfo CreateRedirect () - { - var type = new JniType ("java/lang/System"); - try { - var method = type.GetStaticMethod ("currentTimeMillis", "()J"); - method.StaticRedirect = type; - type = null; - return method; - } finally { - type?.Dispose (); - } - } - - static unsafe void AssertSystemRedirectIsCallable (JniMethodInfo method) - { - Assert.IsTrue (method.StaticRedirect.PeerReference.IsValid); - Assert.Greater (JniEnvironment.StaticMethods.CallStaticLongMethod (method.StaticRedirect.PeerReference, method, null), 0); - } - static unsafe void AssertRedirectIsCallable (JniPeerMembers members, bool isStatic) { if (isStatic) { @@ -177,18 +49,5 @@ static JniMethodInfo GetRedirectedMethod (JniPeerMembers members, bool isStatic) : members.InstanceMethods.GetMethodInfo ("remappedToStaticHashCode.()I"); } - sealed class PublicationFailureComparer : IEqualityComparer - { - public bool Fail; - - public bool Equals (string x, string y) => StringComparer.Ordinal.Equals (x, y); - - public int GetHashCode (string value) - { - if (Fail) - throw new InvalidOperationException ("Publication failed."); - return StringComparer.Ordinal.GetHashCode (value); - } - } } } From a5c8e7a5030355f850983e1f0ea22cd3d0b0d8bc Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:41:30 +0200 Subject: [PATCH 26/37] [Java.Interop] Generalize resource-owning cache Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../JniPeerMembers.JniInstanceMethods.cs | 27 ++----- .../JniPeerMembers.JniMethodInfoCache.cs | 67 ---------------- .../JniPeerMembers.JniStaticMethods.cs | 7 +- .../JniPeerMembers.JniValueCache.cs | 78 +++++++++++++++++++ .../JniSubclassConstructorCacheTests.cs | 38 --------- 5 files changed, 88 insertions(+), 129 deletions(-) delete mode 100644 external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs create mode 100644 external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniValueCache.cs diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs index d4477b35e43..af0215acb07 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs @@ -1,7 +1,6 @@ #nullable enable using System; -using System.Collections.Concurrent; namespace Java.Interop { @@ -39,16 +38,16 @@ internal JniType JniPeerType { readonly Type DeclaringType; - JniMethodInfoCache? instanceMethods; - ConcurrentDictionary? subclassConstructors; + JniValueCache? instanceMethods; + JniValueCache? subclassConstructors; - JniMethodInfoCache InstanceMethods => JniMethodInfoCache.GetOrCreate (ref instanceMethods, 1, 3); - ConcurrentDictionary SubclassConstructors => GetOrCreate (ref subclassConstructors, 1); + JniValueCache InstanceMethods => JniValueCache.GetOrCreate (ref instanceMethods, 1, 3, static value => value.StaticRedirect?.Dispose ()); + JniValueCache SubclassConstructors => JniValueCache.GetOrCreate (ref subclassConstructors, 1, 1, static value => value.Dispose ()); internal void Dispose () { - JniMethodInfoCache.Dispose (ref instanceMethods); - Clear (ref subclassConstructors, static value => value.Dispose ()); + JniValueCache.Dispose (ref instanceMethods); + JniValueCache.Dispose (ref subclassConstructors); if (jniPeerType != null) jniPeerType.Dispose (); @@ -68,10 +67,6 @@ internal JniInstanceMethods GetConstructorsForType (Type declaringType) if (declaringType == DeclaringType) return this; - var cache = SubclassConstructors; - if (cache.TryGetValue (declaringType, out var constructors)) - return constructors; - // Initialize before publication in case construction recursively accesses this cache: // System.ArgumentException: An item with the same key has already been added. Key: Java.Interop.JavaProxyThrowable // at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior) @@ -94,15 +89,7 @@ internal JniInstanceMethods GetConstructorsForType (Type declaringType) // at Java.Interop.JniPeerMembers.JniInstanceMethods..ctor(Type declaringType) in /Users/jon/Developer/src/xamarin/java.interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 27 // at Java.Interop.JniPeerMembers.JniInstanceMethods.GetConstructorsForType(Type declaringType) in /Users/jon/Developer/src/xamarin/java.interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 77 // at Java.Interop.JniPeerMembers.JniInstanceMethods.StartCreateInstance(String constructorSignature, Type declaringType, JniArgumentValue* parameters) in /Users/jon/Developer/src/xamarin/java.interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 146 - var candidate = new JniInstanceMethods (declaringType); - try { - constructors = cache.GetOrAdd (declaringType, candidate); - return constructors; - } finally { - // Only the published candidate transfers ownership to the cache. - if (!ReferenceEquals (constructors, candidate)) - candidate.Dispose (); - } + return SubclassConstructors.GetOrAdd (declaringType, static type => new JniInstanceMethods (type)); } public JniMethodInfo GetMethodInfo (string encodedMember) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs deleted file mode 100644 index 93fd6bcb23f..00000000000 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniMethodInfoCache.cs +++ /dev/null @@ -1,67 +0,0 @@ -#nullable enable - -using System; -using System.Collections.Concurrent; -using System.Threading; - -namespace Java.Interop { - - partial class JniPeerMembers { - - private sealed class JniMethodInfoCache : IDisposable { - - readonly ConcurrentDictionary methods; - - public JniMethodInfoCache (int concurrencyLevel, int capacity) - { - methods = new ConcurrentDictionary (concurrencyLevel, capacity); - } - - internal static JniMethodInfoCache GetOrCreate (ref JniMethodInfoCache? cache, int concurrencyLevel, int capacity) - { - var value = Volatile.Read (ref cache); - if (value != null) - return value; - - var candidate = new JniMethodInfoCache (concurrencyLevel, capacity); - var existing = Interlocked.CompareExchange (ref cache, candidate, null); - if (existing == null) - return candidate; - - candidate.Dispose (); - return existing; - } - - internal static void Dispose (ref JniMethodInfoCache? cache) - { - Interlocked.Exchange (ref cache, null)?.Dispose (); - } - - public JniMethodInfo GetOrAdd (string member, Func factory, TArg argument) - { - if (methods.TryGetValue (member, out var method)) - return method; - - // ConcurrentDictionary may invoke a GetOrAdd factory multiple times and discard - // losing values. Construct explicitly so an unpublished StaticRedirect owner can - // be disposed. JNI lookup can also reenter this cache, so do not lock construction. - var candidate = factory (member, argument); - try { - method = methods.GetOrAdd (member, candidate); - if (ReferenceEquals (method, candidate)) - candidate = null; - return method; - } finally { - candidate?.StaticRedirect?.Dispose (); - } - } - - public void Dispose () - { - foreach (var method in methods.Values) - method.StaticRedirect?.Dispose (); - methods.Clear (); - } - } - } -} diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs index 044d6f567d9..8b1945c3fee 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs @@ -1,7 +1,6 @@ #nullable enable using System; -using System.Collections.Concurrent; namespace Java.Interop { @@ -15,13 +14,13 @@ internal JniStaticMethods (JniPeerMembers members) internal readonly JniPeerMembers Members; - JniMethodInfoCache? staticMethods; + JniValueCache? staticMethods; - JniMethodInfoCache StaticMethods => JniMethodInfoCache.GetOrCreate (ref staticMethods, 1, 3); + JniValueCache StaticMethods => JniValueCache.GetOrCreate (ref staticMethods, 1, 3, static value => value.StaticRedirect?.Dispose ()); internal void Dispose () { - JniMethodInfoCache.Dispose (ref staticMethods); + JniValueCache.Dispose (ref staticMethods); } public JniMethodInfo GetMethodInfo (string encodedMember) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniValueCache.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniValueCache.cs new file mode 100644 index 00000000000..af0f6488cf1 --- /dev/null +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniValueCache.cs @@ -0,0 +1,78 @@ +#nullable enable + +using System; +using System.Collections.Concurrent; +using System.Threading; + +namespace Java.Interop { + + partial class JniPeerMembers { + + private sealed class JniValueCache : IDisposable + where TKey : notnull + where TValue : class + { + + readonly ConcurrentDictionary values; + readonly Action dispose; + + public JniValueCache (int concurrencyLevel, int capacity, Action dispose) + { + values = new ConcurrentDictionary (concurrencyLevel, capacity); + this.dispose = dispose; + } + + internal static JniValueCache GetOrCreate (ref JniValueCache? cache, int concurrencyLevel, int capacity, Action dispose) + { + var value = Volatile.Read (ref cache); + if (value != null) + return value; + + var candidate = new JniValueCache (concurrencyLevel, capacity, dispose); + var existing = Interlocked.CompareExchange (ref cache, candidate, null); + if (existing == null) + return candidate; + + candidate.Dispose (); + return existing; + } + + internal static void Dispose (ref JniValueCache? cache) + { + Interlocked.Exchange (ref cache, null)?.Dispose (); + } + + public TValue GetOrAdd (TKey key, Func factory) + { + return GetOrAdd (key, static (key, factory) => factory (key), factory); + } + + public TValue GetOrAdd (TKey key, Func factory, TArg argument) + { + if (values.TryGetValue (key, out var value)) + return value; + + // ConcurrentDictionary may invoke a GetOrAdd factory multiple times and discard + // losing values. Construct explicitly so an unpublished owner can be disposed. + // JNI lookup can also reenter this cache, so do not lock construction. + TValue? candidate = factory (key, argument); + try { + value = values.GetOrAdd (key, candidate); + if (ReferenceEquals (value, candidate)) + candidate = null; + return value; + } finally { + if (candidate != null) + dispose (candidate); + } + } + + public void Dispose () + { + foreach (var value in values.Values) + dispose (value); + values.Clear (); + } + } + } +} diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniSubclassConstructorCacheTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniSubclassConstructorCacheTests.cs index 185b841c819..b9e3a55f1e8 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniSubclassConstructorCacheTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniSubclassConstructorCacheTests.cs @@ -59,31 +59,6 @@ public void RecursiveCreationDisposesOuterConstructor () }); } - [Test] - public void PublicationFailureDisposesConstructor () - { - RunWithReferenceTracking ((members, references, runtime) => { - var comparer = new ThrowingTypeComparer (); - var cache = new ConcurrentDictionary (comparer); - var field = typeof (JniPeerMembers.JniInstanceMethods).GetField ("subclassConstructors", BindingFlags.NonPublic | BindingFlags.Instance); - Assert.IsNotNull (field); - field.SetValue (members.InstanceMethods, cache); - references.OnCreate = () => comparer.ThrowOnHash = true; - - Assert.Throws (() => members.InstanceMethods.GetConstructorsForType (typeof (MyString))); - references.OnCreate = null; - comparer.ThrowOnHash = false; - - Assert.AreEqual (1, references.Created.Count); - Assert.IsEmpty (cache); - AssertReleased (references, runtime); - - // Failure must leave the cache usable for a subsequent lookup. - var constructor = members.InstanceMethods.GetConstructorsForType (typeof (MyString)); - AssertWinnerAndCleanup (members, references, runtime, constructor, 2); - }); - } - static unsafe void AssertWinnerAndCleanup (JniPeerMembers members, TrackingReferenceManager references, JniRuntime runtime, JniPeerMembers.JniInstanceMethods winner, int created) { var type = winner.JniPeerType; @@ -241,19 +216,6 @@ public override void WriteGlobalReferenceLine (string format, params object [] a inner.WriteGlobalReferenceLine (format, args); } - sealed class ThrowingTypeComparer : IEqualityComparer - { - public bool ThrowOnHash; - - public bool Equals (Type x, Type y) => x == y; - - public int GetHashCode (Type type) - { - if (ThrowOnHash) - throw new InvalidOperationException ("Constructor publication failed."); - return type.GetHashCode (); - } - } } } #endif From a90470a8ced842d8bb191583dc9b01a4553d9101 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:42:10 +0200 Subject: [PATCH 27/37] [Java.Interop] Atomically claim native registration Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/Java.Interop/Java.Interop/JniType.cs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs index b98fdbf1982..36e0adcaa90 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs @@ -154,7 +154,6 @@ public bool IsInstanceOfType (JniObjectReference value) // Retains delegates from the batch JNI may have partially registered. JniNativeMethodRegistration[]? methods; - Lock? nativeRegistrationLock; /// /// Once JNI registration is attempted, the runtime retains this type and its delegates @@ -173,16 +172,13 @@ public void RegisterNativeMethods (params JniNativeMethodRegistration[] methods) internal void RetainNativeMethodRegistrations (JniNativeMethodRegistration[] registrations) { - lock (LazyInitializer.EnsureInitialized (ref nativeRegistrationLock)) { - // RegisterNatives stores unmanaged function pointers without retaining the - // managed delegates behind them. JNI can publish part of a failing batch, so - // the first attempt owns this JniType until disposal and cannot be retried. - if (methods != null) - throw new InvalidOperationException ("Native method registration has already been attempted for this JniType."); + // RegisterNatives stores unmanaged function pointers without retaining the + // managed delegates behind them. JNI can publish part of a failing batch, so + // the first attempt owns this JniType until disposal and cannot be retried. + if (Interlocked.CompareExchange (ref methods, registrations, null) != null) + throw new InvalidOperationException ("Native method registration has already been attempted for this JniType."); - RegisterWithRuntime (); - methods = registrations; - } + RegisterWithRuntime (); } public void UnregisterNativeMethods () From 5fcfc4887ed6bd574afa889217448192e3307a73 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:44:47 +0200 Subject: [PATCH 28/37] [Java.Interop] Clarify repeated registration error Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs index 36e0adcaa90..2f7d17d2315 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs @@ -176,7 +176,7 @@ internal void RetainNativeMethodRegistrations (JniNativeMethodRegistration[] reg // managed delegates behind them. JNI can publish part of a failing batch, so // the first attempt owns this JniType until disposal and cannot be retried. if (Interlocked.CompareExchange (ref methods, registrations, null) != null) - throw new InvalidOperationException ("Native method registration has already been attempted for this JniType."); + throw new InvalidOperationException ("Native methods cannot be registered more than once."); RegisterWithRuntime (); } From 341b06b7eab41eee4bd3dceba257132ff6daf8ac Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:51:47 +0200 Subject: [PATCH 29/37] [Java.Interop] Keep registration ownership in JniType Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/JniEnvironment.Types.cs | 7 ------- .../src/Java.Interop/Java.Interop/JniType.cs | 15 +++++++-------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs index a938b841306..2a543700398 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs @@ -224,12 +224,6 @@ public static void RegisterNatives (JniObjectReference type, JniNativeMethodRegi [RequiresDynamicCode ("Native method registration via JniNativeMethodRegistration[] requires dynamic code generation. Use the blittable RegisterNatives(JniObjectReference, ReadOnlySpan) overload with statically-compiled function pointers for Native AOT compatibility.")] public static unsafe void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration [] methods, int numMethods) - { - RegisterNatives (type, methods, numMethods, null); - } - - [RequiresDynamicCode ("Native method registration via JniNativeMethodRegistration[] requires dynamic code generation. Use the blittable RegisterNatives(JniObjectReference, ReadOnlySpan) overload with statically-compiled function pointers for Native AOT compatibility.")] - internal static unsafe void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration [] methods, int numMethods, JniType? owner) { if ((numMethods < 0) || (numMethods > (methods?.Length ?? 0))) { @@ -279,7 +273,6 @@ internal static unsafe void RegisterNatives (JniObjectReference type, JniNativeM unmanagedStrings [i * 2 + 1] = sig; natives [i] = new JniNativeMethod ((byte*) name, (byte*) sig, Marshal.GetFunctionPointerForDelegate (m.Marshaler)); } - owner?.RetainNativeMethodRegistrations (methods); RegisterNatives (type, natives); // Keep the Marshaler delegates alive at least until JNI has consumed the function pointers. GC.KeepAlive (methods); diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs index 2f7d17d2315..fe4e4c74ced 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs @@ -156,8 +156,9 @@ public bool IsInstanceOfType (JniObjectReference value) JniNativeMethodRegistration[]? methods; /// - /// Once JNI registration is attempted, the runtime retains this type and its delegates - /// until disposal, even if registration throws: JNI may have registered part of the batch. + /// Once a non-empty registration is requested, the runtime retains this type and its + /// delegates until disposal, even if registration throws: JNI may have registered part + /// of the batch. /// [RequiresDynamicCode ("Native method registration via JniNativeMethodRegistration[] requires dynamic code generation. Use the blittable RegisterNatives(JniObjectReference, ReadOnlySpan) overload with statically-compiled function pointers for Native AOT compatibility.")] public void RegisterNativeMethods (params JniNativeMethodRegistration[] methods) @@ -166,19 +167,17 @@ public void RegisterNativeMethods (params JniNativeMethodRegistration[] methods) if (methods == null) throw new ArgumentNullException (nameof (methods)); + if (methods.Length == 0) + return; - JniEnvironment.Types.RegisterNatives (PeerReference, methods, methods.Length, this); - } - - internal void RetainNativeMethodRegistrations (JniNativeMethodRegistration[] registrations) - { // RegisterNatives stores unmanaged function pointers without retaining the // managed delegates behind them. JNI can publish part of a failing batch, so // the first attempt owns this JniType until disposal and cannot be retried. - if (Interlocked.CompareExchange (ref methods, registrations, null) != null) + if (Interlocked.CompareExchange (ref this.methods, methods, null) != null) throw new InvalidOperationException ("Native methods cannot be registered more than once."); RegisterWithRuntime (); + JniEnvironment.Types.RegisterNatives (PeerReference, methods, methods.Length); } public void UnregisterNativeMethods () From 0ace082746bc6d276831613dc948ae9b80ee9087 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 13:59:01 +0200 Subject: [PATCH 30/37] [Java.Interop] Explain registration retention order Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/src/Java.Interop/Java.Interop/JniType.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs index fe4e4c74ced..908101596e5 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs @@ -170,9 +170,10 @@ public void RegisterNativeMethods (params JniNativeMethodRegistration[] methods) if (methods.Length == 0) return; - // RegisterNatives stores unmanaged function pointers without retaining the - // managed delegates behind them. JNI can publish part of a failing batch, so - // the first attempt owns this JniType until disposal and cannot be retried. + // Retain the delegates before calling RegisterNatives: JNI stores only their + // unmanaged function pointers and may publish part of the batch before throwing. + // Storing them afterward could therefore leave callable pointers to collected + // delegates. The first attempt owns this JniType until disposal and cannot be retried. if (Interlocked.CompareExchange (ref this.methods, methods, null) != null) throw new InvalidOperationException ("Native methods cannot be registered more than once."); From 2bc7c2adda3799826b2d301727de505b94491f16 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 14:27:26 +0200 Subject: [PATCH 31/37] [Java.Interop] Encapsulate runtime ownership cleanup Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/src/Java.Interop/Java.Interop/JniType.cs | 8 ++++++-- .../src/Java.Interop/Java.Interop/ManagedPeer.cs | 3 +-- .../Java.Interop/ManagedPeerRegistrationTests.cs | 5 +++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs index 908101596e5..38296c1f346 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs @@ -42,8 +42,6 @@ public static bool TryParse (string name, [NotNullWhen (true)] out JniType? type bool registered; JniObjectReference peerReference; - internal bool IsRegisteredWithRuntime => registered; - public JniObjectReference PeerReference { get {return peerReference;} } @@ -123,6 +121,12 @@ public void Dispose () JniObjectReference.Dispose (ref peerReference); } + internal void DisposeUnlessRegisteredWithRuntime () + { + if (!registered) + Dispose (); + } + public JniType? GetSuperclass () { AssertValid (); diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/ManagedPeer.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/ManagedPeer.cs index 5d939ebbd71..4ae7a346de0 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/ManagedPeer.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/ManagedPeer.cs @@ -306,8 +306,7 @@ static unsafe void RegisterNativeMembers ( JniEnvironment.Strings.ReleaseStringChars (methodsRef, methodsChars); } } finally { - if (!nativeClass.IsRegisteredWithRuntime) - nativeClass.Dispose (); + nativeClass.DisposeUnlessRegisteredWithRuntime (); } } catch (Exception e) { diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs index 53b9aee288e..ff19d0c9854 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs @@ -27,11 +27,12 @@ sealed class NativeTarget { } [Test] - public void EmptyRegistration_DoesNotAdoptOwner () + public void EmptyRegistration_AllowsNonEmptyRegistration () { using var owner = new JniType (JniTypeName); owner.RegisterNativeMethods (); - Assert.IsFalse (owner.IsRegisteredWithRuntime); + owner.RegisterNativeMethods (new JniNativeMethodRegistration ("value", "()I", new GetValue (static (env, klass) => 42))); + Assert.AreEqual (42, Call (owner, "value")); } [TestCase (false)] From 13088c8932511fd5386a142386ef475d02541481 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 14:29:48 +0200 Subject: [PATCH 32/37] [Mono.Android] Avoid callback removal closures Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Android.Graphics.Drawables/ClipDrawable.cs | 2 +- .../Android.Graphics.Drawables/Drawable.cs | 2 +- .../DrawableContainer.cs | 2 +- .../Android.Graphics.Drawables/InsetDrawable.cs | 2 +- .../Android.Graphics.Drawables/LayerDrawable.cs | 2 +- .../Android.Graphics.Drawables/RotateDrawable.cs | 2 +- .../Android.Graphics.Drawables/ScaleDrawable.cs | 2 +- src/Mono.Android/Android.OS/Handler.cs | 4 ++-- src/Mono.Android/Android.Views/View.cs | 2 +- src/Mono.Android/Java.Lang/Thread.cs | 16 ++++++++++++++++ 10 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/Mono.Android/Android.Graphics.Drawables/ClipDrawable.cs b/src/Mono.Android/Android.Graphics.Drawables/ClipDrawable.cs index dd92d372c59..2b5d26fbcf5 100644 --- a/src/Mono.Android/Android.Graphics.Drawables/ClipDrawable.cs +++ b/src/Mono.Android/Android.Graphics.Drawables/ClipDrawable.cs @@ -12,7 +12,7 @@ public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action wh public void UnscheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what) { - Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleDrawable (who, runnable)); + Java.Lang.Thread.RunnableImplementor.Remove (what, this, who, static (runnable, drawable, who) => drawable.UnscheduleDrawable (who, runnable)); } } } diff --git a/src/Mono.Android/Android.Graphics.Drawables/Drawable.cs b/src/Mono.Android/Android.Graphics.Drawables/Drawable.cs index f4e83ab9a56..2d359d46967 100644 --- a/src/Mono.Android/Android.Graphics.Drawables/Drawable.cs +++ b/src/Mono.Android/Android.Graphics.Drawables/Drawable.cs @@ -12,7 +12,7 @@ public void ScheduleSelf (Action what, long when) public void UnscheduleSelf (Action what) { - Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleSelf (runnable)); + Java.Lang.Thread.RunnableImplementor.Remove (what, this, static (runnable, drawable) => drawable.UnscheduleSelf (runnable)); } } } diff --git a/src/Mono.Android/Android.Graphics.Drawables/DrawableContainer.cs b/src/Mono.Android/Android.Graphics.Drawables/DrawableContainer.cs index 1bbf66ee73d..b4c3f984d23 100644 --- a/src/Mono.Android/Android.Graphics.Drawables/DrawableContainer.cs +++ b/src/Mono.Android/Android.Graphics.Drawables/DrawableContainer.cs @@ -12,7 +12,7 @@ public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action wh public void UnscheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what) { - Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleDrawable (who, runnable)); + Java.Lang.Thread.RunnableImplementor.Remove (what, this, who, static (runnable, drawable, who) => drawable.UnscheduleDrawable (who, runnable)); } } } diff --git a/src/Mono.Android/Android.Graphics.Drawables/InsetDrawable.cs b/src/Mono.Android/Android.Graphics.Drawables/InsetDrawable.cs index 52e1c714926..a14f2a76ae6 100644 --- a/src/Mono.Android/Android.Graphics.Drawables/InsetDrawable.cs +++ b/src/Mono.Android/Android.Graphics.Drawables/InsetDrawable.cs @@ -12,7 +12,7 @@ public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action wh public void UnscheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what) { - Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleDrawable (who, runnable)); + Java.Lang.Thread.RunnableImplementor.Remove (what, this, who, static (runnable, drawable, who) => drawable.UnscheduleDrawable (who, runnable)); } } } diff --git a/src/Mono.Android/Android.Graphics.Drawables/LayerDrawable.cs b/src/Mono.Android/Android.Graphics.Drawables/LayerDrawable.cs index 3807229ef5b..2e1209b46e9 100644 --- a/src/Mono.Android/Android.Graphics.Drawables/LayerDrawable.cs +++ b/src/Mono.Android/Android.Graphics.Drawables/LayerDrawable.cs @@ -12,7 +12,7 @@ public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action wh public void UnscheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what) { - Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleDrawable (who, runnable)); + Java.Lang.Thread.RunnableImplementor.Remove (what, this, who, static (runnable, drawable, who) => drawable.UnscheduleDrawable (who, runnable)); } } } diff --git a/src/Mono.Android/Android.Graphics.Drawables/RotateDrawable.cs b/src/Mono.Android/Android.Graphics.Drawables/RotateDrawable.cs index f28ee041c9b..c4adf94e1c6 100644 --- a/src/Mono.Android/Android.Graphics.Drawables/RotateDrawable.cs +++ b/src/Mono.Android/Android.Graphics.Drawables/RotateDrawable.cs @@ -12,7 +12,7 @@ public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action wh public void UnscheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what) { - Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleDrawable (who, runnable)); + Java.Lang.Thread.RunnableImplementor.Remove (what, this, who, static (runnable, drawable, who) => drawable.UnscheduleDrawable (who, runnable)); } } } diff --git a/src/Mono.Android/Android.Graphics.Drawables/ScaleDrawable.cs b/src/Mono.Android/Android.Graphics.Drawables/ScaleDrawable.cs index 66d0981fef1..0b32dbeea80 100644 --- a/src/Mono.Android/Android.Graphics.Drawables/ScaleDrawable.cs +++ b/src/Mono.Android/Android.Graphics.Drawables/ScaleDrawable.cs @@ -12,7 +12,7 @@ public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action wh public void UnscheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what) { - Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleDrawable (who, runnable)); + Java.Lang.Thread.RunnableImplementor.Remove (what, this, who, static (runnable, drawable, who) => drawable.UnscheduleDrawable (who, runnable)); } } } diff --git a/src/Mono.Android/Android.OS/Handler.cs b/src/Mono.Android/Android.OS/Handler.cs index 047cc1adc8c..d4cba0a67d8 100644 --- a/src/Mono.Android/Android.OS/Handler.cs +++ b/src/Mono.Android/Android.OS/Handler.cs @@ -63,12 +63,12 @@ public bool PostDelayed (Action action, long delayMillis) public void RemoveCallbacks (Action action) { - Java.Lang.Thread.RunnableImplementor.Remove (action, runnable => RemoveCallbacks (runnable)); + Java.Lang.Thread.RunnableImplementor.Remove (action, this, static (runnable, handler) => handler.RemoveCallbacks (runnable)); } public void RemoveCallbacks (Action action, Java.Lang.Object token) { - Java.Lang.Thread.RunnableImplementor.Remove (action, runnable => RemoveCallbacks (runnable, token)); + Java.Lang.Thread.RunnableImplementor.Remove (action, this, token, static (runnable, handler, token) => handler.RemoveCallbacks (runnable, token)); } } diff --git a/src/Mono.Android/Android.Views/View.cs b/src/Mono.Android/Android.Views/View.cs index 5348ad2a258..9d5f5024648 100644 --- a/src/Mono.Android/Android.Views/View.cs +++ b/src/Mono.Android/Android.Views/View.cs @@ -79,7 +79,7 @@ public void ScheduleDrawable (Android.Graphics.Drawables.Drawable who, Action wh public void UnscheduleDrawable (Android.Graphics.Drawables.Drawable who, Action what) { - Java.Lang.Thread.RunnableImplementor.Remove (what, runnable => UnscheduleDrawable (who, runnable)); + Java.Lang.Thread.RunnableImplementor.Remove (what, this, who, static (runnable, view, who) => view.UnscheduleDrawable (who, runnable)); } #if ANDROID_11 diff --git a/src/Mono.Android/Java.Lang/Thread.cs b/src/Mono.Android/Java.Lang/Thread.cs index ff070b18417..7389e4815ea 100644 --- a/src/Mono.Android/Java.Lang/Thread.cs +++ b/src/Mono.Android/Java.Lang/Thread.cs @@ -85,6 +85,22 @@ public static void Remove (Action handler, Action remove) }); } + public static void Remove (Action handler, TState state, Action remove) + { + Remove (handler, (state, remove), static (context, runnable) => { + context.remove (runnable, context.state); + return false; + }); + } + + public static void Remove (Action handler, TState1 state1, TState2 state2, Action remove) + { + Remove (handler, (state1, state2, remove), static (context, runnable) => { + context.remove (runnable, context.state1, context.state2); + return false; + }); + } + public static bool Remove (Action handler, TState state, Func remove) { List pending = new (); From 8c55d90b8df9add202ead8f70d593cebe1999199 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 14:34:43 +0200 Subject: [PATCH 33/37] [Java.Interop] Delay registration ownership transfer Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/Java.Interop/JniEnvironment.Types.cs | 7 +++++++ .../src/Java.Interop/Java.Interop/JniType.cs | 10 ++++++++-- .../Java.Interop/ManagedPeerRegistrationTests.cs | 9 +++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs index 2a543700398..59054d2d62b 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs @@ -224,6 +224,12 @@ public static void RegisterNatives (JniObjectReference type, JniNativeMethodRegi [RequiresDynamicCode ("Native method registration via JniNativeMethodRegistration[] requires dynamic code generation. Use the blittable RegisterNatives(JniObjectReference, ReadOnlySpan) overload with statically-compiled function pointers for Native AOT compatibility.")] public static unsafe void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration [] methods, int numMethods) + { + RegisterNatives (type, methods, numMethods, null, null); + } + + [RequiresDynamicCode ("Native method registration via JniNativeMethodRegistration[] requires dynamic code generation. Use the blittable RegisterNatives(JniObjectReference, ReadOnlySpan) overload with statically-compiled function pointers for Native AOT compatibility.")] + internal static unsafe void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration [] methods, int numMethods, TState state, Action? beforeJniCall) { if ((numMethods < 0) || (numMethods > (methods?.Length ?? 0))) { @@ -273,6 +279,7 @@ public static unsafe void RegisterNatives (JniObjectReference type, JniNativeMet unmanagedStrings [i * 2 + 1] = sig; natives [i] = new JniNativeMethod ((byte*) name, (byte*) sig, Marshal.GetFunctionPointerForDelegate (m.Marshaler)); } + beforeJniCall?.Invoke (state); RegisterNatives (type, natives); // Keep the Marshaler delegates alive at least until JNI has consumed the function pointers. GC.KeepAlive (methods); diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs index 38296c1f346..92ff1e874cd 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs @@ -181,8 +181,14 @@ public void RegisterNativeMethods (params JniNativeMethodRegistration[] methods) if (Interlocked.CompareExchange (ref this.methods, methods, null) != null) throw new InvalidOperationException ("Native methods cannot be registered more than once."); - RegisterWithRuntime (); - JniEnvironment.Types.RegisterNatives (PeerReference, methods, methods.Length); + // Transfer ownership only after managed marshalling succeeds, immediately before + // JNI can publish any function pointers. + JniEnvironment.Types.RegisterNatives ( + PeerReference, + methods, + methods.Length, + this, + static type => type.RegisterWithRuntime ()); } public void UnregisterNativeMethods () diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs index ff19d0c9854..1bc6371f023 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs @@ -35,6 +35,15 @@ public void EmptyRegistration_AllowsNonEmptyRegistration () Assert.AreEqual (42, Call (owner, "value")); } + [Test] + public void ManagedMarshallingFailure_DoesNotAdoptOwner () + { + using var owner = new JniType (JniTypeName); + Assert.Throws (() => owner.RegisterNativeMethods (new JniNativeMethodRegistration [1])); + owner.DisposeUnlessRegisteredWithRuntime (); + Assert.IsFalse (owner.PeerReference.IsValid); + } + [TestCase (false)] [TestCase (true)] public void RegistrationAttempt_RetainsOwnerAndDelegate (bool fail) From 2424be13a855bf67e4de02e90fbeb794075a5e38 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 14:38:56 +0200 Subject: [PATCH 34/37] [Java.Interop] Pass native registration owner directly Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/Java.Interop/Java.Interop/JniEnvironment.Types.cs | 6 +++--- .../Java.Interop/src/Java.Interop/Java.Interop/JniType.cs | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs index 59054d2d62b..8d85d74b0ae 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs @@ -225,11 +225,11 @@ public static void RegisterNatives (JniObjectReference type, JniNativeMethodRegi [RequiresDynamicCode ("Native method registration via JniNativeMethodRegistration[] requires dynamic code generation. Use the blittable RegisterNatives(JniObjectReference, ReadOnlySpan) overload with statically-compiled function pointers for Native AOT compatibility.")] public static unsafe void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration [] methods, int numMethods) { - RegisterNatives (type, methods, numMethods, null, null); + RegisterNatives (type, methods, numMethods, null); } [RequiresDynamicCode ("Native method registration via JniNativeMethodRegistration[] requires dynamic code generation. Use the blittable RegisterNatives(JniObjectReference, ReadOnlySpan) overload with statically-compiled function pointers for Native AOT compatibility.")] - internal static unsafe void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration [] methods, int numMethods, TState state, Action? beforeJniCall) + internal static unsafe void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration [] methods, int numMethods, JniType? owner) { if ((numMethods < 0) || (numMethods > (methods?.Length ?? 0))) { @@ -279,7 +279,7 @@ internal static unsafe void RegisterNatives (JniObjectReference type, Jn unmanagedStrings [i * 2 + 1] = sig; natives [i] = new JniNativeMethod ((byte*) name, (byte*) sig, Marshal.GetFunctionPointerForDelegate (m.Marshaler)); } - beforeJniCall?.Invoke (state); + owner?.RegisterWithRuntime (); RegisterNatives (type, natives); // Keep the Marshaler delegates alive at least until JNI has consumed the function pointers. GC.KeepAlive (methods); diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs index 92ff1e874cd..59286b5882a 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs @@ -187,8 +187,7 @@ public void RegisterNativeMethods (params JniNativeMethodRegistration[] methods) PeerReference, methods, methods.Length, - this, - static type => type.RegisterWithRuntime ()); + this); } public void UnregisterNativeMethods () From 70c79a8df60ecfde0059294a24767a6fa26c5ac0 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 14:49:47 +0200 Subject: [PATCH 35/37] [Java.Interop] Track native registration before marshaling Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Java.Interop/Java.Interop/JniEnvironment.Types.cs | 7 ------- .../src/Java.Interop/Java.Interop/JniType.cs | 9 ++------- .../Java.Interop/ManagedPeerRegistrationTests.cs | 9 --------- 3 files changed, 2 insertions(+), 23 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs index 8d85d74b0ae..2a543700398 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs @@ -224,12 +224,6 @@ public static void RegisterNatives (JniObjectReference type, JniNativeMethodRegi [RequiresDynamicCode ("Native method registration via JniNativeMethodRegistration[] requires dynamic code generation. Use the blittable RegisterNatives(JniObjectReference, ReadOnlySpan) overload with statically-compiled function pointers for Native AOT compatibility.")] public static unsafe void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration [] methods, int numMethods) - { - RegisterNatives (type, methods, numMethods, null); - } - - [RequiresDynamicCode ("Native method registration via JniNativeMethodRegistration[] requires dynamic code generation. Use the blittable RegisterNatives(JniObjectReference, ReadOnlySpan) overload with statically-compiled function pointers for Native AOT compatibility.")] - internal static unsafe void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration [] methods, int numMethods, JniType? owner) { if ((numMethods < 0) || (numMethods > (methods?.Length ?? 0))) { @@ -279,7 +273,6 @@ internal static unsafe void RegisterNatives (JniObjectReference type, JniNativeM unmanagedStrings [i * 2 + 1] = sig; natives [i] = new JniNativeMethod ((byte*) name, (byte*) sig, Marshal.GetFunctionPointerForDelegate (m.Marshaler)); } - owner?.RegisterWithRuntime (); RegisterNatives (type, natives); // Keep the Marshaler delegates alive at least until JNI has consumed the function pointers. GC.KeepAlive (methods); diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs index 59286b5882a..38296c1f346 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs @@ -181,13 +181,8 @@ public void RegisterNativeMethods (params JniNativeMethodRegistration[] methods) if (Interlocked.CompareExchange (ref this.methods, methods, null) != null) throw new InvalidOperationException ("Native methods cannot be registered more than once."); - // Transfer ownership only after managed marshalling succeeds, immediately before - // JNI can publish any function pointers. - JniEnvironment.Types.RegisterNatives ( - PeerReference, - methods, - methods.Length, - this); + RegisterWithRuntime (); + JniEnvironment.Types.RegisterNatives (PeerReference, methods, methods.Length); } public void UnregisterNativeMethods () diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs index 1bc6371f023..ff19d0c9854 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs @@ -35,15 +35,6 @@ public void EmptyRegistration_AllowsNonEmptyRegistration () Assert.AreEqual (42, Call (owner, "value")); } - [Test] - public void ManagedMarshallingFailure_DoesNotAdoptOwner () - { - using var owner = new JniType (JniTypeName); - Assert.Throws (() => owner.RegisterNativeMethods (new JniNativeMethodRegistration [1])); - owner.DisposeUnlessRegisteredWithRuntime (); - Assert.IsFalse (owner.PeerReference.IsValid); - } - [TestCase (false)] [TestCase (true)] public void RegistrationAttempt_RetainsOwnerAndDelegate (bool fail) From 92eeb00b9acc1d2d3811f4986789190d2f6538fd Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 11 Sep 2026 17:45:19 +0200 Subject: [PATCH 36/37] [Java.Interop] Fix cache ownership CI failures Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../JniPeerMembers.JniStaticMethods.cs | 20 +++++++++++++------ .../JniPeerMembers.JniValueCache.cs | 16 ++++++++++++++- .../Java.Interop/JavaVMFixture.cs | 1 + .../Java.Interop/JniPeerMembersTests.cs | 18 ++++++++--------- .../JniRedirectCacheOwnershipTests.cs | 7 +++---- .../Mono.Android-Tests/Remaps.xml | 6 ++++++ 6 files changed, 48 insertions(+), 20 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs index 8b1945c3fee..db5d7ff72af 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs @@ -37,12 +37,20 @@ JniMethodInfo GetMethodInfo (ReadOnlySpan method, ReadOnlySpan signa var m = (JniMethodInfo?) null; var newMethod = JniEnvironment.Runtime.TypeManager.GetReplacementMethodInfo (Members.JniPeerTypeName, method, signature); if (newMethod.HasValue) { - using var t = new JniType (newMethod.Value.TargetJniType ?? Members.JniPeerTypeName); - if (t.TryGetStaticMethod ( - newMethod.Value.TargetJniMethodName is string name ? name.AsSpan () : method, - newMethod.Value.TargetJniMethodSignature is string sig ? sig.AsSpan () : signature, - out m)) { - return m; + JniType? t = new JniType (newMethod.Value.TargetJniType ?? Members.JniPeerTypeName); + try { + if (t.TryGetStaticMethod ( + newMethod.Value.TargetJniMethodName is string name ? name.AsSpan () : method, + newMethod.Value.TargetJniMethodSignature is string sig ? sig.AsSpan () : signature, + out m)) { + if (!JniEnvironment.Types.IsSameObject (t.PeerReference, Members.JniPeerType.PeerReference)) { + m.StaticRedirect = t; + t = null; + } + return m; + } + } finally { + t?.Dispose (); } } if (Members.JniPeerType.TryGetStaticMethod (method, signature, out m)) { diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniValueCache.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniValueCache.cs index af0f6488cf1..24cf259db88 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniValueCache.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniValueCache.cs @@ -1,14 +1,17 @@ #nullable enable using System; +using System.Collections; using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Threading; namespace Java.Interop { partial class JniPeerMembers { - private sealed class JniValueCache : IDisposable + private sealed class JniValueCache : IDisposable, IReadOnlyDictionary where TKey : notnull where TValue : class { @@ -22,6 +25,17 @@ public JniValueCache (int concurrencyLevel, int capacity, Action dispose this.dispose = dispose; } + public int Count => values.Count; + public IEnumerable Keys => values.Keys; + public IEnumerable Values => values.Values; + public TValue this [TKey key] => values [key]; + + public bool ContainsKey (TKey key) => values.ContainsKey (key); + public bool TryGetValue (TKey key, [MaybeNullWhen (false)] out TValue value) => values.TryGetValue (key, out value); + + public IEnumerator> GetEnumerator () => values.GetEnumerator (); + IEnumerator IEnumerable.GetEnumerator () => GetEnumerator (); + internal static JniValueCache GetOrCreate (ref JniValueCache? cache, int concurrencyLevel, int capacity, Action dispose) { var value = Volatile.Read (ref cache); diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs index 99004f98c2b..00dc145951f 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs @@ -126,6 +126,7 @@ IEnumerable CreateSimpleReferencesEnumerator (Type type) [("java/lang/Object", "remappedToToString", "()Ljava/lang/String;")] = (null, "toString", null, null, false), [("java/lang/Object", "remappedToStaticHashCode", null)] = ("net/dot/jni/test/ObjectHelper", "getHashCodeHelper", null, null, true), [("java/lang/Runtime", "remappedToGetRuntime", null)] = (null, "getRuntime", null, null, false), + [("java/lang/Runtime", "remappedToCurrentTimeMillis", "()J")] = ("java/lang/System", "currentTimeMillis", null, null, false), // NOTE: key must use *post-renamed* value, not pre-renamed value // NOTE: SourceSignature lacking return type; "closer in spirit" to what `remapping-config.json` allows diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniPeerMembersTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniPeerMembersTests.cs index 51a11362b45..7ffbb724846 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniPeerMembersTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniPeerMembersTests.cs @@ -1,5 +1,5 @@ using System; -using System.Collections.Concurrent; +using System.Collections.Generic; using System.Reflection; using System.Threading.Tasks; @@ -122,7 +122,7 @@ public void ConcurrentFirstUsePublishesSingleFieldAndStaticMethodCaches () } } - static void AssertSingleCachedValue (ConcurrentDictionary cache, string key, T [] values) + static void AssertSingleCachedValue (IReadOnlyDictionary cache, string key, T [] values) where T : class { Assert.AreEqual (1, cache.Count); @@ -131,39 +131,39 @@ static void AssertSingleCachedValue (ConcurrentDictionary cache, s Assert.AreSame (cache [key], values [0]); } - static ConcurrentDictionary GetInstanceFields (JniPeerMembers.JniInstanceFields fields) + static IReadOnlyDictionary GetInstanceFields (JniPeerMembers.JniInstanceFields fields) { var field = typeof (JniPeerMembers.JniInstanceFields).GetField ("instanceFields", BindingFlags.NonPublic | BindingFlags.Instance); return GetCache (field, fields); } - static ConcurrentDictionary GetInstanceMethods (JniPeerMembers.JniInstanceMethods methods) + static IReadOnlyDictionary GetInstanceMethods (JniPeerMembers.JniInstanceMethods methods) { var field = typeof (JniPeerMembers.JniInstanceMethods).GetField ("instanceMethods", BindingFlags.NonPublic | BindingFlags.Instance); return GetCache (field, methods); } - static ConcurrentDictionary GetSubclassConstructors (JniPeerMembers.JniInstanceMethods methods) + static IReadOnlyDictionary GetSubclassConstructors (JniPeerMembers.JniInstanceMethods methods) { var field = typeof (JniPeerMembers.JniInstanceMethods).GetField ("subclassConstructors", BindingFlags.NonPublic | BindingFlags.Instance); return GetCache (field, methods); } - static ConcurrentDictionary GetStaticFields (JniPeerMembers.JniStaticFields fields) + static IReadOnlyDictionary GetStaticFields (JniPeerMembers.JniStaticFields fields) { var field = typeof (JniPeerMembers.JniStaticFields).GetField ("staticFields", BindingFlags.NonPublic | BindingFlags.Instance); return GetCache (field, fields); } - static ConcurrentDictionary GetStaticMethods (JniPeerMembers.JniStaticMethods methods) + static IReadOnlyDictionary GetStaticMethods (JniPeerMembers.JniStaticMethods methods) { var field = typeof (JniPeerMembers.JniStaticMethods).GetField ("staticMethods", BindingFlags.NonPublic | BindingFlags.Instance); return GetCache (field, methods); } - static ConcurrentDictionary GetCache (FieldInfo field, object owner) + static IReadOnlyDictionary GetCache (FieldInfo field, object owner) { - return (ConcurrentDictionary) field.GetValue (owner); + return (IReadOnlyDictionary) field.GetValue (owner); } [Test] diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs index a167bb9c2a9..f19136efe99 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRedirectCacheOwnershipTests.cs @@ -12,7 +12,7 @@ public class JniRedirectCacheOwnershipTests : JavaVMFixture public void DisposingPeerMembersReleasesRedirect (bool isStatic) { var members = isStatic - ? new JniPeerMembers (IAndroidInterface.JniTypeName, typeof (IAndroidInterface)) + ? new JniPeerMembers (JavaLangRemappingTestRuntime.JniTypeName, typeof (JavaLangRemappingTestRuntime)) : new JniPeerMembers (JavaLangRemappingTestObject.JniTypeName, typeof (JavaLangRemappingTestObject)); JniType redirect = null; try { @@ -34,8 +34,7 @@ public void DisposingPeerMembersReleasesRedirect (bool isStatic) static unsafe void AssertRedirectIsCallable (JniPeerMembers members, bool isStatic) { if (isStatic) { - var value = members.StaticMethods.InvokeObjectMethod ("getClassName.()Ljava/lang/String;", null); - Assert.AreEqual ("DesugarAndroidInterface$-CC", JniEnvironment.Strings.ToString (ref value, JniObjectReferenceOptions.CopyAndDispose)); + Assert.Greater (members.StaticMethods.InvokeInt64Method ("remappedToCurrentTimeMillis.()J", null), 0); } else { using var value = new JavaLangRemappingTestObject (); Assert.AreEqual (value.GetHashCode (), members.InstanceMethods.InvokeNonvirtualInt32Method ("remappedToStaticHashCode.()I", value, null)); @@ -45,7 +44,7 @@ static unsafe void AssertRedirectIsCallable (JniPeerMembers members, bool isStat static JniMethodInfo GetRedirectedMethod (JniPeerMembers members, bool isStatic) { return isStatic - ? members.StaticMethods.GetMethodInfo ("getClassName.()Ljava/lang/String;") + ? members.StaticMethods.GetMethodInfo ("remappedToCurrentTimeMillis.()J") : members.InstanceMethods.GetMethodInfo ("remappedToStaticHashCode.()I"); } diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Remaps.xml b/tests/Mono.Android-Tests/Mono.Android-Tests/Remaps.xml index 53a299d9d49..09e93c89fef 100644 --- a/tests/Mono.Android-Tests/Mono.Android-Tests/Remaps.xml +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Remaps.xml @@ -21,6 +21,12 @@ source-method-name="remappedToGetRuntime" target-type="java/lang/Runtime" target-method-name="getRuntime" target-method-instance-to-static="false" /> + Date: Fri, 11 Sep 2026 22:56:56 +0200 Subject: [PATCH 37/37] [Java.Interop] Preserve repeated native registration Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/Java.Interop/Java.Interop/JniType.cs | 43 +++++++--- .../ManagedPeerRegistrationTests.cs | 85 +++++++++++++++++-- 2 files changed, 106 insertions(+), 22 deletions(-) diff --git a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs index 38296c1f346..27e7733940a 100644 --- a/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs +++ b/external/Java.Interop/src/Java.Interop/Java.Interop/JniType.cs @@ -156,13 +156,24 @@ public bool IsInstanceOfType (JniObjectReference value) return JniEnvironment.Types.IsInstanceOf (value, PeerReference); } - // Retains delegates from the batch JNI may have partially registered. - JniNativeMethodRegistration[]? methods; + object? nativeMethodsLock; + // Retains delegates from every batch JNI may have partially registered. + List? methods; + + object GetNativeMethodsLock () + { + var value = Volatile.Read (ref nativeMethodsLock); + if (value != null) + return value; + + var candidate = new object (); + return Interlocked.CompareExchange (ref nativeMethodsLock, candidate, null) ?? candidate; + } /// /// Once a non-empty registration is requested, the runtime retains this type and its - /// delegates until disposal, even if registration throws: JNI may have registered part - /// of the batch. + /// delegates until unregistration or disposal, even if registration throws: JNI may + /// have registered part of the batch. /// [RequiresDynamicCode ("Native method registration via JniNativeMethodRegistration[] requires dynamic code generation. Use the blittable RegisterNatives(JniObjectReference, ReadOnlySpan) overload with statically-compiled function pointers for Native AOT compatibility.")] public void RegisterNativeMethods (params JniNativeMethodRegistration[] methods) @@ -174,22 +185,26 @@ public void RegisterNativeMethods (params JniNativeMethodRegistration[] methods) if (methods.Length == 0) return; - // Retain the delegates before calling RegisterNatives: JNI stores only their - // unmanaged function pointers and may publish part of the batch before throwing. - // Storing them afterward could therefore leave callable pointers to collected - // delegates. The first attempt owns this JniType until disposal and cannot be retried. - if (Interlocked.CompareExchange (ref this.methods, methods, null) != null) - throw new InvalidOperationException ("Native methods cannot be registered more than once."); - - RegisterWithRuntime (); - JniEnvironment.Types.RegisterNatives (PeerReference, methods, methods.Length); + lock (GetNativeMethodsLock ()) { + // Retain each batch before calling RegisterNatives: JNI stores only the + // unmanaged function pointers and may publish part of a batch before throwing. + // Storing it afterward could therefore leave callable pointers to collected + // delegates. + this.methods ??= new List (); + this.methods.Add (methods); + RegisterWithRuntime (); + JniEnvironment.Types.RegisterNatives (PeerReference, methods, methods.Length); + } } public void UnregisterNativeMethods () { AssertValid (); - JniEnvironment.Types.UnregisterNatives (PeerReference); + lock (GetNativeMethodsLock ()) { + JniEnvironment.Types.UnregisterNatives (PeerReference); + methods = null; + } } public JniMethodInfo GetConstructor (string signature) diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs index ff19d0c9854..0a87d17dd1e 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/ManagedPeerRegistrationTests.cs @@ -5,6 +5,7 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; +using System.Threading.Tasks; using Java.Interop; @@ -65,19 +66,87 @@ public void RegistrationAttempt_RetainsOwnerAndDelegate (bool fail) [TestCase (false)] [TestCase (true)] - public void RepeatedRegistration_Throws (bool firstAttemptFails) + public void RepeatedRegistration_RetainsEveryDelegateBatch (bool firstAttemptFails) { - using var owner = new JniType (JniTypeName); - var target = new NativeTarget (); + var retained = RegisterRepeatedly (firstAttemptFails); + Collect (); + Assert.IsTrue (retained.First.IsAlive, "The first registration delegate must remain retained."); + Assert.IsTrue (retained.Second.IsAlive, "The second registration delegate must remain retained."); + using var owner = retained.Owner; + Assert.AreEqual (42, Call (owner, "value")); + Assert.AreEqual (42, Call (owner, "existing")); + } + + [MethodImpl (MethodImplOptions.NoInlining)] + static (JniType Owner, WeakReference First, WeakReference Second) RegisterRepeatedly (bool firstAttemptFails) + { + var owner = new JniType (JniTypeName); + var first = new NativeTarget (); + var second = new NativeTarget (); if (firstAttemptFails) { using var error = Assert.Throws (() => owner.RegisterNativeMethods ( - new JniNativeMethodRegistration ("value", "()I", new GetValue (target.Value)), - new JniNativeMethodRegistration ("missing", "()I", new GetValue (target.Value)))); + new JniNativeMethodRegistration ("value", "()I", new GetValue (first.Value)), + new JniNativeMethodRegistration ("missing", "()I", new GetValue (first.Value)))); } else { - owner.RegisterNativeMethods (new JniNativeMethodRegistration ("existing", "()I", new GetValue (target.Value))); + owner.RegisterNativeMethods (new JniNativeMethodRegistration ("value", "()I", new GetValue (first.Value))); } - Assert.Throws (() => - owner.RegisterNativeMethods (new JniNativeMethodRegistration ("value", "()I", new GetValue (target.Value)))); + owner.RegisterNativeMethods (new JniNativeMethodRegistration ("existing", "()I", new GetValue (second.Value))); + return (owner, new WeakReference (first), new WeakReference (second)); + } + + [Test] + public void ConcurrentRegistration_SerializesAndRetainsEveryDelegateBatch () + { + var retained = RegisterConcurrently (); + Collect (); + Assert.IsTrue (retained.First.IsAlive, "The first registration delegate must remain retained."); + Assert.IsTrue (retained.Second.IsAlive, "The second registration delegate must remain retained."); + using var owner = retained.Owner; + Assert.AreEqual (42, Call (owner, "value")); + Assert.AreEqual (42, Call (owner, "existing")); + } + + [MethodImpl (MethodImplOptions.NoInlining)] + static (JniType Owner, WeakReference First, WeakReference Second) RegisterConcurrently () + { + var owner = new JniType (JniTypeName); + var first = new NativeTarget (); + var second = new NativeTarget (); + Parallel.Invoke ( + () => owner.RegisterNativeMethods (new JniNativeMethodRegistration ("value", "()I", new GetValue (first.Value))), + () => owner.RegisterNativeMethods (new JniNativeMethodRegistration ("existing", "()I", new GetValue (second.Value)))); + return (owner, new WeakReference (first), new WeakReference (second)); + } + + [Test] + public void Unregister_AllowsRegistrationAgain () + { + using var owner = new JniType (JniTypeName); + owner.RegisterNativeMethods (new JniNativeMethodRegistration ("value", "()I", new GetValue (static (env, klass) => 41))); + Assert.AreEqual (41, Call (owner, "value")); + + owner.UnregisterNativeMethods (); + owner.RegisterNativeMethods (new JniNativeMethodRegistration ("value", "()I", new GetValue (static (env, klass) => 42))); + Assert.AreEqual (42, Call (owner, "value")); + } + + [Test] + public void Unregister_ReleasesRegisteredDelegates () + { + var retained = RegisterThenUnregister (); + Collect (); + Assert.IsFalse (retained.Target.IsAlive, "Unregistering must release retained delegates."); + retained.Owner.Dispose (); + } + + [MethodImpl (MethodImplOptions.NoInlining)] + static (JniType Owner, WeakReference Target) RegisterThenUnregister () + { + var owner = new JniType (JniTypeName); + var target = new NativeTarget (); + owner.RegisterNativeMethods (new JniNativeMethodRegistration ("value", "()I", new GetValue (target.Value))); + owner.UnregisterNativeMethods (); + return (owner, new WeakReference (target)); } static int Call (JniType owner, string name)