dotnet / dotnet/android

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

Open
#12,031 2 comments 0 reactions 0 assignees View on GitHub
flaky-ci needs-triage
Dominant language
C#
Stars
2.1k
Forks
579
Avg merge
1d 20h
Merged PRs (30d)
257

Description

Exact process-wide JNI reference and peer-count assertions in the shared on-device test process are failing intermittently.

Although NUnit test execution is serialized (`NumberOfTestWorkers=0`), runtime and test-infrastructure work can still run on background threads and create persistent references or peers between the before/after snapshots. A changed process-wide count therefore does not necessarily identify a leak in the operation under test.

## Affected tests

### `TryFindClass_Utf8_DoesNotLeakGlobalRefs`

Example build:
https://dev.azure.com/dnceng-public/public/_build/results?buildId=1498907&view=results

```text
failed TryFindClass_Utf8_DoesNotLeakGlobalRefs
TryFindClass for non-existent classes should not leak global references
Expected: 492
But was: 546
```

Source:
`external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniTypeUtf8Test.cs`

Temporarily disabled by #12032.

### `UnregisterFromRuntime`

Example build:
https://dev.azure.com/dnceng-public/public/_build/results?buildId=1504514&view=results

```text
failed UnregisterFromRuntime
registeredCount+1 should match!
Expected: 68
But was: 71
```

Source:
`external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaObjectTest.cs`

This test counts surfaced peers rather than GREFs directly, but has the same isolation problem: it assumes no unrelated process activity changes a shared runtime-wide count while the assertion runs.

## Codebase audit candidates

### Enabled exact process-wide assertions — high risk

- [ ] `JniTypeUtf8Test.TryFindClass_String_DoesNotLeakGlobalRefs`
- Exact before/after `GlobalReferenceCount` equality.
- `external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniTypeUtf8Test.cs`
- [ ] `JniEnvironmentTests.References_CreatedReference_GlobalRef`
- Exact `GlobalReferenceCount` baseline around direct JNI global-reference calls.
- `external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniEnvironmentTests.cs`
- [ ] `JavaObjectArray_object_ContractTest.EndCheckGlobalRefCount`
- Fixture-wide GREF baseline across many tests. Ambient additions can create false positives, while ambient deletions can mask a real leak.
- `external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JavaObjectArrayTest.cs`
- [ ] `JniRuntimeJniValueManagerContract.ConstructPeer_CalledMultipleTimes_ShouldNotLeakGlobalRefs`
- Exact process-wide GREF equality between two `ConstructPeer` calls.
- `external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs`
- [ ] `BindingTests.JavaSideActivation`
- Exact before/after GREF equality in the JcwGen on-device test app.
- `tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs`

### Runtime-global surfaced-peer assertions — medium risk

The Android value-manager contract installs a fresh value manager, which reduces stale state, but installs it globally. Background runtime activity can therefore still perturb exact peer totals.

- [ ] `JniRuntimeJniValueManagerContract.AddPeer_NoDuplicates`
- [ ] `JniRuntimeJniValueManagerContract.ConstructPeer_ImplicitViaBindingConstructor_PeerIsInSurfacedPeers`
- [ ] `JniRuntimeJniValueManagerContract.ConstructPeer_ImplicitViaBindingMethod_PeerIsInSurfacedPeers`

Source:
`external/Java.Interop/tests/Java.Interop-Tests/Java.Interop/JniRuntimeJniValueManagerContract.cs`

These tests should prefer assertions about the specific peer under test (`PeekPeer`, identity, removal) rather than the total registry size.

### Already quarantined or noise-tolerant

- `TryFindClass_Utf8_DoesNotLeakGlobalRefs` — disabled by #12032.
- `UnregisterFromRuntime` — disabled by #12037.
- `InflateCustomView_ShouldNotLeakGlobalRefs` — disabled under #11201; it already amplifies the operation and allows ambient GREF noise.
- `DoNotLeakWeakReferences` — disabled under #11690; it already documents process-global peer-count drift and uses a tolerance.

### Lower-risk thread-local LREF assertions

The exact LREF checks in `JniEnvironmentTests`, `TestTypeTests`, and the `JavaArrayContract` fixture family use `JniEnvironment.LocalReferenceCount`, which is thread-local. The Android runner executes NUnit tests synchronously on one instrumentation thread, so background threads cannot change these counts.

They do not share the GREF failure mechanism, but the fixture-wide checks can still include test-runner activity on the instrumentation thread and should eventually move into the isolated harness with the other leak checks.

## Proposed direction

Reference and peer leak checks should run in a dedicated device test app/process rather than in the same test project as unrelated runtime tests. The dedicated harness should:

- start from a fresh process for each leak scenario or tightly controlled group;
- avoid unrelated tests and background test-runner activity;
- control ordering and concurrency explicitly;
- amplify the operation under test where useful, so a per-call leak is distinguishable from setup noise;
- perform any required GC/finalizer synchronization before checking cleanup.

This harness does not need to use NUnit if a simpler dedicated executable provides better lifecycle and concurrency control.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by reading the affected tests in JniTypeUtf8Test.cs, JavaObjectTest.cs, JniEnvironmentTests.cs, JavaObjectArrayTest.cs, JniRuntimeJniValueManagerContract.cs, and BindingTests.cs, then review the quarantined leak checks. Define the dedicated device test app or process and move selected leak scenarios into it with controlled ordering, synchronization, and focused assertions; done means ambient runtime activity no longer causes false results.

Written by the indexing model from the issue text.

Assessment

Tech stack
android, csharp
Domain
mobile-dev, testing-qa
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.