[TrimmableTypeMap] fix JavaPeerableProxy activation gref leak#11116
Closed
simonrozsival wants to merge 8 commits intomainfrom
Closed
[TrimmableTypeMap] fix JavaPeerableProxy activation gref leak#11116simonrozsival wants to merge 8 commits intomainfrom
simonrozsival wants to merge 8 commits intomainfrom
Conversation
simonrozsival
force-pushed
the
dev/simonrozsival/11101-trimmable-typemap-followup
branch
from
007428d to
ab6d8da
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a remaining JNI global reference leak in the NativeAOT / trimmable typemap activation path by ensuring activated peers are constructed (and marked) before invoking inherited activation constructors, and adds regression coverage for both runtime and generator behavior.
Changes:
- Add
JavaPeerProxy.ConstructActivatedPeer()helper for typemap-generated proxy activation. - Update typemap proxy IL emission to pre-construct activated peers before calling inherited activation constructors/UCO constructors.
- Add regression tests: device-side gref leak test during inflation, and generator-side metadata coverage for the new helper reference.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Mono.Android-Tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs | Adds a device regression test to detect global-ref growth across repeated inflations. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs | Extends generator tests to ensure the new helper is referenced in generated assemblies. |
| src/Mono.Android/Java.Interop/TypeManager.cs | Updates Activate() to construct the peer via ValueManager.ConstructPeer() using the shared uninitialized-object helper. |
| src/Mono.Android/Java.Interop/JavaPeerProxy.cs | Introduces ConstructActivatedPeer() to centralize “pre-construct activated peer” logic for proxies. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs | Emits calls to ConstructActivatedPeer() for inherited activation ctor/UCO paths to prevent duplicate gref creation. |
simonrozsival
changed the title
simonrozsival
changed the title
simonrozsival
added
copilot
simonrozsival
changed the base branch from
dev/peppers/fix-activate-gref-leak
to
main
Fixes: #11101 Fixes: #10989 Context: 5c23bcda8 (PR #9640) The Java.Interop Unification (5c23bcd) changed `Object` to extend `JavaObject`, introducing an additional `ConstructPeer` call in the constructor chain. `TypeManager.Activate` was updated to use `SetPeerReference` but never set the `Activatable` state flag. Without it, each `ConstructPeer` call in the constructor chain (`JavaObject()` and `Object.SetHandle()`) creates a new JNI global ref, overwriting the previous one without deleting it — leaking 3 global refs per `LayoutInflater.Inflate` call. Before the fix, the new test fails with: Global reference leak detected: 30 extra global refs after inflating/GC'ing 10 custom views. Before=207, After=237 This went unnoticed because the `Activate` path is only triggered when Java creates .NET objects (not the other way around). The two main scenarios are Activity recreation and custom C# views in Android XML layouts. Most developers use .NET MAUI, which has a single Activity and does not use custom C# views in Android layout XML files, so neither scenario was commonly hit. Changes: - Promote `GetUninitializedObject` from a local function in `CreateProxy` to a shared static method. It sets `Activatable | Replaceable`, which tells `ConstructPeer` to return early and not create duplicate global refs. - Have `Activate` call `GetUninitializedObject` then `ConstructPeer` to create one global ref while `jobject` is still a valid JNI local ref. The `Activatable` flag then prevents duplicates during the constructor chain. - Add regression test that inflates custom views and asserts JNI global reference count does not grow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…23556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
force-pushed
the
dev/simonrozsival/11101-trimmable-typemap-followup
branch
from
125e014 to
19c0bc7
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #11112.
This PR keeps the trimmable typemap side of the remaining activation leak fix, scoped to generated proxy activation:
JavaPeerProxy.CreateActivatedPeer(Type, IntPtr)as the shared helper for inherited Java activation pathsTypeinstead of relying on a trim suppressionTypeMapAssemblyGeneratorTestscoverage