diff --git a/build-tools/automation/yaml-templates/stage-package-tests.yaml b/build-tools/automation/yaml-templates/stage-package-tests.yaml index caf98d3d1e4..bec988bfba0 100644 --- a/build-tools/automation/yaml-templates/stage-package-tests.yaml +++ b/build-tools/automation/yaml-templates/stage-package-tests.yaml @@ -155,6 +155,17 @@ stages: - template: /build-tools/automation/yaml-templates/start-stop-emulator.yaml + # Process-wide JNI reference counts need a fresh process without unrelated tests. + - template: /build-tools/automation/yaml-templates/apk-instrumentation.yaml + parameters: + buildConfiguration: $(XA.Build.Configuration) + configuration: Debug + testName: Mono.Android.NET_Tests-JniReferenceLeaks + project: tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj + extraBuildArgs: -p:TestsFlavor=JniReferenceLeaks -p:IncludeCategories=JniReferenceLeak -p:UseMonoRuntime=false + artifactSource: bin/TestDebug/$(DotNetTargetFramework)-android/Mono.Android.NET_Tests-Signed.apk + artifactFolder: $(DotNetTargetFramework)-JniReferenceLeaks + # Smoke coverage that the test app still builds and runs under Mono. - template: /build-tools/automation/yaml-templates/apk-instrumentation.yaml parameters: 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..b17d0bd44a8 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 @@ -1,6 +1,7 @@ #nullable enable using System; +using System.Collections.Generic; using Java.Interop; @@ -11,6 +12,9 @@ namespace Java.InteropTests [TestFixture] public class JniTypeUtf8Test : JavaVMFixture { + const string JniReferenceLeakCategory = "JniReferenceLeak"; + const int LeakCheckIterations = 100; + [Test] public unsafe void Sanity_Utf8 () { @@ -91,24 +95,77 @@ public void TryFindClass_Utf8 () } [Test] - [Ignore ("Frequently failing: https://github.com/dotnet/android/issues/12031")] + [Category (JniReferenceLeakCategory)] public void TryFindClass_Utf8_DoesNotLeakGlobalRefs () { - int grefsBefore = JniEnvironment.Runtime.GlobalReferenceCount; - JniEnvironment.Types.TryFindClass ("does/not/Exist"u8, out _); - int grefsAfter = JniEnvironment.Runtime.GlobalReferenceCount; - Assert.AreEqual (grefsBefore, grefsAfter, - "TryFindClass for non-existent classes should not leak global references"); + AssertNoSustainedGlobalReferenceGrowth (() => { + Assert.IsFalse (JniEnvironment.Types.TryFindClass ("does/not/Exist"u8, out var notFound)); + Assert.IsFalse (notFound.IsValid); + }); } [Test] + [Category (JniReferenceLeakCategory)] public void TryFindClass_String_DoesNotLeakGlobalRefs () { + AssertNoSustainedGlobalReferenceGrowth (() => { + Assert.IsFalse (JniEnvironment.Types.TryFindClass ("does/not/Exist", out var notFound)); + Assert.IsFalse (notFound.IsValid); + }); + } + + [Test] + [Category (JniReferenceLeakCategory)] + public void AssertNoSustainedGlobalReferenceGrowth_DetectsRetainedGlobalReference () + { + var objectClass = JniEnvironment.Types.FindClass ("java/lang/Object"); + var retainedReferences = new List (); + try { + Assert.Throws (() => AssertNoSustainedGlobalReferenceGrowth (() => { + retainedReferences.Add (objectClass.NewGlobalRef ()); + })); + } finally { + foreach (var retainedReference in retainedReferences) { + var reference = retainedReference; + JniObjectReference.Dispose (ref reference); + } + JniObjectReference.Dispose (ref objectClass); + } + } + + static void AssertNoSustainedGlobalReferenceGrowth (Action action) + { + for (int i = 0; i < LeakCheckIterations; i++) { + action (); + } + CollectPeers (); + int grefsBefore = JniEnvironment.Runtime.GlobalReferenceCount; - JniEnvironment.Types.TryFindClass ("does/not/Exist", out _); + for (int i = 0; i < LeakCheckIterations; i++) { + action (); + } + CollectGarbage (); int grefsAfter = JniEnvironment.Runtime.GlobalReferenceCount; - Assert.AreEqual (grefsBefore, grefsAfter, - "TryFindClass for non-existent classes should not leak global references"); + + Assert.LessOrEqual (grefsAfter, grefsBefore, + $"Operation should not leak global references after {LeakCheckIterations} iterations. " + + $"Before={grefsBefore}, After={grefsAfter}, Delta={grefsAfter - grefsBefore}"); + } + + static void CollectPeers () + { + CollectGarbage (); + JniEnvironment.Runtime.ValueManager.CollectPeers (); + JniEnvironment.Runtime.ValueManager.WaitForGCBridgeProcessing (); + CollectGarbage (); + } + + static void CollectGarbage () + { + for (int i = 0; i < 3; i++) { + GC.Collect (); + GC.WaitForPendingFinalizers (); + } } [Test] diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.RuntimeTests/TestInstrumentation.cs b/tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.RuntimeTests/TestInstrumentation.cs index 8970d10ed5b..868cf25760c 100644 --- a/tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.RuntimeTests/TestInstrumentation.cs +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.RuntimeTests/TestInstrumentation.cs @@ -11,6 +11,8 @@ namespace Xamarin.Android.RuntimeTests [Instrumentation (Name = "xamarin.android.runtimetests.TestInstrumentation")] public class TestInstrumentation : Xamarin.Android.UnitTests.TestInstrumentation { + const string JniReferenceLeakCategory = "JniReferenceLeak"; + protected TestInstrumentation (IntPtr handle, JniHandleOwnership transfer) : base (handle, transfer) { @@ -54,6 +56,11 @@ protected override IEnumerable? ExcludedCategories { categories.Add ("NetworkInterfaces"); } + // Process-wide reference counts are only stable in the dedicated filtered run. + if (!IsOnlyIncludedCategory (JniReferenceLeakCategory)) { + categories.Add (JniReferenceLeakCategory); + } + return categories.Count > 0 ? categories : null; } } @@ -66,13 +73,25 @@ protected override IEnumerable? IncludedCategories { // `configProperties` section, and we read it back with `AppContext.GetData`. // Used by lanes that want to scope a run to specific categories, e.g. // `-p:IncludeCategories=Intune` in stage-package-tests.yaml. - var value = AppContext.GetData ("IncludeCategories") as string; - if (string.IsNullOrEmpty (value)) - return null; - return value!.Split (new [] { ',', ';' }, StringSplitOptions.RemoveEmptyEntries); + var categories = GetIncludedCategories (); + return categories.Length > 0 ? categories : null; } } + static bool IsOnlyIncludedCategory (string category) + { + var categories = GetIncludedCategories (); + return categories.Length == 1 && string.Equals (categories [0], category, StringComparison.Ordinal); + } + + static string [] GetIncludedCategories () + { + var value = AppContext.GetData ("IncludeCategories") as string; + if (value == null) + return []; + return value.Split (new [] { ',', ';' }, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + } + static bool HasAppContextSwitch (string key) => AppContext.TryGetSwitch (key, out var value) && value;