[TrimmableTypeMap] Fix UCO constructor activations by simonrozsival · Pull Request #11094 · dotnet/android

simonrozsival · 2026-04-09T07:40:13Z

Summary

This PR fixes the UCO constructor activation parity regression for trimmable typemaps in one review.

What it does

keep the JniEnvironment.WithinNewObjectScope guard in generated nctor_*_uco wrappers to avoid double peer creation from managed construction paths
remove the reflection-based ActivatePeerFromJavaConstructor() path entirely
inline activation directly in the generated wrappers, removing the TrimmableTypeMap.ActivateInstance() indirection
preserve the fully generated, non-reflective activation flow for both leaf and inherited activation ctors
keep the minimal generator plumbing needed by the emitted IL (PEAssemblyBuilder.EmitBody(..., useBranches: true)) and the RegisterNatives UTF-8 helper materialization

Non-goals

no reflection
no runtime fallback behavior
no unrelated trimmable typemap follow-up work

Notes

the previously split follow-up from [TrimmableTypeMap] Inline activation ctor in UCO wrappers #11089 has been folded into this PR to keep the review in one place
open generic UCO wrappers remain no-ops so native registration stays valid

Refactor UCO constructor wrappers to match legacy ManagedPeer.Construct activation behavior: - Add WithinNewObjectScope guard: skip activation when the object is being created from managed code (JNIEnv.StartCreateInstance/NewObject), preventing double peer creation and GC crashes. - For non-leaf types (activation ctor on a base type), call ActivatePeerFromJavaConstructor at runtime instead of inline IL. This finds the matching managed ctor for the JNI signature (e.g., parameterless for "()V") and invokes it via ActivatePeer, preserving user constructor side effects like field initialization. - For leaf types, inline the activation ctor call directly in the UCO with the WithinNewObjectScope guard. - Emit no-op UCO for open generic type definitions (type params unknown). - Add ControlFlowBuilder support to PEAssemblyBuilder.EmitBody for branch instructions (useBranches parameter). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- UCO constructors directly call activation ctor (no ActivateInstance indirection) - WithinNewObjectScope guard prevents double peer creation - No-op UCO for open generic type definitions - ControlFlowBuilder support in PEAssemblyBuilder - Remove TrimmableNativeRegistration wrapper and ActivateInstance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR updates the trimmable typemap generator/runtime path to match legacy UCO constructor activation behavior by guarding against double peer creation and inlining the activation IL in generated nctor_*_uco wrappers (removing the previous runtime helper entrypoint).

Changes:

Inline managed peer activation directly in generated UCO constructor wrappers, including a JniEnvironment.WithinNewObjectScope guard.
Remove TrimmableTypeMap.ActivateInstance() from the runtime path.
Extend PEAssemblyBuilder.EmitBody(...) to optionally enable branch/label emission (via ControlFlowBuilder) and add a generator test for the new behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs	Adds a regression test asserting UCO constructors reference `WithinNewObjectScope` + inlined activation APIs and do not reference removed helpers.
src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMap.cs	Removes the now-unused `ActivateInstance` runtime helper.
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs	Switches UCO constructor wrapper emission to guarded, inlined activation and adjusts supporting references.
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/PEAssemblyBuilder.cs	Adds a `useBranches` option to allow emitting branching IL via `ControlFlowBuilder`; adjusts UTF-8 helper nested type visibility.
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/Model/TypeMapAssemblyData.cs	Updates model documentation to reflect the new non-helper activation behavior.

Copilot · 2026-04-09T16:15:46Z

src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs

 				encoder.OpCode (ILOpCode.Ret);
 			});
+		metadata.AddCustomAttribute (typeDefHandle, ctorHandle, _ucoAttrBlobHandle);


This adds a custom attribute to the proxy type using ctorHandle (the proxy’s own .ctor MethodDef) as the attribute constructor. That produces invalid metadata (the ctor handle must point to the attribute’s ctor, e.g. _ucoAttrCtorRef), and UnmanagedCallersOnlyAttribute shouldn’t be applied to the proxy type/instance ctor anyway. Remove this AddCustomAttribute call (UCO should only be applied via AddUnmanagedCallersOnlyAttribute() on the generated wrapper methods).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

simonrozsival

🤖 AI Review Summary

Verdict: ⚠️ Needs Changes

Found 1 follow-up item:

💡 Testing: the new UTF-8 helper pre-materialization is a subtle metadata-ordering workaround and would benefit from more targeted regression coverage (src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs:348).

The main UCO activation change looks consistent with the intended non-reflective design. Public CI is green, but the internal Xamarin.Android-PR pipeline is still queued, so this is not ready for LGTM yet.

Review generated by android-reviewer from review guidelines.

simonrozsival · 2026-04-09T20:34:07Z

src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs

 	void EmitProxyType (JavaPeerProxyData proxy, Dictionary<string, MethodDefinitionHandle> wrapperHandles)
 	{
+		if (proxy.IsAcw) {
+			// RegisterNatives uses RVA-backed UTF-8 fields under <PrivateImplementationDetails>.


🤖 💡 Testing — this eager GetOrAddUtf8Field() loop is fixing a very subtle metadata-ordering problem in the generated assembly. Could we strengthen Generate_AcwProxy_HasRegisterNativesAndUcoMethods() to assert that RegisterNatives is owned by the proxy type, not just present somewhere in MethodDefinitions? That would lock in the exact behavior this workaround depends on and make future cleanup safer.

{Rule: Missing regression tests for subtle generator fixes}

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

simonrozsival mentioned this pull request Apr 9, 2026

[TrimmableTypeMap] Resolve all TrimmableIgnore, SSL, and Java.Interop-Tests exclusions #11099

Open

4 tasks

simonrozsival added trimmable-type-map copilot `copilot-cli` or other AIs were used to author this labels Apr 9, 2026

[TrimmableTypeMap] Remove UCO reflection fallback

cfbf866

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

simonrozsival mentioned this pull request Apr 9, 2026

[TrimmableTypeMap] Inline activation ctor in UCO wrappers #11089

Closed

simonrozsival marked this pull request as ready for review April 9, 2026 16:10

simonrozsival requested a review from jonathanpeppers as a code owner April 9, 2026 16:10

Copilot AI review requested due to automatic review settings April 9, 2026 16:10

Copilot started reviewing on behalf of simonrozsival April 9, 2026 16:11 View session

simonrozsival changed the title ~~[TrimmableTypeMap] Fix UCO constructor activation parity with legacy~~ [TrimmableTypeMap] Fix UCO constructor activations Apr 9, 2026

Copilot AI reviewed Apr 9, 2026

View reviewed changes

[TrimmableTypeMap] Remove invalid proxy attribute metadata

e620465

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

simonrozsival commented Apr 9, 2026

View reviewed changes

[TrimmableTypeMap] Strengthen RegisterNatives regression test

145d2d9

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

simonrozsival merged commit 2d453fc into main Apr 10, 2026
6 checks passed

simonrozsival deleted the dev/simonrozsival/fix-uco-activation-parity branch April 10, 2026 07:34

simonrozsival mentioned this pull request Apr 10, 2026

[TrimmableTypeMap] Runtime-only trimmable typemap support #11090

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TrimmableTypeMap] Fix UCO constructor activations#11094

[TrimmableTypeMap] Fix UCO constructor activations#11094
simonrozsival merged 5 commits intomainfrom
dev/simonrozsival/fix-uco-activation-parity

simonrozsival commented Apr 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

simonrozsival left a comment

Uh oh!

simonrozsival Apr 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

simonrozsival commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What it does

Non-goals

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

simonrozsival left a comment

Choose a reason for hiding this comment

🤖 AI Review Summary

Uh oh!

simonrozsival Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

simonrozsival commented Apr 9, 2026 •

edited

Loading