[Tests] Define managed object proxy contract - #12612
Conversation
Cover plain managed object round trips through Java arrays and collections, including identity, nulls, nested containers, Java-visible object semantics, and GC lifetime across runtime configurations.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
State the shared round-trip contract separately from typemap-specific Java object methods. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/ManagedObjectProxyTests.cs — 💡 suggestion — AssertEventuallyCollected uses a fixed attempt count with Thread.Yield(), which can… |
What changed in this PR
Adds focused device tests to explicitly define and validate the managed object proxy round-trip contract (identity, equality semantics, and lifetime) across llvm-ir and trimmable typemap configurations, without changing production runtime behavior.
Changes:
- Introduces
ManagedObjectProxyTestscoveringJavaObjectArray<object>,JavaList<object>, andJavaDictionary<object, object>round-trip identity/semantics and bounded GC/disposal behavior. - Asserts the intentional semantic split for Java-visible object methods between llvm-ir (managed forwarding) and trimmable typemap (Java identity semantics).
- Wires the new test file into the Mono.Android.NET device test project build.
| File | Description |
|---|---|
| tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj | Includes the new ManagedObjectProxy test fixture in the device test build. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/ManagedObjectProxyTests.cs | New device tests defining managed proxy identity/semantics/lifetime contract across typemap modes. |
Wait for an observed JNI bridge-processing generation when testing proxy lifetime, and derive the exact default Java object string from the runtime proxy class and unsigned identity hash. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Always perform a full collection and bridge synchronization cycle, while requiring bridge-generation advancement only on CoreCLR and NativeAOT. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use exception-safe JNI reference acquisition and collection disposal, and measure the bounded GC wait with a monotonic stopwatch. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep every JNI local reference exception-safe, retain caller ownership while constructing the unregistered Java object view, and create the lifetime-test target on a short-lived stack to avoid Mono false pinning. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Run weak-reference probes and retention assertions on short-lived stacks so Mono conservative scanning cannot retain stale strong target references between forced collections. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Reject
Findings: 1 error · 0 warnings · 0 suggestions.
The new tests provide useful coverage for proxy identity, JNI reference cleanup, and GC lifetime across typemap implementations. However, the Java collection equality assertions currently contradict the documented LLVM-IR behavior and will fail in that configuration; the inline comment identifies both affected tests.
CI build 1576501 was still in progress during review: 7 of 44 checks had passed, 36 were running, and 1 was queued, with no reported failures yet.
Generated by Android PR Reviewer for #12612 · gpt56 · 99.3 AIC · ⌖ 9.27 AIC · ⊞ 25.7K
Comment /review to run again
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
There was a problem hiding this comment.
Review summary
Verdict:
The typemap-specific identity, equality, hashing, string, nested-collection, and array-lifetime assertions match the runtime implementations I traced. I left one inline suggestion to extend lifetime coverage to the separate JavaList/JavaDictionary marshaling path.
Findings: 0 errors · 0 warnings · 1 suggestion
CI: Azure DevOps build 1576501 remains in progress; completed checks were successful when reviewed, with no known failure yet.
Generated by Android PR Reviewer for #12612 · gpt56 · 122.3 AIC · ⌖ 8.9 AIC · ⊞ 25.7K
Comment /review to run again
| } | ||
|
|
||
| [Test] | ||
| public async Task JavaObjectArrayRetainsManagedValueUntilReleased () |
There was a problem hiding this comment.
🤖 💡 Testing — This lifecycle test only exercises JavaObjectArray<object>, which marshals arbitrary managed values through CreateLocalObjectReferenceArgument(). The JavaList and JavaDictionary tests above use the distinct JavaConvert.ToLocalJniHandle() / Android.Runtime.JavaObject fallback, but only round-trip values while strong managed references still exist. Please consider adding the same retain-while-Java-rooted and collect-after-release coverage for that collection path (one list case plus a dictionary key/value case would cover it), so the proxy lifetime contract is tested for both implementations.
Rule: Test each distinct marshaling path
Context
Fixes #11703.
Plain managed objects already preserve their managed reference across Java round trips in both llvm-ir and trimmable typemap builds. The remaining gap was explicit contract coverage, especially because trimmable proxies intentionally use Java identity semantics instead of forwarding managed
Equals(),GetHashCode(), andToString().Changes
JavaObjectArray<object>,JavaList<object>, andJavaDictionary<object, object>round trips.Java.Lang.Objectviews, and bounded GC/disposal lifetime.Validation
llvm-ir CoreCLR Release: 9 passed, 0 failed, 0 skipped
trimmable CoreCLR Release: 9 passed, 0 failed, 0 skipped
trimmable NativeAOT Release: 9 passed, 0 failed, 0 skipped
Java.Interop legacy proxy host contract: 14 passed, 0 failed, 0 skipped
Useful description of why the change is necessary
Links to issues fixed
Unit tests