diff --git a/.github/skills/tests/references/test-catalog.md b/.github/skills/tests/references/test-catalog.md index ca48bdadc0d..441d74dc117 100644 --- a/.github/skills/tests/references/test-catalog.md +++ b/.github/skills/tests/references/test-catalog.md @@ -123,6 +123,7 @@ Device: **Yes** | Test Area | Project | Notes | |-----------|---------|-------| | **runtime** (all) | `tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj` | Core runtime tests | +| **JNI reference leaks** | `tests/JniReferenceLeakTests/JniReferenceLeakTests.csproj` | Dedicated serial MSTest/MTP app for JNI reference and peer leak checks | | **networking** | Same project — tests in `Xamarin.Android.Net/` and `System.Net/` | `AndroidMessageHandlerTests`, `AndroidMessageHandlerIntegrationTests` | | **java interop (on-device)** | Same project — tests in `Java.Interop/` | `JnienvTest`, `JavaListTest` | | **android app** | Same project — tests in `Android.App/` | `Application`, `Activity` tests | @@ -154,6 +155,15 @@ Command: Results are `.trx` files under `bin/TestDebug/TestResults/` and are published as VSTest results in CI. Always pass the same configuration and MSBuild properties to the install and `dotnet test --no-build` commands (for example, `-c Release -p:UseMonoRuntime=false`). +For the dedicated JNI reference leak tests: +```bash +./dotnet-local.sh build -t:Install -c Debug tests/JniReferenceLeakTests/JniReferenceLeakTests.csproj +( + cd tests/JniReferenceLeakTests + ../../dotnet-local.sh test JniReferenceLeakTests.csproj --no-build -c Debug --report-trx --results-directory ../../bin/TestDebug/TestResults +) +``` + For `Xamarin.Android.JcwGen-Tests`, use the same pattern from `tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/`: ```bash ./dotnet-local.sh build -t:Install -c Debug tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj diff --git a/Xamarin.Android-Tests.slnx b/Xamarin.Android-Tests.slnx index 14fb985a7a5..6d830e68144 100644 --- a/Xamarin.Android-Tests.slnx +++ b/Xamarin.Android-Tests.slnx @@ -27,6 +27,7 @@ + diff --git a/build-tools/automation/azure-pipelines-nightly.yaml b/build-tools/automation/azure-pipelines-nightly.yaml index 12733b86759..a4bb08d0281 100644 --- a/build-tools/automation/azure-pipelines-nightly.yaml +++ b/build-tools/automation/azure-pipelines-nightly.yaml @@ -145,6 +145,16 @@ stages: artifactSource: bin/Test$(XA.Build.Configuration)/$(DotNetTargetFramework)-android/Mono.Android.NET_Tests-Signed.aab artifactFolder: Mono.Android-$(XA.Build.Configuration)-$(avdApiLevel) + - template: /build-tools/automation/yaml-templates/apk-instrumentation.yaml + parameters: + buildConfiguration: $(XA.Build.Configuration) + configuration: Debug + testName: JniReferenceLeakTests-$(avdApiLevel) + project: tests/JniReferenceLeakTests/JniReferenceLeakTests.csproj + extraBuildArgs: -p:UseMonoRuntime=false -p:TestAvdApiLevel=$(avdApiLevel) -p:TestAvdAbi=$(avdAbi) -p:TestAvdType=$(avdType) + artifactSource: bin/TestDebug/$(DotNetTargetFramework)-android/JniReferenceLeakTests-Signed.apk + artifactFolder: JniReferenceLeakTests-$(avdApiLevel) + - template: /build-tools/automation/yaml-templates/upload-results.yaml parameters: configuration: $(XA.Build.Configuration) diff --git a/build-tools/automation/yaml-templates/stage-package-tests.yaml b/build-tools/automation/yaml-templates/stage-package-tests.yaml index d4a684b183c..dc233aaff3b 100644 --- a/build-tools/automation/yaml-templates/stage-package-tests.yaml +++ b/build-tools/automation/yaml-templates/stage-package-tests.yaml @@ -140,6 +140,16 @@ stages: - template: /build-tools/automation/yaml-templates/start-stop-emulator.yaml + - template: /build-tools/automation/yaml-templates/apk-instrumentation.yaml + parameters: + buildConfiguration: $(XA.Build.Configuration) + configuration: Debug + testName: JniReferenceLeakTests-CoreCLR + project: tests/JniReferenceLeakTests/JniReferenceLeakTests.csproj + extraBuildArgs: -p:UseMonoRuntime=false + artifactSource: bin/TestDebug/$(DotNetTargetFramework)-android/JniReferenceLeakTests-Signed.apk + artifactFolder: $(DotNetTargetFramework)-JniReferenceLeakTests-CoreCLR + - template: /build-tools/automation/yaml-templates/apk-instrumentation.yaml parameters: configuration: $(XA.Build.Configuration) diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index 15fa92708b6..bba8767a19e 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -48,9 +48,9 @@ https://github.com/dotnet/dotnet b4b350a66ea5dcf13420747036d8b263cdf6cbef - + https://github.com/microsoft/testfx - 8609d3bf430db360f420fbe9af9b223aa372ec2b + 79f26ff23fac8769c84783fe2fa0cfc20134bcf8 diff --git a/eng/Versions.props b/eng/Versions.props index 7323919cabd..d92ef0b8f14 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -15,7 +15,7 @@ $(MicrosoftNETWorkloadEmscriptenCurrentManifest110100preview4PackageVersion) 11.0.100-preview.7.26355.102 0.11.5-preview.26355.102 - 4.3.0-preview.26355.11 + 4.3.0 10.0.9 11.0.0-preview.1.26104.118 diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaArrayContract.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaArrayContract.cs index 1279e28426c..5dd341166fa 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaArrayContract.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaArrayContract.cs @@ -10,6 +10,7 @@ namespace Java.InteropTests { public abstract class JavaArrayContract : ListContract { +#if !__ANDROID__ int lrefStartCount; [OneTimeSetUp] @@ -24,6 +25,7 @@ public void EndArrayTests () int lref = JniEnvironment.LocalReferenceCount; Assert.AreEqual (lrefStartCount, lref, "JNI local references"); } +#endif // !__ANDROID__ [Test] public void ToArray () @@ -41,4 +43,3 @@ public void ToArray () } } } - diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaObjectArrayTest.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaObjectArrayTest.cs index 3e5b9c9fc56..30bdbc21868 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaObjectArrayTest.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaObjectArrayTest.cs @@ -157,6 +157,7 @@ public class JavaObjectArray_object_ContractTest : JavaObjectArrayContractTest (() => o.UnregisterFromRuntime ()); } +#endif // !__ANDROID__ [Test] public void RegisterWithVM_PermitsAliases () diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniEnvironmentTests.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniEnvironmentTests.cs index fd91beb3d72..e71cf856db4 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniEnvironmentTests.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniEnvironmentTests.cs @@ -55,6 +55,7 @@ public unsafe void Handles_NewReturnToJniRef () } } +#if !__ANDROID__ [Test] public void References_CreatedReference_InvalidRef () { @@ -142,6 +143,6 @@ public void References_CreatedReference_GlobalRef () GetDeleteRefFunc ("java_interop_jnienv_delete_global_ref")?.Invoke (JniEnvironment.EnvironmentPointer, o.Handle); } } +#endif // !__ANDROID__ } } - diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs index dc41461b685..48a73295035 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs @@ -61,45 +61,33 @@ public void AddPeer () { } - int GetSurfacedPeersCount () + int GetSurfacedPeerCount (IJavaPeerable peer) { - return valueManager.GetSurfacedPeers ().Count; + return valueManager.GetSurfacedPeers ().Count (candidate => { + return candidate.SurfacedPeer.TryGetTarget (out var target) && object.ReferenceEquals (target, peer); + }); } [Test] public void AddPeer_NoDuplicates () { - int startPeerCount = GetSurfacedPeersCount (); using (var v = new MyDisposableObject ()) { // MyDisposableObject ctor implicitly calls AddPeer(); - Assert.AreEqual (startPeerCount + 1, GetSurfacedPeersCount (), DumpPeers ()); + Assert.AreEqual (1, GetSurfacedPeerCount (v), DumpPeers ()); valueManager.AddPeer (v); - Assert.AreEqual (startPeerCount + 1, GetSurfacedPeersCount (), DumpPeers ()); + Assert.AreEqual (1, GetSurfacedPeerCount (v), DumpPeers ()); } } [Test] public void ConstructPeer_ImplicitViaBindingConstructor_PeerIsInSurfacedPeers () { - int startPeerCount = GetSurfacedPeersCount (); - var g = new GetThis (); - var surfaced = valueManager.GetSurfacedPeers (); - Assert.AreEqual (startPeerCount + 1, surfaced.Count); - - var found = false; - foreach (var pr in surfaced) { - if (!pr.SurfacedPeer.TryGetTarget (out var p)) - continue; - if (object.ReferenceEquals (g, p)) { - found = true; - } - } - Assert.IsTrue (found); + Assert.AreEqual (1, GetSurfacedPeerCount (g), DumpPeers ()); var localRef = g.PeerReference.NewLocalRef (); g.Dispose (); - Assert.AreEqual (startPeerCount, GetSurfacedPeersCount ()); + Assert.AreEqual (0, GetSurfacedPeerCount (g)); Assert.IsNull (valueManager.PeekPeer (localRef)); JniObjectReference.Dispose (ref localRef); } @@ -107,29 +95,17 @@ public void ConstructPeer_ImplicitViaBindingConstructor_PeerIsInSurfacedPeers () [Test] public void ConstructPeer_ImplicitViaBindingMethod_PeerIsInSurfacedPeers () { - int startPeerCount = GetSurfacedPeersCount (); - var g = new GetThis (); - var surfaced = valueManager.GetSurfacedPeers (); - Assert.AreEqual (startPeerCount + 1, surfaced.Count); - - var found = false; - foreach (var pr in surfaced) { - if (!pr.SurfacedPeer.TryGetTarget (out var p)) - continue; - if (object.ReferenceEquals (g, p)) { - found = true; - } - } - Assert.IsTrue (found); + Assert.AreEqual (1, GetSurfacedPeerCount (g), DumpPeers ()); var localRef = g.PeerReference.NewLocalRef (); g.Dispose (); - Assert.AreEqual (startPeerCount, GetSurfacedPeersCount ()); + Assert.AreEqual (0, GetSurfacedPeerCount (g)); Assert.IsNull (valueManager.PeekPeer (localRef)); JniObjectReference.Dispose (ref localRef); } +#if !__ANDROID__ // https://github.com/dotnet/android/issues/11101 [Test] public void ConstructPeer_CalledMultipleTimes_ShouldNotLeakGlobalRefs () @@ -169,6 +145,7 @@ public void ConstructPeer_CalledMultipleTimes_ShouldNotLeakGlobalRefs () } } } +#endif // !__ANDROID__ [Test] diff --git a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniTypeUtf8Test.cs b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniTypeUtf8Test.cs index feba304c0a5..753acf11fef 100644 --- a/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniTypeUtf8Test.cs +++ b/external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniTypeUtf8Test.cs @@ -90,8 +90,8 @@ public void TryFindClass_Utf8 () Assert.IsFalse (notFound.IsValid); } +#if !__ANDROID__ [Test] - [Ignore ("Frequently failing: https://github.com/dotnet/android/issues/12031")] public void TryFindClass_Utf8_DoesNotLeakGlobalRefs () { int grefsBefore = JniEnvironment.Runtime.GlobalReferenceCount; @@ -110,6 +110,7 @@ public void TryFindClass_String_DoesNotLeakGlobalRefs () Assert.AreEqual (grefsBefore, grefsAfter, "TryFindClass for non-existent classes should not leak global references"); } +#endif // !__ANDROID__ [Test] public void GetMethodID_Utf8_MatchesStringOverload () diff --git a/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs b/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs index 33132c9479e..c4054b74286 100644 --- a/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs +++ b/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs @@ -101,16 +101,12 @@ public void JavaSideActivation () // To ensure that CallMethodFromCtor.class_ref is initialized } using (var c = Java.Lang.Class.FromType (typeof (ConstructorTest))) { - int initGref = Java.Interop.Runtime.GlobalReferenceCount; using (var j = Com.Xamarin.Android.CallMethodFromCtor.NewInstance (c)) { var instance = j.JavaCast(); Assert.AreSame (j, instance); Assert.IsTrue (instance.DefaultConstructorInvoked); Assert.IsTrue (instance.ActivationConstructorInvoked); } - int finiGref = Java.Interop.Runtime.GlobalReferenceCount; - Assert.AreEqual (initGref, finiGref, - string.Format ("Initial grefc={0}; final gref={1}; No GREFs should be lost!", initGref, finiGref)); } } @@ -391,4 +387,3 @@ public class B : Java.Lang.Object { } } } - diff --git a/tests/JniReferenceLeakTests/AndroidManifest.xml b/tests/JniReferenceLeakTests/AndroidManifest.xml new file mode 100644 index 00000000000..960b6866d49 --- /dev/null +++ b/tests/JniReferenceLeakTests/AndroidManifest.xml @@ -0,0 +1,6 @@ + + + + + + diff --git a/tests/JniReferenceLeakTests/AssemblyInfo.cs b/tests/JniReferenceLeakTests/AssemblyInfo.cs new file mode 100644 index 00000000000..3119b2d65c5 --- /dev/null +++ b/tests/JniReferenceLeakTests/AssemblyInfo.cs @@ -0,0 +1,3 @@ +using Microsoft.VisualStudio.TestTools.UnitTesting; + +[assembly: DoNotParallelize] diff --git a/tests/JniReferenceLeakTests/GlobalReferenceTests.cs b/tests/JniReferenceLeakTests/GlobalReferenceTests.cs new file mode 100644 index 00000000000..48dc05f52d4 --- /dev/null +++ b/tests/JniReferenceLeakTests/GlobalReferenceTests.cs @@ -0,0 +1,84 @@ +using System.Runtime.CompilerServices; + +using Android.Runtime; +using Java.Interop; + +namespace JniReferenceLeakTests; + +[TestClass] +public sealed class GlobalReferenceTests +{ + [TestMethod] + public void TryFindClassUtf8DoesNotLeakGlobalReferences () + { + ReferenceTestHelpers.AssertNoGlobalReferenceLeak (() => { + Assert.IsFalse (JniEnvironment.Types.TryFindClass ("does/not/Exist"u8, out var notFound)); + Assert.IsFalse (notFound.IsValid); + }); + } + + [TestMethod] + public void TryFindClassStringDoesNotLeakGlobalReferences () + { + ReferenceTestHelpers.AssertNoGlobalReferenceLeak (() => { + Assert.IsFalse (JniEnvironment.Types.TryFindClass ("does/not/Exist", out var notFound)); + Assert.IsFalse (notFound.IsValid); + }); + } + + [TestMethod] + public void JavaObjectArrayOperationsDoNotLeakGlobalReferences () + { + ReferenceTestHelpers.AssertNoGlobalReferenceLeak (CreateAndDisposeObjectArray); + } + + [TestMethod] + public void JavaSideActivationDoesNotLeakGlobalReferences () + { + ReferenceTestHelpers.AssertNoGlobalReferenceLeak (() => { + using var instance = ActivationProbe.CreateFromJava (); + Assert.IsTrue (instance.DefaultConstructorInvoked); + }); + } + + [MethodImpl (MethodImplOptions.NoInlining)] + static void CreateAndDisposeObjectArray () + { + var value = new object (); + using var array = new JavaObjectArray (1); + array [0] = value; + } +} + +[Register (JniName)] +public class ActivationProbe : Java.Lang.Object +{ + internal const string JniName = "net/dot/jni/referenceleaktests/ActivationProbe"; + const string FactoryJniName = "net/dot/jni/referenceleaktests/ActivationProbeFactory"; + + public bool DefaultConstructorInvoked { get; } + + public ActivationProbe () + { + DefaultConstructorInvoked = true; + } + + public static unsafe ActivationProbe CreateFromJava () + { + using var type = new JniType (JniName); + using var factory = new JniType (FactoryJniName); + var create = factory.GetStaticMethod ("create", "(Ljava/lang/Class;)Ljava/lang/Object;"); + JniArgumentValue* arguments = stackalloc JniArgumentValue [1]; + arguments [0] = new JniArgumentValue (type.PeerReference); + var reference = JniEnvironment.StaticMethods.CallStaticObjectMethod (factory.PeerReference, create, arguments); + try { + if (JniEnvironment.Runtime.ValueManager.PeekPeer (reference) is not ActivationProbe instance) { + throw new InvalidOperationException ("Java-side activation did not create an ActivationProbe peer."); + } + + return instance; + } finally { + JniObjectReference.Dispose (ref reference); + } + } +} diff --git a/tests/JniReferenceLeakTests/JniReferenceLeakTests.csproj b/tests/JniReferenceLeakTests/JniReferenceLeakTests.csproj new file mode 100644 index 00000000000..923b8dd72fe --- /dev/null +++ b/tests/JniReferenceLeakTests/JniReferenceLeakTests.csproj @@ -0,0 +1,32 @@ + + + + + $(DotNetAndroidTargetFramework) + $(AndroidMinimumDotNetApiLevel) + ..\..\bin\Test$(Configuration) + Exe + true + true + enable + enable + true + net.dot.jni.referenceleaktests + 1 + 1.0 + + full + + + + + + + + + + + diff --git a/tests/JniReferenceLeakTests/LocalReferenceTests.cs b/tests/JniReferenceLeakTests/LocalReferenceTests.cs new file mode 100644 index 00000000000..22917cc9b49 --- /dev/null +++ b/tests/JniReferenceLeakTests/LocalReferenceTests.cs @@ -0,0 +1,69 @@ +using Java.Interop; + +namespace JniReferenceLeakTests; + +[TestClass] +public sealed class LocalReferenceTests +{ + [TestMethod] + public void InvalidCreatedReferenceDoesNotChangeLocalReferenceCount () + { + int before = JniEnvironment.LocalReferenceCount; + var reference = new JniObjectReference (); + JniEnvironment.References.CreatedReference (reference); + Assert.AreEqual (before, JniEnvironment.LocalReferenceCount); + } + + [TestMethod] + public void ArrayOperationsDoNotLeakLocalReferences () + { + int before = JniEnvironment.LocalReferenceCount; + for (int i = 0; i < 20; i++) { + ExerciseArray (new JavaBooleanArray (new [] { true }), false); + ExerciseArray (new JavaSByteArray (new sbyte [] { 1 }), (sbyte)2); + ExerciseArray (new JavaCharArray (new [] { 'a' }), 'b'); + ExerciseArray (new JavaInt16Array (new short [] { 1 }), (short)2); + ExerciseArray (new JavaInt32Array (new [] { 1 }), 2); + ExerciseArray (new JavaInt64Array (new long [] { 1 }), 2L); + ExerciseArray (new JavaSingleArray (new [] { 1f }), 2f); + ExerciseArray (new JavaDoubleArray (new [] { 1d }), 2d); + ExerciseArray (new JavaObjectArray (new [] { 1 }), 2); + ExerciseArray (new JavaObjectArray (new [] { new [] { 1 } }), new [] { 2 }); + ExerciseArray (new JavaObjectArray (new [] { new [] { new [] { 1 } } }), new [] { new [] { 2 } }); + ExerciseArray (new JavaObjectArray (new [] { "value" }), "replacement"); + ExerciseArray (new JavaObjectArray (new object [] { new object () }), new object ()); + + using var firstJavaArray = new JavaInt32Array (new [] { 1 }); + using var secondJavaArray = new JavaInt32Array (new [] { 2 }); + ExerciseArray (new JavaObjectArray (new [] { firstJavaArray }), secondJavaArray); + + using var firstJavaObject = new JavaObject (); + using var secondJavaObject = new JavaObject (); + ExerciseArray (new JavaObjectArray (new [] { firstJavaObject }), secondJavaObject); + } + + Assert.AreEqual (before, JniEnvironment.LocalReferenceCount); + } + + [TestMethod] + public void JavaSideActivationDoesNotLeakLocalReferences () + { + int before = JniEnvironment.LocalReferenceCount; + for (int i = 0; i < 20; i++) { + using var instance = ActivationProbe.CreateFromJava (); + Assert.IsTrue (instance.DefaultConstructorInvoked); + } + + Assert.AreEqual (before, JniEnvironment.LocalReferenceCount); + } + + static void ExerciseArray (JavaArray array, T replacement) + { + using (array) { + _ = array [0]; + array [0] = replacement; + Assert.AreEqual (1, array.ToArray ().Length); + } + } + +} diff --git a/tests/JniReferenceLeakTests/PeerReferenceTests.cs b/tests/JniReferenceLeakTests/PeerReferenceTests.cs new file mode 100644 index 00000000000..4bb463fecc6 --- /dev/null +++ b/tests/JniReferenceLeakTests/PeerReferenceTests.cs @@ -0,0 +1,135 @@ +using System.Runtime.CompilerServices; + +using Java.Interop; + +namespace JniReferenceLeakTests; + +[TestClass] +public sealed class PeerReferenceTests +{ + [TestMethod] + public void UnregisterFromRuntimeRemovesSurfacedPeer () + { + using (var warmup = new JavaObject ()) { + } + ReferenceTestHelpers.CollectGarbage (); + + JniObjectReference localReference; + JavaObject instance; + using (instance = new JavaObject ()) { + localReference = instance.PeerReference.NewLocalRef (); + Assert.AreEqual (JniObjectReferenceType.Global, instance.PeerReference.Type); + Assert.AreEqual (1, CountSurfacedPeer (instance)); + Assert.AreSame (instance, JniRuntime.CurrentRuntime.ValueManager.PeekPeer (localReference)); + } + + Assert.AreEqual (0, CountSurfacedPeer (instance)); + Assert.IsNull (JniRuntime.CurrentRuntime.ValueManager.PeekPeer (localReference)); + JniObjectReference.Dispose (ref localReference); + Assert.Throws (() => instance.UnregisterFromRuntime ()); + } + + [TestMethod] + public void AddPeerDoesNotRegisterDuplicates () + { + var instance = new JavaObject (); + try { + Assert.AreEqual (1, CountSurfacedPeer (instance)); + JniRuntime.CurrentRuntime.ValueManager.AddPeer (instance); + Assert.AreEqual (1, CountSurfacedPeer (instance)); + } finally { + instance.Dispose (); + } + + Assert.AreEqual (0, CountSurfacedPeer (instance)); + } + + [TestMethod] + public void RepeatedConstructPeerDoesNotLeakGlobalReferences () + { + ReferenceTestHelpers.AssertNoGlobalReferenceLeak (() => { + using var original = new JavaObject (); + var handle = original.PeerReference; + var peer = (IJavaPeerable)RuntimeHelpers.GetUninitializedObject (typeof (JavaObject)); + peer.SetPeerReference (new JniObjectReference (handle.Handle)); + + try { + var first = new JniObjectReference (handle.Handle); + JniRuntime.CurrentRuntime.ValueManager.ConstructPeer (peer, ref first, JniObjectReferenceOptions.Copy); + + var second = new JniObjectReference (handle.Handle); + JniRuntime.CurrentRuntime.ValueManager.ConstructPeer (peer, ref second, JniObjectReferenceOptions.Copy); + } finally { + peer.Dispose (); + } + }); + } + + [TestMethod] + public void WeakPeerIsCollectedWithoutLeakingReferences () + { + if (AppContext.TryGetSwitch ("Microsoft.Android.Runtime.RuntimeFeature.IsNativeAotRuntime", out bool isNativeAot) && isNativeAot) { + Assert.Inconclusive ("Failing in NativeAOT: https://github.com/dotnet/android/issues/11690"); + return; + } + + ReferenceTestHelpers.CollectGarbage (); + int before = JniEnvironment.Runtime.ValueManager.GetSurfacedPeers ().Count; + + WeakReference []? weakReferences = null; + Exception? threadException = null; + var thread = new Thread (() => { + try { + weakReferences = CreateWeakReferences (100); + } catch (Exception ex) { + threadException = ex; + } + }); + thread.Start (); + thread.Join (); + + if (threadException is not null) { + throw new AssertFailedException ("Worker thread failed.", threadException); + } + + ReferenceTestHelpers.CollectGarbage (); + if (weakReferences is null) { + throw new AssertFailedException ("The worker thread did not create the expected weak references."); + } + + Assert.IsTrue (weakReferences.All (reference => !reference.TryGetTarget (out _))); + + int after = JniEnvironment.Runtime.ValueManager.GetSurfacedPeers ().Count; + Assert.IsTrue ( + after - before <= 5, + $"Surfaced peer count increased after collecting 100 weak peers. Before={before}, After={after}, Delta={after - before}"); + } + + [MethodImpl (MethodImplOptions.NoInlining)] + static WeakReference [] CreateWeakReferences (int count) + { + var instances = new LeakRunnable [count]; + var references = new WeakReference [count]; + for (int i = 0; i < count; i++) { + instances [i] = new LeakRunnable (); + references [i] = new WeakReference (instances [i]); + } + + GC.KeepAlive (instances); + return references; + } + + static int CountSurfacedPeer (IJavaPeerable peer) + { + return JniRuntime.CurrentRuntime.ValueManager.GetSurfacedPeers ().Count (candidate => { + return candidate.SurfacedPeer.TryGetTarget (out var target) && ReferenceEquals (target, peer); + }); + } +} + +sealed class LeakRunnable : Java.Lang.Object, Java.Lang.IRunnable +{ + public void Run () + { + } +} diff --git a/tests/JniReferenceLeakTests/ReferenceTestHelpers.cs b/tests/JniReferenceLeakTests/ReferenceTestHelpers.cs new file mode 100644 index 00000000000..7961134a0bb --- /dev/null +++ b/tests/JniReferenceLeakTests/ReferenceTestHelpers.cs @@ -0,0 +1,36 @@ +using Java.Interop; + +namespace JniReferenceLeakTests; + +static class ReferenceTestHelpers +{ + public static void AssertNoGlobalReferenceLeak (Action action, int iterations = 100, int allowedIncrease = 5) + { + for (int i = 0; i < iterations; i++) { + action (); + } + CollectGarbage (); + + int before = JniEnvironment.Runtime.GlobalReferenceCount; + for (int i = 0; i < iterations; i++) { + action (); + } + CollectGarbage (); + int after = JniEnvironment.Runtime.GlobalReferenceCount; + + Assert.IsTrue ( + after - before <= allowedIncrease, + $"Global reference count increased by more than {allowedIncrease} after {iterations} iterations. Before={before}, After={after}, Delta={after - before}"); + } + + public static void CollectGarbage () + { + for (int i = 0; i < 3; i++) { + GC.Collect (); + GC.WaitForPendingFinalizers (); + } + + JniEnvironment.Runtime.ValueManager.CollectPeers (); + JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); + } +} diff --git a/tests/JniReferenceLeakTests/Resources/layout/leak_test_widget.axml b/tests/JniReferenceLeakTests/Resources/layout/leak_test_widget.axml new file mode 100644 index 00000000000..ab55cb00d79 --- /dev/null +++ b/tests/JniReferenceLeakTests/Resources/layout/leak_test_widget.axml @@ -0,0 +1,6 @@ + + diff --git a/tests/JniReferenceLeakTests/TestInstrumentation.cs b/tests/JniReferenceLeakTests/TestInstrumentation.cs new file mode 100644 index 00000000000..9dfdc0bf4be --- /dev/null +++ b/tests/JniReferenceLeakTests/TestInstrumentation.cs @@ -0,0 +1,110 @@ +using Android.Runtime; +using Microsoft.Testing.Extensions; +using Microsoft.Testing.Platform.Builder; +using Microsoft.Testing.Platform.Extensions; +using Microsoft.Testing.Platform.Extensions.Messages; + +namespace JniReferenceLeakTests; + +[Instrumentation (Name = "net.dot.jni.referenceleaktests.TestInstrumentation")] +public class TestInstrumentation : Instrumentation +{ + protected TestInstrumentation (IntPtr handle, JniHandleOwnership ownership) + : base (handle, ownership) + { + } + + public override void OnCreate (Bundle? arguments) + { + base.OnCreate (arguments); + Start (); + } + + public override async void OnStart () + { + base.OnStart (); + + var consumer = new ResultConsumer (this); + var bundle = new Bundle (); + try { + var writeablePath = Application.Context.GetExternalFilesDir (null)?.AbsolutePath ?? Path.GetTempPath (); + var resultsPath = Path.Combine (writeablePath, "TestResults"); + var builder = await TestApplication.CreateBuilderAsync ([ + "--results-directory", resultsPath, + "--report-trx", + ]); + builder.AddMSTest (() => [GetType ().Assembly]); + builder.AddTrxReportProvider (); + builder.TestHost.AddDataConsumer (_ => consumer); + + using ITestApplication app = await builder.BuildAsync (); + await app.RunAsync (); + + bundle.PutInt ("passed", consumer.Passed); + bundle.PutInt ("failed", consumer.Failed); + bundle.PutInt ("skipped", consumer.Skipped); + bundle.PutString ("resultsPath", consumer.TrxReportPath); + Finish (Result.Ok, bundle); + } catch (Exception ex) { + bundle.PutString ("error", ex.ToString ()); + Finish (Result.Canceled, bundle); + } + } + + class ResultConsumer (Instrumentation instrumentation) : IDataConsumer + { + int passed; + int failed; + int skipped; + + public int Passed => passed; + public int Failed => failed; + public int Skipped => skipped; + public string? TrxReportPath { get; private set; } + + public string Uid => nameof (ResultConsumer); + public string DisplayName => nameof (ResultConsumer); + public string Description => ""; + public string Version => "1.0"; + + public Type [] DataTypesConsumed => [typeof (TestNodeUpdateMessage), typeof (SessionFileArtifact)]; + + public Task IsEnabledAsync () => Task.FromResult (true); + + public Task ConsumeAsync (IDataProducer dataProducer, IData value, CancellationToken cancellationToken) + { + if (value is SessionFileArtifact artifact) { + TrxReportPath = artifact.FileInfo.FullName; + return Task.CompletedTask; + } + + if (value is not TestNodeUpdateMessage { TestNode: var node }) { + return Task.CompletedTask; + } + + var state = node.Properties.SingleOrDefault (); + string? outcome = state switch { + PassedTestNodeStateProperty => "passed", + FailedTestNodeStateProperty or ErrorTestNodeStateProperty or TimeoutTestNodeStateProperty => "failed", + SkippedTestNodeStateProperty => "skipped", + _ => null, + }; + if (outcome is null) { + return Task.CompletedTask; + } + + _ = outcome switch { + "passed" => Interlocked.Increment (ref passed), + "failed" => Interlocked.Increment (ref failed), + _ => Interlocked.Increment (ref skipped), + }; + + var id = node.Properties.SingleOrDefault (); + var bundle = new Bundle (); + bundle.PutString ("test", id is not null ? $"{id.Namespace}.{id.TypeName}.{id.MethodName}" : node.DisplayName); + bundle.PutString ("outcome", outcome); + instrumentation.SendStatus (0, bundle); + return Task.CompletedTask; + } + } +} diff --git a/tests/JniReferenceLeakTests/WidgetReferenceTests.cs b/tests/JniReferenceLeakTests/WidgetReferenceTests.cs new file mode 100644 index 00000000000..3e5b5aafa19 --- /dev/null +++ b/tests/JniReferenceLeakTests/WidgetReferenceTests.cs @@ -0,0 +1,39 @@ +using Android.App; +using Android.Content; +using Android.Runtime; +using Android.Util; +using Android.Views; +using Android.Widget; + +namespace JniReferenceLeakTests; + +[TestClass] +public sealed class WidgetReferenceTests +{ + [TestMethod] + public void InflateCustomViewDoesNotLeakGlobalReferences () + { + var inflater = Application.Context.GetSystemService (Context.LayoutInflaterService) as LayoutInflater; + if (inflater is null) { + throw new AssertFailedException ("Could not obtain LayoutInflater."); + } + + ReferenceTestHelpers.AssertNoGlobalReferenceLeak (() => { + using var view = inflater.Inflate (Resource.Layout.leak_test_widget, null); + }); + } +} + +[Register ("net/dot/jni/referenceleaktests/LeakTestButton")] +public class LeakTestButton : Button +{ + public LeakTestButton (Context context) + : base (context) + { + } + + public LeakTestButton (Context context, IAttributeSet attributes) + : base (context, attributes) + { + } +} diff --git a/tests/JniReferenceLeakTests/global.json b/tests/JniReferenceLeakTests/global.json new file mode 100644 index 00000000000..3140116df39 --- /dev/null +++ b/tests/JniReferenceLeakTests/global.json @@ -0,0 +1,5 @@ +{ + "test": { + "runner": "Microsoft.Testing.Platform" + } +} diff --git a/tests/JniReferenceLeakTests/java/net/dot/jni/referenceleaktests/ActivationProbeFactory.java b/tests/JniReferenceLeakTests/java/net/dot/jni/referenceleaktests/ActivationProbeFactory.java new file mode 100644 index 00000000000..57cb2d6b389 --- /dev/null +++ b/tests/JniReferenceLeakTests/java/net/dot/jni/referenceleaktests/ActivationProbeFactory.java @@ -0,0 +1,9 @@ +package net.dot.jni.referenceleaktests; + +public final class ActivationProbeFactory +{ + public static Object create (Class type) throws ReflectiveOperationException + { + return type.getDeclaredConstructor ().newInstance (); + } +} diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs b/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs index 455905b1bd6..28ed9a36246 100644 --- a/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs @@ -46,47 +46,6 @@ public void UpperAndLowerCaseCustomWidget_FromLibrary_ShouldNotThrowInflateExcep }, "Regression test for widgets with uppercase and lowercase namespace (bug #23880) failed."); } - // https://github.com/dotnet/android/issues/11101 - [Test] - [Ignore ("Currently failing/flaky due to unresolved custom view global ref leak: https://github.com/dotnet/android/issues/11201")] - public void InflateCustomView_ShouldNotLeakGlobalRefs () - { - var inflater = (LayoutInflater) Application.Context.GetSystemService (Context.LayoutInflaterService); - Assert.IsNotNull (inflater); - - // Warm up: inflate once to populate caches and type mappings, - // and let any background thread activity from previous tests settle. - inflater.Inflate (Resource.Layout.lowercase_custom, null); - CollectGarbage (times: 3); - - int grefBefore = Java.Interop.Runtime.GlobalReferenceCount; - - // Use a large number of inflations so that a real leak (3+ global refs - // per inflate) produces a delta far above any background noise from - // Android system services, GC bridge processing, or finalizer threads. - const int inflateCount = 100; - for (int i = 0; i < inflateCount; i++) { - inflater.Inflate (Resource.Layout.lowercase_custom, null); - } - - CollectGarbage (times: 3); - - int grefAfter = Java.Interop.Runtime.GlobalReferenceCount; - int delta = grefAfter - grefBefore; - - // A real leak would produce delta >= 300 (3 leaked refs per inflate). - // Use a generous threshold to tolerate background noise on real devices. - Assert.IsTrue (delta <= 100, - $"Global reference leak detected: {delta} extra global refs after inflating/GC'ing {inflateCount} custom views. Before={grefBefore}, After={grefAfter}"); - - static void CollectGarbage (int times) - { - for (int i = 0; i < times; i++) { - GC.Collect (); - GC.WaitForPendingFinalizers (); - } - } - } } public class CustomButton : Button diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs index 917710ab797..50908570941 100644 --- a/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs @@ -506,52 +506,6 @@ public void ManagedToJavaTypeMapping () Assert.AreEqual (null, m, "`JnienvTest` does *not* subclass Java.Lang.Object, it should *not* be in the typemap!"); } - [Test, Category ("GCBridge")] - [Ignore ("Failing in NativeAOT: https://github.com/dotnet/android/issues/11690")] - public void DoNotLeakWeakReferences () - { - GC.Collect (); - GC.WaitForPendingFinalizers (); - - var surfaced = Runtime.GetSurfacedObjects (); - int startCount = surfaced.Count; - - Assert.IsTrue (surfaced.All (s => s.Target != null), "#1"); - - // `Runtime.GetSurfacedObjects()` is process-global: NUnit/MTP - // infrastructure (per-test TestExecutionContext, listeners, Console - // capture, etc.) creates and releases transient Java.Lang.Object - // peers around every test, so the count drifts by a few entries - // between the snapshots below. Allow a small tolerance instead of - // requiring an exact count -- a real leak would dwarf this. - const int tolerance = 10; - - WeakReference r = null; - Exception threadException = null; - var t = new Thread (() => { - try { - var c = new MyCb (); - Assert.That (Runtime.GetSurfacedObjects ().Count, - Is.EqualTo (startCount + 1).Within (tolerance), "#2"); - r = new WeakReference (c); - } catch (Exception e) { - threadException = e; - } - }); - t.Start (); - t.Join (); - if (threadException != null) - throw new Exception ("Worker thread failed", threadException); - - GC.Collect (); - GC.WaitForPendingFinalizers (); - GC.Collect (); - GC.WaitForPendingFinalizers (); - - surfaced = Runtime.GetSurfacedObjects (); - Assert.That (surfaced.Count, Is.EqualTo (startCount).Within (tolerance), "#3"); - Assert.IsTrue (surfaced.All (s => s.Target != null), "#4"); - } } [Register ("from/NewNativeThreadOne")] @@ -576,13 +530,6 @@ public override void Run () } } - class MyCb : Java.Lang.Object, Java.Lang.IRunnable { - public void Run () - { - Console.WriteLine ("MyCb.Run! JNIEnv.Handle={0}", JNIEnv.Handle.ToString ("x")); - } - } - class ContainsExportedMethods : Java.Lang.Object { public bool Constructed;