Skip to content

[TrimmableTypeMap] Fix UCO activation bugs: ctor memberref and duplicate peer creation#11177

Closed
simonrozsival wants to merge 4 commits intodev/simonrozsival/trimmable-test-plumbingfrom
dev/simonrozsival/trimmable-typemap-fixes
Closed

[TrimmableTypeMap] Fix UCO activation bugs: ctor memberref and duplicate peer creation#11177
simonrozsival wants to merge 4 commits intodev/simonrozsival/trimmable-test-plumbingfrom
dev/simonrozsival/trimmable-typemap-fixes

Conversation

@simonrozsival
Copy link
Copy Markdown
Member

@simonrozsival simonrozsival commented Apr 22, 2026

Summary

Fixes two bugs in the trimmable typemap UCO constructor callbacks that caused JavaObjectTest SIGABRT crashes on device.

Part of #10788

Bug 1: JniObjectReference ctor memberref (1-arg → 2-arg)

The IL generator emitted a memberref for JniObjectReference..ctor(IntPtr) but the actual CLR method is JniObjectReference..ctor(IntPtr, JniObjectReferenceType) — default parameters are C# compiler sugar, not CLR metadata. At runtime this threw MissingMethodException.

Fix: Changed Parameters(1, ...) to Parameters(2, ...) in the memberref signature and added ldc.i4.0 (JniObjectReferenceType.Invalid) at all 3 call sites.

Bug 2: UCO activation creating duplicate managed peers

The nctor_0_uco callback only checked JniEnvironment.WithinNewObjectScope to decide whether to skip activation. However, StartCreateInstance/FinishCreateInstance uses AllocObject + CallNonvirtualVoidMethod which does not set WithinNewObjectScope. This caused the UCO to create a second managed instance via GetUninitializedObject with all-null fields. When GC finalized this ghost instance, a null field access crashed the finalizer thread (SIGABRT).

Fix: Added JavaPeerProxy.ShouldSkipActivation(IntPtr) that calls PeekPeer to check if a managed peer already exists for the JNI handle (mirroring ManagedPeer.Construct's logic in external/Java.Interop/src/Java.Interop/Java.Interop/ManagedPeer.cs:80-88). The UCO now calls this after the WithinNewObjectScope check.

Both guards are needed because they cover different creation patterns:

  • WithinNewObjectScope — covers JniType.NewObject() where the callback fires before any peer is registered
  • ShouldSkipActivation — covers AllocObject + CallNonvirtualVoidMethod where the peer is already registered but the scope flag is not set

Test results

Device tests (CoreCLR + trimmable typemap): 855 passed, 4 failed, 58 skipped, 0 crashes

  • JavaObjectTest.Dispose ✅ (was SIGABRT)
  • JavaObjectTest.Dispose_Finalized ✅ (was SIGABRT)

Files changed

  • src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs — Fixed ctor memberref + added ShouldSkipActivation call
  • src/Mono.Android/Java.Interop/JavaPeerProxy.cs — Added ShouldSkipActivation(IntPtr) static method
  • tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.RuntimeTests/NUnitInstrumentation.cs — Removed JavaObjectTest exclusions

Follow-up

simonrozsival and others added 3 commits April 21, 2026 23:41
@simonrozsival
Fix trimmable typemap: JniTypeSignature scanner, build bugs, cross-as…
1511232 
…sembly aliases, proxy type map

Scanner:
- Parse [JniTypeSignature] attribute in addition to [Register]
- Track IsFromJniTypeSignature flag on JavaPeerInfo

Build fixes:
- Skip _GenerateTrimmableTypeMap in inner per-RID builds to prevent
  overwriting correct deferred JCW registrations
- Fix _ShrunkAssemblies item group to include typemap DLLs in assembly
  store count

Cross-assembly alias merge:
- MergeCrossAssemblyAliases() resolves collisions where types from
  different assemblies map to the same JNI name (e.g. JavaObject from
  Java.Interop and Java.Lang.Object from Mono.Android both claim
  java/lang/Object). [Register] types take precedence.

Proxy type map fix:
- Emit TypeMapAssociation attributes for ALL entries with proxies, not
  just alias groups — the runtime populates _proxyTypeMap from
  TypeMapAssociation, not from TypeMap's 3rd argument
- Force all TypeMap entries to 2-arg (unconditional) as workaround for
  dotnet/runtime#127004 where the trimmer strips TypeMapAssociation
  when a 3-arg TypeMap references the same type. Controlled by
  ForceUnconditionalEntries constant for easy revert.

Test exclusions:
- Exclude JnienvTest class (SIGSEGV crashes in threading/JNI tests)
- Update test expectations for ForceUnconditionalEntries workaround

Device test results: 816 passed, 7 failed, 94 skipped (917 total)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival
Minimize trimmable typemap test exclusions
094a4e2 
Replace 10 broad fixture-level exclusions with 21 specific test-level
exclusions. Use fixture-level exclusions only where 0 tests pass
(JavaObjectTest, InvokeVirtualFromConstructorTests,
JavaObjectArray_object_ContractTest). Use specific test exclusions
for fixtures where some tests pass (JniPeerMembersTests 4/8,
JniTypeManagerTests 3/6, JniValueMarshaler_object 6/13,
JavaPeerableExtensions 1/4).

Add [Category("TrimmableIgnore")] to InflateCustomView_ShouldNotLeakGlobalRefs
which has a genuine gref leak under trimmable typemap.

Device test results: 844 passed, 3 failed, 70 skipped (917 total)
The 3 failures are known trimmable typemap limitations tracked in #11170.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival
Fix UCO activation creating duplicate managed peers
4eb6644 
The UCO constructor callback (nctor_0_uco) only checked
WithinNewObjectScope to skip activation. However, the
StartCreateInstance/FinishCreateInstance pattern uses
AllocObject + CallNonvirtualVoidMethod, which does NOT set
WithinNewObjectScope. This caused the UCO to create a second
managed instance via GetUninitializedObject with null fields.
When GC later finalized this ghost instance, the null field
access crashed the finalizer thread (SIGABRT).

Fix: Add JavaPeerProxy.ShouldSkipActivation(IntPtr) that checks
if a managed peer already exists for the JNI handle (mirroring
ManagedPeer's PeekPeer logic). The UCO now calls this after the
WithinNewObjectScope check.

Also fixes the JniObjectReference ctor memberref from 1-arg to
2-arg (default params are C# sugar, not CLR). The constructor
is JniObjectReference(IntPtr, JniObjectReferenceType) — both
params must be in the memberref signature.

Device test results: 855 passed, 4 failed, 58 skipped (917 total).
JavaObjectTest.Dispose and Dispose_Finalized now pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival simonrozsival added copilot `copilot-cli` or other AIs were used to author this trimmable-type-map labels Apr 22, 2026
@simonrozsival simonrozsival mentioned this pull request Apr 22, 2026
Closed
@simonrozsival
Update CoreCLRTrimmable test exclusions with proper documentation
b29e12c 
- Re-add [Category("TrimmableIgnore")] for 7 tests in our code
- Re-add ExcludedTestNames for 40 external Java.Interop-Tests
- Add 3 newly-discovered failures (ObjectTest, DisposeAccessesThis, IJavaPeerable marshaler)
- Remove exclusion for InflateCustomView_ShouldNotLeakGlobalRefs (now passes)
- Every exclusion has a comment documenting the failure and links to #11170

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival added a commit that referenced this pull request Apr 24, 2026
@simonrozsival
Apply UCO activation fixes from PR #11177
f743229 
- Fix JniObjectReference ctor to use 2-arg overload (IntPtr, JniObjectReferenceType)
  since default parameters are C# sugar only and don't exist at IL level
- Add JavaPeerProxy.ShouldSkipActivation(IntPtr) to detect existing managed peers
  and prevent duplicate peer creation during StartCreateInstance/FinishCreateInstance
- Replace WithinNewObjectScope check with ShouldSkipActivation in UCO nctor callbacks
- Simplify DotNetNewAndroidTest (remove runtime parameter, CoreCLR is now default)
- Remove ObjectReferenceLogging test case and related DynamicDependency attributes
- Remove _AndroidEnableObjectReferenceLogging property from test csproj

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

Assignees

No one assigned

Labels

copilot `copilot-cli` or other AIs were used to author this trimmable-type-map

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@simonrozsival