dotnet / dotnet/android

Investigate JNI global-reference leaks in cache ownership, activation cleanup, and callback retention

Open
#12,760 0 comments 0 reactions 0 assignees View on GitHub
Area: App Runtime needs-triage
Dominant language
C#
Stars
2.1k
Forks
579
Avg merge
1d 20h
Merged PRs (30d)
257

Description

### Android framework version

net11.0-android (Preview)

### Affected platform version

Source audit of `dotnet/android` at [71c0b4947d969790159483da51a809c892659d7f](https://github.com/dotnet/android/commit/71c0b4947d969790159483da51a809c892659d7f), including vendored Java.Interop. Earlier release applicability has not been established.

### Description

A read-only source audit identified seven JNI global-reference ownership/retention paths. This issue preserves their allocation-to-cleanup traces and proposed follow-up coverage.

**Evidence level:** these are source-traced findings, not device-reproduced leak measurements. No runtime reproduction or regression tests were run during this audit. In particular, these findings must not be treated as proof of the cause of an existing customer report.

The distinctions matter: some paths permanently abandon raw handles; others retain unnecessary class references until runtime disposal; one retains callbacks while their delegates remain rooted; and one loses only a single slot at startup.

#### 1. Redirected-method caches abandon owned class grefs

Sources: [instance-method cache/factory](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs#L98-L119), [static-method cache/fallback](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs#L29-L88), [instance-cache teardown](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs#L48-L55), [static-cache teardown](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniStaticMethods.cs#L22-L25).

The `ConcurrentDictionary.GetOrAdd` factories can allocate a `JniMethodInfo.StaticRedirect` containing a gref-owning `JniType`. Concurrent first lookup can execute multiple factories, but unpublished results are discarded without disposing their redirects. Teardown also clears published method entries without disposing the redirects.

These redirect types are neither finalizable nor registered with the runtime. Abandoned handles therefore cannot be recovered by runtime disposal.

**Trigger/impact:** successful instance-to-static JNI remapping or static-method fallback, not ordinary nonredirected methods. One permanent class gref per discarded redirect. Growth stops after warm-up for fixed caches, but recreated/disposed caches can accumulate indefinitely.

**Fix direction:** explicitly dispose unpublished candidates and cached redirects, preserving construction/publication and reentrancy semantics.

#### 2. Subclass-constructor cache races retain orphaned registered class grefs

Source: [JniInstanceMethods construction and GetConstructorsForType](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs#L17-L94).

`GetConstructorsForType` uses a `GetOrAdd` factory whose constructor allocates a `JniType` and immediately registers it with the runtime. Two concurrent first lookups for the same subclass can register two types while only one candidate enters the cache. The losing candidate is never disposed, and its type remains in `JniRuntime.TrackedInstances`.

**Trigger/impact:** generated constructors reach this through `StartCreateInstance` when `GetType()` differs from the declaring binding type. Extra class grefs persist until runtime teardown, normally process lifetime on Android. Member-cache disposal only finds winning entries. This is per first-use race/cache lifecycle, not per warmed-up constructor call.

**Fix direction:** publish explicitly constructed candidates and dispose losers. A simple `Lazy` substitution needs care because recursive lookup during construction is supported.

#### 3. Member disposal skips independently populated subclass caches

Sources: [JniPeerMembers.Dispose](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.cs#L137-L149), [independent subclass construction](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs#L129-L144).

Disposal returns when the owner's `jniPeerType` is null, but subclass constructor lookups can initialize their own `JniType` without initializing that field. Constructing only a derived type through base members, then correctly disposing those members, can therefore skip populated child caches.

**Trigger/impact:** explicit `JniPeerMembers` cache disposal, not ordinary `Java.Lang.Object.Dispose()`. References survive until runtime teardown; repeated member-cache creation/disposal can accumulate them without a race.

**Fix direction:** always dispose child caches when disposing; make only disposal of the owner's class reference conditional.

#### 4. Failed GetObject activation leaks the transferred input global reference

Source: [Java.Lang.Object.GetObject](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/src/Mono.Android/Java.Lang/Object.cs#L160-L171).

`GetObject(handle, TransferGlobalRef, type)` calls `ValueManager.GetPeer` before `JNIEnv.DeleteRef`. A missing or throwing activation constructor bypasses that cleanup. The value manager only borrows the input (`Copy` / `DoNotTransfer`); a separately created peer reference does not adopt the original handle. Finalization or bridge reclamation of that peer therefore does not release the transferred input.

**Trigger/impact:** one permanently lost global per failed transferred-handle conversion; repeated handled failures can accumulate indefinitely.

**Fix direction:** release the transferred input in `finally`. The similar success-only input cleanup in `Object.SetHandle` and `Throwable.SetHandle` should be considered when addressing this ownership pattern.

#### 5. Canceled callbacks can remain rooted in the runnable cache

Sources: [RunnableImplementor cache and Run cleanup](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/src/Mono.Android/Java.Lang/Thread.cs#L20-L54), [token-based Action posting](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/src/Mono.Android/Android.OS/Handler.cs#L44-L50).

Action-based posting stores the runnable in a static `ConditionalWeakTable`. Native `RemoveCallbacksAndMessages(token/null)` can remove queued work without removing that entry or disposing the runnable. If the Action remains independently rooted, the cached runnable and its gref remain alive after cancellation. A throwing Action also bypasses the cleanup after `Handler()` when the exception is handled and the process continues.

**Trigger/impact:** conditional retention, not an irretrievably lost raw handle. One stale runnable per retained Action; repeated use of one key is not inherently unbounded growth.

**Fix direction:** cancellation-aware cache ownership and exception-safe `Run` cleanup. Preserve the weak-key semantics.

#### 6. Startup loses an unused java.lang.Class global reference

Sources: [Mono initialization](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/src/native/mono/monodroid/monodroid-glue.cc#L839-L840), [CoreCLR initialization](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/src/native/clr/host/host.cc#L495-L496), [unused managed field](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/src/Mono.Android/Android.Runtime/JNIEnvInit.cs#L25).

Initialization obtains `init.grefClass` with `make_gref=true`. Managed initialization neither retains nor deletes it, so the native stack-local argument structure ultimately loses the handle. The cached method ID is not ownership of the reference.

**Trigger/impact:** exactly one strong-global slot per normal MonoVM/CoreCLR process startup; not NativeAOT. It references a bootstrap class, so this does not explain ongoing application-object growth. The raw allocation bypasses managed gref counters.

**Fix direction:** use a temporary local class reference for `GetMethodID`; any removal of the unused argument field must keep native/managed layouts synchronized.

#### 7. Empty/failed standalone JavaInterop1 registration abandons a class gref

Sources: [ManagedPeer.RegisterNativeMembers](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/external/Java.Interop/src/Java.Interop/Java.Interop/ManagedPeer.cs#L283-L307), [conditional native registration](https://github.com/dotnet/android/blob/71c0b4947d969790159483da51a809c892659d7f/external/Java.Interop/src/Java.Interop/Java.Interop/JniRuntime.ReflectionJniTypeManager.cs#L419-L427).

Registration allocates a `JniType` without cleanup, assuming registration adopts it. Empty registrations skip `RegisterNativeMethods`, leaving the type untracked and its gref abandoned. Exceptions before ownership transfer have the same outcome.

**Trigger/impact:** standalone/JavaInterop1 `ManagedPeer` registration, **not ordinary Android callable wrappers using `mono.android.Runtime.register`**. One permanent gref per empty/failed registration. Included because this implementation is vendored here.

**Fix direction:** dispose the temporary type unless ownership is successfully adopted; do not unconditionally dispose successful delegate registrations.

#### Existing issues and exclusions

Searched open and closed issues in `dotnet/android` and `dotnet/java-interop`, including exact cache/ownership symbols. No existing issue covering these specific paths was found. Related reports:

- #12516 tracks retained Android views/activities. These audit findings have not been connected to its reproductions.
- #12253 discusses activity-switching memory growth and GC scheduling. Reclaimable peers waiting for a collection are distinct from abandoned handles.
- #7324 concerns constructor exception propagation, rather than cleanup of a transferred input gref.
- Closed #11854 concerns unused interop code, rather than these ownership paths.

Intentional process-lifetime caches, local-reference leaks, and ordinary finalizer-reclaimable temporary wrappers were excluded. No unbounded native GC-bridge or weak-global-table leak was established. The optional native-tracing initialization race lacks a demonstrated live in-tree caller. Constructor-failure `SuppressFinalize` alone was not counted because CoreCLR can reclaim successfully registered peers through the GC bridge independently of finalization.

### Steps to Reproduce

The following are **proposed focused reproduction/regression cases**, not executed results:

1. For finding 1, arrange concurrent cold lookup of a successfully redirected instance method or static fallback, release all callers, and dispose the member cache. Account for every redirect gref, including unpublished candidates. Existing `JniPeerMembersTests.ReplaceInstanceMethodWithStaticMethod` provides a starting fixture.
2. For finding 2, concurrently perform first constructor lookup for the same managed subclass through one base member cache. Dispose the cache and inspect remaining tracked class references before runtime teardown.
3. For finding 3, create base members, call `StartCreateInstance` only for a derived type without accessing the base members' own `JniPeerType`, release the returned local, and dispose the members. Child-cache globals should be released immediately rather than at runtime shutdown.
4. For finding 4, pass a fresh global into `GetObject(..., TransferGlobalRef)` with no compatible existing peer and a missing/throwing activation constructor. Catch the failure and establish whether the original global was deleted; repeat after warming unrelated caches.
5. For finding 5, keep an Action independently rooted, post it at a future time with a token, cancel via `RemoveCallbacksAndMessages(token)`, and drop other runnable/handler references. Inspect the retained runnable while keeping the Action alive as an explicit control. Also cover an Action exception handled without process termination.
6. For finding 6, trace the startup `NewGlobalRef`/`DeleteGlobalRef` pair for `init.grefClass`; managed gref counters alone do not cover this raw allocation.
7. For finding 7, invoke the standalone `ManagedPeer` registration path with zero registrations and with an error before adoption. The existing `GetThis.java` fixture contains an empty registration example. Confirm temporary class globals are released in both cases.

Use isolated ownership assertions or isolated processes rather than assuming the shared device test process has a stable absolute gref count; see #12031. Distinguish immediate disposal, ordinary GC reclamation, and runtime-shutdown cleanup in assertions.

### Did you find any workaround?

No general workaround was established. The fix directions above are proposals, not implemented changes. Forced GC cannot recover an abandoned untracked raw global handle. For the callback case, Action-specific `RemoveCallbacks(action)` follows the managed removal/disposal path, unlike token-wide native cancellation; this is not a general replacement for token-wide semantics.

### Relevant log output

N/A. This investigation was a source audit; no device logs, measured leak rates, or executable reproduction are attached.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the linked JniPeerMembers cache paths, Java.Lang.Object.GetObject, Thread.cs, Handler.cs, the native initialization files, and ManagedPeer.RegisterNativeMembers. Review the proposed focused reproduction cases and existing ReplaceInstanceMethodWithStaticMethod fixture before tracing ownership. Done means each affected global reference is released or intentionally retained across success, failure, races, cancellation, and disposal, with regression coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
mobile-dev
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.