Skip to content

[tests] Isolate JNI reference leak tests#12038

Open
simonrozsival wants to merge 8 commits into
mainfrom
dev/simonrozsival/dedicated-jni-reference-leak-tests
Open

[tests] Isolate JNI reference leak tests#12038
simonrozsival wants to merge 8 commits into
mainfrom
dev/simonrozsival/dedicated-jni-reference-leak-tests

Conversation

@simonrozsival

@simonrozsival simonrozsival commented Jul 11, 2026

Copy link
Copy Markdown
Member

Summary

  • add a dedicated on-device MSTest/Microsoft Testing Platform app for JNI reference and peer leak checks
  • serialize the entire assembly with [assembly: DoNotParallelize]
  • migrate Android GREF, LREF, surfaced-peer, weak-peer, Java-side activation, object-array, and custom-widget leak checks out of shared test apps
  • replace ambient process-total assertions with amplified leak checks or object-specific peer assertions
  • run the app under CoreCLR in package CI, with additional nightly emulator coverage
  • update the central MSTest dependency to the stable 4.3.0 release

Why

GREF and surfaced-peer counts are process-wide. The existing Android test apps load many unrelated fixtures into one process, and runtime or test-infrastructure work on background threads can change those totals while a test is taking before/after snapshots. Serializing NUnit workers did not prevent these failures.

The new app contains only reference-accounting tests and disables MSTest parallelism at the assembly level. Process-wide checks execute equal 100-operation warmup and measurement batches, allowing only a small fixed amount of ambient drift, so a per-operation leak is strongly amplified. Thread-local LREF checks retain exact accounting, while registry tests count only the specific peer under test.

Functional coverage remains in the original suites, and JVM-only accounting tests continue to run there. The custom-widget test previously quarantined under #11201 now passes reliably in the isolated process.

Validation

  • make prepare && make all with serialized MSBuild
  • local restore/build with TreatWarningsAsErrors=true: 0 warnings
  • source-built JniReferenceLeakTests: 12 passed, 0 failed, 0 skipped under CoreCLR
  • 10 additional fresh-process CoreCLR runs: all passed
  • Java.Interop-Tests: 672 passed, 4 existing skips
  • Mono.Android.NET-Tests.csproj build
  • modified YAML and XML files parsed successfully
  • git diff --check

Fixes #12031.
Supersedes #12037.

Move JNI reference and peer leak checks into a dedicated serial MSTest/MTP app and run it under CoreCLR and Mono in CI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 7be7d34c-aff3-4338-b15b-6061bc7a3ba9
Copilot AI review requested due to automatic review settings July 11, 2026 17:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR isolates JNI reference/peer leak checks into a dedicated on-device MSTest/Microsoft Testing Platform (MTP) test app (tests/JniReferenceLeakTests) to avoid flaky process-wide assertions caused by unrelated background activity in shared device test processes.

Changes:

  • Added a dedicated JniReferenceLeakTests Android test app with serialized MSTest execution ([assembly: DoNotParallelize]) and focused leak/peer assertions.
  • Migrated/removed flaky or process-global JNI reference/peer-count assertions from shared device test suites; narrowed remaining assertions to object-specific checks or restricted them to non-Android runs.
  • Wired the new test app into solution/test documentation and into CI (package tests for CoreCLR + Mono; nightly emulator coverage).
Show a summary per file
File Description
Xamarin.Android-Tests.slnx Adds the new JniReferenceLeakTests project to the tests solution structure.
tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs Removes the weak-peer surfaced-object drift test from the shared device suite.
tests/Mono.Android-Tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs Removes the previously ignored/flaky custom-widget GREF leak test from the shared suite.
tests/JniReferenceLeakTests/JniReferenceLeakTests.csproj New dedicated Android MSTest/MTP app project for leak/peer accounting tests.
tests/JniReferenceLeakTests/TestInstrumentation.cs Implements an MTP-based Android Instrumentation runner for MSTest with TRX reporting.
tests/JniReferenceLeakTests/AssemblyInfo.cs Disables MSTest parallelism at the assembly level to stabilize process-wide accounting.
tests/JniReferenceLeakTests/ReferenceTestHelpers.cs Adds helpers to amplify leak detection and synchronize GC/peer collection.
tests/JniReferenceLeakTests/GlobalReferenceTests.cs Adds amplified GREF leak checks (TryFindClass, activation, object arrays).
tests/JniReferenceLeakTests/LocalReferenceTests.cs Adds thread-local LREF accounting checks in an isolated process.
tests/JniReferenceLeakTests/PeerReferenceTests.cs Adds object-specific surfaced-peer assertions + weak-peer collection check in isolation.
tests/JniReferenceLeakTests/WidgetReferenceTests.cs Reintroduces the custom-view inflate leak check in the isolated harness.
tests/JniReferenceLeakTests/Resources/layout/leak_test_widget.axml Layout used to inflate a custom managed widget for the leak test.
tests/JniReferenceLeakTests/java/net/dot/jni/referenceleaktests/ActivationProbeFactory.java Java-side factory to exercise Java-side activation paths.
tests/JniReferenceLeakTests/AndroidManifest.xml Manifest for the new test app.
tests/JniReferenceLeakTests/global.json Forces Microsoft Testing Platform runner behavior when invoking dotnet test.
tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs Removes process-global GREF equality assertion from Java-side activation test.
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniTypeUtf8Test.cs Restricts process-global GREF leak assertions to non-Android runs.
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs Changes peer assertions from total-count based to peer-specific checks; restricts GREF leak test to non-Android runs.
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniEnvironmentTests.cs Restricts process-global reference-count assertions to non-Android runs.
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaObjectTest.cs Restricts runtime-global surfaced-peer count assertion to non-Android runs.
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaObjectArrayTest.cs Restricts fixture-wide process-global GREF baseline checks to non-Android runs.
external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaArrayContract.cs Restricts fixture-wide LREF baseline checks to non-Android runs (covered in new isolated app).
build-tools/automation/yaml-templates/stage-package-tests.yaml Adds CI legs to run the new app under CoreCLR and Mono in package tests.
build-tools/automation/azure-pipelines-nightly.yaml Adds nightly emulator coverage for the new isolated leak test app.
.github/skills/tests/references/test-catalog.md Documents how to build/run the new dedicated leak test app locally.

Copilot's findings

  • Files reviewed: 25/25 changed files
  • Comments generated: 0

Keep the dedicated on-device JNI reference leak checks on CoreCLR only.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 7be7d34c-aff3-4338-b15b-6061bc7a3ba9
Treat warnings as errors in the leak-test project locally so unavailable package versions fail before CI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 7be7d34c-aff3-4338-b15b-6061bc7a3ba9
Track the stable TestFX release centrally and keep the leak-test project on MSTestPackageVersion with local warnings treated as errors.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 7be7d34c-aff3-4338-b15b-6061bc7a3ba9
@simonrozsival

Copy link
Copy Markdown
Member Author

✅ New JniReferenceLeakTests project — all tests green on CI

Confirmed on the public dotnet-android build #1504876, in Package Tests → macOS → APKs 2: the new isolated project built, deployed to the emulator, ran under CoreCLR, and passed 12/12 tests — 0 failed, 0 skipped.

Lane tasks all succeeded: build JniReferenceLeakTests-CoreCLRrun JniReferenceLeakTests-CoreCLRpublish JniReferenceLeakTests-CoreCLR results (the diagnose install failure step was skipped, i.e. a clean deploy).

12 passing tests

GlobalReferenceTests

  • JavaObjectArrayOperationsDoNotLeakGlobalReferences
  • JavaSideActivationDoesNotLeakGlobalReferences
  • TryFindClassStringDoesNotLeakGlobalReferences
  • TryFindClassUtf8DoesNotLeakGlobalReferences

LocalReferenceTests

  • ArrayOperationsDoNotLeakLocalReferences
  • InvalidCreatedReferenceDoesNotChangeLocalReferenceCount
  • JavaSideActivationDoesNotLeakLocalReferences

PeerReferenceTests

  • AddPeerDoesNotRegisterDuplicates
  • RepeatedConstructPeerDoesNotLeakGlobalReferences
  • UnregisterFromRuntimeRemovesSurfacedPeer
  • WeakPeerIsCollectedWithoutLeakingReferences

WidgetReferenceTests

  • InflateCustomViewDoesNotLeakGlobalReferences

ℹ️ The one red lane on this build (APKs 1build Mono.Android.NET_Tests-Debug) is a separate binding-compile break in the pre-existing Java.Interop-Tests project (CS0436/CS0108/CS0114 in generated Net.Dot.Jni.Test.* code) — unrelated to these new tests.

🤖 Automated CI status summary via GitHub Copilot CLI.

@simonrozsival simonrozsival added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Jul 12, 2026
Comment thread eng/Version.Details.xml Outdated
…icated-jni-reference-leak-tests

# Conflicts:
#	eng/Version.Details.xml
#	eng/Versions.props
@simonrozsival

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Android PR Reviewer completed successfully!

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Review — Isolate JNI reference leak tests

Solid, well-motivated PR. Moving process-wide GREF/surfaced-peer accounting out of the shared multi-fixture test apps into a dedicated [assembly: DoNotParallelize] MSTest/MTP app is the right fix for the flakiness described in #12031: the refactored contract tests now count only the specific peer under test (GetSurfacedPeerCount(peer)) instead of process-global totals, and the warmup+measurement amplification in AssertNoGlobalReferenceLeak is a sound way to turn per-op leaks into detectable deltas. Guarding the process-wide assertions in the Java.Interop suite with #if !__ANDROID__ cleanly leaves JVM-only accounting where it belongs.

Verdict: ⚠️ Needs Changes (minor) — nothing blocking; mostly the Debug-vs-Release CI question worth a maintainer decision.

Findings

  • 💡 Unreachable return; after Assert.Inconclusive in PeerReferenceTests.
  • 💡 Surfaced-peer tolerance/batch duplicated inline instead of reusing the ReferenceTestHelpers amplification helper.
  • 💡 MTP run exceptions in TestInstrumentation are only surfaced via the result Bundle; also log to adb logcat.
  • ⚠️ Nightly + package lanes build this app as Debug only, so full trimming (Release-only) leaks aren't exercised — please confirm intent.

Notes

  • CI status is still pending on the head commit — the Azure DevOps dotnet-android pipeline hadn't reported at review time. This isn't LGTM-able until those checks are green; please re-check the pipeline (and the newly-added JniReferenceLeakTests lanes) before merge.
  • Nice touches: per-thread exact LREF accounting retained, [MethodImpl(NoInlining)] on the weak-reference/array creators to keep locals from being kept alive, and the migrated custom-widget test (#11201) now running in isolation.

Positive callout: the diff meaningfully reduces ambient-noise coupling rather than just papering over flakes with wider tolerances. 👍

Generated by Android PR Reviewer for #12038 · 116.1 AIC · ⌖ 13.3 AIC · ⊞ 6.8K
Comment /review to run again

Comment thread tests/JniReferenceLeakTests/PeerReferenceTests.cs
Comment thread build-tools/automation/azure-pipelines-nightly.yaml
Comment thread tests/JniReferenceLeakTests/TestInstrumentation.cs
Comment thread tests/JniReferenceLeakTests/PeerReferenceTests.cs Outdated
simonrozsival and others added 2 commits July 13, 2026 23:48
Addresses the Android PR Reviewer feedback on #12038:

- Remove the unreachable `return;` after `Assert.Inconclusive` in
  PeerReferenceTests (Assert.Inconclusive always throws).
- Log MTP run exceptions to logcat in TestInstrumentation so failures
  remain observable via `adb logcat` even if the result Bundle is
  missed or truncated.
- Add a Release JniReferenceLeakTests lane to the nightly pipeline so
  full trimming (TrimMode=full, Release-only) reference leaks are
  exercised. The PR package-tests lane stays Debug to keep PR CI fast.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ad688158-19a2-49f4-a9d3-09a9457d0dfd
JniReferenceLeakTests is the first project to restore the Maestro-managed
MSTest package ($(MSTestPackageVersion)) in the public dnceng-public
pipeline. That version (currently 4.4.0-preview.26360.5, flowed from
microsoft/testfx) is published to the public `dotnet-tools` feed, which
was missing from NuGet.config. Restore therefore failed with:

  error NU1102: Unable to find package MSTest with version
  (>= 4.4.0-preview.26360.5)  [nearest on dotnet-public: 4.3.2]

Add the `dotnet-tools` feed so the darc-managed MSTest preview resolves
in the public pipeline, fixing the JniReferenceLeakTests build lanes
(and any other $(MSTestPackageVersion) consumer) without pinning away
from the centrally-managed version.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ad688158-19a2-49f4-a9d3-09a9457d0dfd
@simonrozsival

Copy link
Copy Markdown
Member Author

@copilot build fails with:

Build FAILED.

/Users/runner/work/1/s/tests/JniReferenceLeakTests/JniReferenceLeakTests.csproj : error NU1603: Warning As Error: MSTest 4.4.0-preview.26360.5 depends on Microsoft.Testing.Extensions.CodeCoverage (>= 18.9.0-preview.26358.3) but Microsoft.Testing.Extensions.CodeCoverage 18.9.0-preview.26358.3 was not found. Microsoft.Testing.Extensions.CodeCoverage 18.9.0 was resolved instead.
    0 Warning(s)
    1 Error(s)

look for best ways to resolve this.

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot AI commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@copilot build fails with:

Build FAILED.

...

Fixed in e254bf4 by adding an explicit Microsoft.Testing.Extensions.CodeCoverage 18.9.0 reference. The required preview package is no longer available on configured feeds; explicitly selecting its compatible stable release removes NU1603. Restore and build now complete with TreatWarningsAsErrors=true (0 warnings).

@simonrozsival simonrozsival removed the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Jul 14, 2026
Comment thread NuGet.config
Comment on lines +16 to +17
<!-- Hosts the Maestro-flowed MSTest previews (from microsoft/testfx) referenced via $(MSTestPackageVersion); required to restore MSTest-based test apps such as tests/JniReferenceLeakTests. -->
<add key="dotnet-tools" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-tools/nuget/v3/index.json" protocolVersion="3" />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this true? I had been using:

<add key="test-tools" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/test-tools/nuget/v3/index.json" protocolVersion="3" />

How does main work, if this is needed?

Comment on lines +9 to +15
[Instrumentation (Name = "net.dot.jni.referenceleaktests.TestInstrumentation")]
public class TestInstrumentation : Instrumentation
{
protected TestInstrumentation (IntPtr handle, JniHandleOwnership ownership)
: base (handle, ownership)
{
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an instrumentation class that can be shared instead of making a new one:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky JNI reference/peer-count assertions in shared device test process

4 participants