Repository navigation
[Java.Interop] Extend peer cleanup regression coverage - #13292
Open
simonrozsival wants to merge 1 commit into
Open
simonrozsival wants to merge 1 commit into
simonrozsival wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are identified for the peer-cleanup regression coverage and disposal-contract documentation changes.
No open issues 🎉
What changed in this PR
Adds regression coverage for existing Java.Interop peer-cleanup fixes related to #13290 and documents their disposal contract. Production behavior and release branches are unchanged.
Changes:
- Expands peer-lifetime coverage from 30 to 49 cases, including unregistered disposal, aliases, throwing callbacks, and reconstructed references.
- Adds the
Android.Graphics.Rectreproduction and separate JNI-reference deletion checks. - Documents unregistered-peer cleanup and preservation of another registered wrapper.
| File | Description |
|---|---|
| tests/Mono.Android-Tests/Java.Interop-Tests/Java.Interop/JavaPeerControlBlockTests.cs | Extends disposal, finalization, native-block, and JNI-reference regression coverage. |
| external/Java.Interop/src/Java.Interop/Documentation/Java.Interop/IJavaPeerable.xml | Clarifies the existing unregistered-peer disposal and cleanup contract. |
Review effort: Balanced
jonathanpeppers
pushed a commit
that referenced
this pull request
Oct 28, 2026
Backport only the unregistered-disposal correction from c663cd1 (#12772), without its peer-identity and activation changes. DisposePeerUnlessReferenced() bypasses Disposed()/Dispose(true) when it calls the private reference-deletion helper. JavaObject and JavaException then retain their native control blocks after the JNI reference is cleared and finalization is suppressed. Use the public DisposePeer(value) path so managed disposal precedes normal cleanup. Add a distinct JavaVMFixture-based fixture for this branch's host JVM and Android inclusion paths. Cover valid-reference callback entry, native-block release, registered no-ops, alias winners, throwing callbacks and retry, repeated disposal, exact Rect disposal, and separate warmed global-reference deletion checks. The source-built Release SDK passes 12 host-JVM tests and 9 ordinary plus 7 dedicated GREF tests on each of CoreCLR and NativeAOT. The original helper produces exactly 6 host and 7 device failures; restore the production correction before repeating positive validation. Complement the finalization backport in #13294 without duplicating its implementation or JavaPeerControlBlockTests fixture. Refs #13290, #13292, #13294 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 97270e0d-9fba-4a8b-8a04-9b7c25e1cd21
Context: #13290 Main already contains the production fixes in e5b44a4 and c663cd1. Add coverage and document the existing unregistered-disposal contract. Add 19 peer-lifetime cases for unregistered disposal and callbacks. Cover native-block release, alias ownership, exceptions, and JNI handles. Extend reconstruction checks to registered finalization callbacks. Source-built Release CoreCLR and NativeAOT each pass all 49 fixture cases. The old disposal bypass produces seven expected regression failures. Restore the correct production path and repeat positive validation. No production behavior changes or servicing backports are included. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f7772f6f-f6e3-4b4a-b9f2-5abe3f7fd82f
simonrozsival
force-pushed
the
simonrozsival-main-peer-cleanup-fix
branch
from
November 4, 2026 11:59
1e0e0ab to
8b99f07
Compare
simonrozsival
added
the
ready-to-review
This branch has not been deployed
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.
Refs #13290.
mainalready contains both production corrections relevant to this issue:e5b44a4186(#13016) releases invalid native control blocks after finalization callbacks, andc663cd110f(#12772) routes unregistered disposal through the normal managed-disposal path. This PR adds regression coverage and documents that existing contract; it does not change production behavior or service the affected release branches.Extend
JavaPeerControlBlockTestsfrom 30 to 49 cases:Java.Interop.JavaObjectandJava.Interop.JavaExceptionafter unregistration, including aliases whose registered wrapper must remain usable, registered-peer no-op disposal, callback entry with a valid JNI reference, repeated disposal, and retry after a managed disposal callback throws.Android.Graphics.Rect.UnregisterFromRuntime()/DisposeUnlessReferenced()sequence from the issue, and check JNI-reference deletion separately in the dedicatedJniReferenceLeakrun.Document that unregistered peers receive normal managed disposal, release their JNI reference and native block before finalization is suppressed, and do not invalidate another registered wrapper for the same Java instance.
Local validation
Built the in-tree Release SDK and tested on an Android 15/API 35 ARM64 emulator. The source-pinned SDK is
12.0.100-alpha.1.26523.103; these tests targetnet11.0-android, using the source-built Android pack37.99.0and runtime packs11.0.0-preview.7.26381.103, not the installed RC2 Android workload.Both
make prepare CONFIGURATION=Releaseandmake leeroy CONFIGURATION=Release MSBUILD_ARGS='-v:minimal -m:4 -nr:false'passed.leeroyincludes the additional API-level bindings before local workload configuration.PeerControlBlockJniReferenceLeakPeerControlBlockJniReferenceLeakEach dedicated reference run includes six peer-control-block cases and two existing class-lookup checks. The actual TRX results contain the same 49 peer-lifetime case names on both runtimes: 102 passing results in total, including the four ancillary checks.
Negative control: Temporarily restoring only the old
DisposePeerUnlessReferenced()call to privateDisposePeer(h, value), then rebuilding the SDK and rerunningPeerControlBlock, produced exactly seven expected failures: two missing throwing callbacks, four missing managed-disposal callbacks, and the retained native-block pointer in theRectreproduction. The correct production source was restored byte-for-byte, the SDK rebuilt, and both CoreCLR categories passed again. No mutation remains in this PR.Exact successful build/install and test commands
Commands ran with isolated SDK, JDK, NDK, and NuGet directories;
AndroidNdkDirectorypoints to the prepared local NDK. All build/install commands below passed. The test outcomes are listed in the table above.SDK preparation and build:
make prepare CONFIGURATION=Release make leeroy CONFIGURATION=Release MSBUILD_ARGS='-v:minimal -m:4 -nr:false'CoreCLR lifetime cases:
CoreCLR dedicated reference cases:
NativeAOT lifetime cases:
NativeAOT dedicated reference cases:
The scoped clean passed and was needed because the first incremental NativeAOT category switch reran the ordinary category. That run was not counted as dedicated reference validation; the clean rebuild produced the verified eight-case result.
Coverage is limited to Android 15 ARM64 on the current source-built CoreCLR and NativeAOT implementations. These results do not validate .NET 10 servicing or a future .NET 11 backport.
Pull Request
title and
description
should follow the
commit-messages.mdworkflow documentation, and in particular should include: