Skip to content

Split reflection-based JniTypeManager and JniValueManager behavior#1441

Merged
jonathanpeppers merged 30 commits into
mainfrom
dev/simonrozsival/fix-aot-warnings-jni-type-manager
Jun 9, 2026
Merged

Split reflection-based JniTypeManager and JniValueManager behavior#1441
jonathanpeppers merged 30 commits into
mainfrom
dev/simonrozsival/fix-aot-warnings-jni-type-manager

Conversation

@simonrozsival

@simonrozsival simonrozsival commented Jun 4, 2026

Copy link
Copy Markdown
Member

Splits the reflection-backed paths out of JniRuntime.JniTypeManager and JniRuntime.JniValueManager so callers that don't need reflection (e.g. NativeAOT) can opt out, while keeping the public API surface as compatible as possible for callers migrating from .NET 10.

What changed

  • JniRuntime.ReflectionJniTypeManager (new): all reflection-based type lookup, signature discovery, and native-method registration moves here. Annotated with [RequiresDynamicCode] / [RequiresUnreferencedCode].
  • JniRuntime.ReflectionJniValueManager (new): all reflection-based peer construction, value marshaling, and activation moves here.
  • JniRuntime.JniTypeManager stays a non-abstract base class. Methods that previously had reflection defaults are now virtual no-ops (or return null/empty). Subclasses that want the old default behavior should derive from ReflectionJniTypeManager.
  • JniRuntime.JniValueManager stays abstract (it already was). A few previously-virtual methods (CreatePeer, GetValueMarshalerCore(Type)) become abstract since their reflection-based defaults moved out — public consumers (dotnet/android's value managers) already override these.
  • In-tree consumers updated: JreTypeManager, ManagedValueManager, MonoRuntimeValueManager now derive from the new Reflection* bases. The NativeAOT samples keep deriving from the abstract bases with explicit type mappings.

Public API impact

PublicAPI.Shipped.txt is unchanged. All breaking changes are tracked in PublicAPI.Unshipped.txt with *REMOVED* markers:

  • JniTypeManager.TryRegisterNativeMembers (both overloads) — moved to ReflectionJniTypeManager as protected helpers.
  • JniValueManager.CreatePeer and GetValueMarshalerCore(Type)virtualabstract.

A non-abstract JniTypeManager keeps new JniRuntime.JniTypeManager() and direct subclasses compiling without changes.

Validation

  • CI is green.
  • Local dotnet build Java.Interop.sln passes (PublicApiAnalyzer enforces tracking is consistent).

@simonrozsival simonrozsival force-pushed the dev/simonrozsival/fix-aot-warnings-jni-type-manager branch from 5ff4c17 to 6d72140 Compare June 4, 2026 14:24
@simonrozsival simonrozsival force-pushed the dev/simonrozsival/fix-aot-warnings-jni-type-manager branch from 6d72140 to 58f0dbd Compare June 4, 2026 14:30
@simonrozsival

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
No pipelines are associated with this pull request.

@simonrozsival simonrozsival marked this pull request as ready for review June 8, 2026 11:55
Copilot AI review requested due to automatic review settings June 8, 2026 11:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This draft PR refactors JniRuntime.JniTypeManager into an abstract base and moves the default reflection-based behavior into a new JniRuntime.DynamicJniTypeManager, with corresponding updates across tests, JRE runtime helpers, samples, and public API baselines.

Changes:

  • Introduces JniRuntime.DynamicJniTypeManager and makes JniRuntime.JniTypeManager abstract, shifting default/reflection-backed logic into the new derived type.
  • Updates test and JRE type managers to inherit from DynamicJniTypeManager, and adjusts tests for new behaviors (e.g., generic invoker handling, GetType array behavior).
  • Updates trimmer/AOT annotations and PublicAPI baselines to reflect the split and new abstract members.
Show a summary per file
File Description
tests/Java.Interop-Tests/Java.Interop/JniTypeManagerTests.cs Updates expectations around GetType behavior (notably array signatures and generic holder mapping).
tests/Java.Interop-Tests/Java.Interop/JniRuntimeTest.cs Switches proxy manager to DynamicJniTypeManager and adds trim suppression for test-only reflection use.
tests/Java.Interop-Tests/Java.Interop/JniRuntime.JniTypeManagerTests.cs Adds a generic invoker negative test and updates test manager base type.
tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs Updates fixture type manager to DynamicJniTypeManager and refines type lookup with trimming annotations.
src/Java.Runtime.Environment/Java.Interop/JreTypeManager.cs Moves to DynamicJniTypeManager and adds a DAM-annotated GetTypeForSimpleReference override.
src/Java.Interop/PublicAPI.Unshipped.txt Records newly introduced/reshaped APIs (new type, newly-abstract members, overrides).
src/Java.Interop/PublicAPI.Shipped.txt Updates shipped API surface to reflect newly-abstract members and removed methods from JniTypeManager.
src/Java.Interop/Java.Interop/JniValueMarshaler.cs Simplifies expression-based marshaler instantiation and adjusts trimming annotations.
src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs Switches peer-type selection to GetReflectionConstructibleTypes() and refactors uninitialized peer creation.
src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs Makes JniTypeManager abstract and factors previously-inlined logic into abstract core methods.
src/Java.Interop/Java.Interop/JniRuntime.DynamicJniTypeManager.cs Adds the new reflection-backed type manager implementation and related registration behavior.
src/Java.Interop/Java.Interop/JniRuntime.cs Removes the default new JniTypeManager() fallback; requires an explicit type manager in creation options.
src/Java.Interop/Java.Interop/JavaPrimitiveArrays.tt Retargets primitive array helper generation to DynamicJniTypeManager.
src/Java.Interop/Java.Interop/JavaPrimitiveArrays.cs Updates the generated primitive array helper container type to DynamicJniTypeManager.
src/Java.Interop/GlobalSuppressions.cs Adds CA1034 suppression for the new nested DynamicJniTypeManager type.
samples/Hello-NativeAOTFromJNI/NativeAotTypeManager.cs Updates sample type manager implementation to new hierarchy and adds explicit native member registration.
samples/Hello-NativeAOTFromJNI/ManagedType.cs Changes the native registration hook to internal for sample-driven registration.
samples/Hello-NativeAOTFromAndroid/NativeAotTypeManager.cs Updates sample type manager implementation to new hierarchy and implements abstract members.

Copilot's findings

  • Files reviewed: 18/18 changed files
  • Comments generated: 3

Comment thread samples/Hello-NativeAOTFromJNI/NativeAotTypeManager.cs Outdated
Comment thread samples/Hello-NativeAOTFromAndroid/NativeAotTypeManager.cs Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival simonrozsival marked this pull request as draft June 8, 2026 14:24
simonrozsival and others added 9 commits June 8, 2026 16:32
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>
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>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Map Java.Interop's internal proxy peer types in the base type manager so custom AOT-safe managers do not need reflection-based DynamicJniTypeManager behavior for JavaProxyThrowable/JavaProxyObject.

Register built-in proxy native members by JNI type name before asking custom managers for a managed Type, avoiding DAM-annotated lookups of internal Exception/Delegate-derived types.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Let the base JniTypeManager resolve primitive JNI keyword signatures without requiring custom AOT-safe managers to duplicate the dynamic type manager's primitive mappings.

Use that base lookup as a fallback when matching Java-to-managed constructor signatures so signatures such as (I)V resolve to System.Int32.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival simonrozsival marked this pull request as ready for review June 9, 2026 07:23
simonrozsival and others added 2 commits June 9, 2026 09:31
Rename DynamicJniTypeManager to ReflectionJniTypeManager so the type name describes the reflection-based behavior that is incompatible with Native AOT.

Update the matching source file name, API baseline, JRE type manager inheritance, generated primitive-array partial, and tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival simonrozsival changed the title Split reflection-based JniTypeManager behavior Split reflection-based JniTypeManager and JniValueManager behavior Jun 9, 2026
…l-forgiving operator

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot AI commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

@copilot address review comments

Addressed all review comments in commit Address review comments: EnsureNotDisposed, spacing, Array.Empty, null-forgiving operator:

  1. EnsureNotDisposed() in GetValueCore<T> (ReflectionJniValueManager.cs:377): Added EnsureNotDisposed() before the !reference.IsValid check, consistent with the non-generic GetValueCore overload.
  2. Missing space in GetListType (ReflectionJniValueManager.cs:472): Fixed to GetListType (Type type) per Mono style.
  3. Array.Empty<Type>()[] (ReflectionJniTypeManager.cs:129): Updated EmptyTypeArray initialization.
  4. Null-forgiving ! → null check + throw (ReflectionJniTypeManager.cs:91): Replaced type.GetElementType ()! with type.GetElementType () ?? throw new InvalidOperationException ("Array type has no element type.").

@simonrozsival

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

Java.Interop PR Reviewer completed successfully!

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Review Summary — ✅ LGTM (minor suggestions)

The split of JniTypeManager → abstract base + ReflectionJniTypeManager and JniValueManager → abstract base + ReflectionJniValueManager is clean and well-executed. All issues flagged in the prior two reviews have been properly addressed in the latest commits.

Issues by severity

Severity Count
⚠️ Warning 1
💡 Suggestion 1

Positive callouts

  • All previously-flagged issues (runtime crash in GetObjectArrayMarshaler, TODO placeholders, EnsureNotDisposed() consistency, formatting, Array.Empty<T>(), null-forgiving in GetElementType) are resolved
  • The abstract base classes are AOT-safe with no reflection dependencies; [RequiresDynamicCode]/[RequiresUnreferencedCode] annotations correctly scoped to the reflection subclasses
  • ReflectionConstructibleType wrapper preserves [DynamicallyAccessedMembers] flow through the type hierarchy nicely
  • KnownArrayTypes lookup table is a solid AOT-safe alternative to MakeArrayType()/MakeGenericType()
  • NativeAOT sample managers demonstrate the intended inheritance pattern — implementing abstract methods with explicit mappings, no reflection
  • Public API baselines are comprehensive and consistent
  • JreRuntime.CreateJreVM(JniTypeManager) overload provides a clean API for AOT scenarios

CI Status

The dotnet.java-interop build is queued but hasn't completed yet. license/cla passed. Please confirm CI is green before merging.

Generated by Java.Interop PR Reviewer for issue #1441

Generated by Java.Interop PR Reviewer for issue #1441 · ● 14.3M

Comment thread samples/Hello-NativeAOTFromAndroid/NativeAotTypeManager.cs
Java.Runtime.Environment remains the standalone desktop/JRE runtime
path until #1447 removes it. Suppress the reflection-based value
manager trim/AOT diagnostics on its constructors so existing NativeAOT
samples can continue to publish while that cleanup is in flight.

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

Copy link
Copy Markdown
Member Author

@copilot fix test failure:

Tests: run Hello-NativeAOTFromJNI

[View raw log](https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_apis/build/builds/1456181/logs/81)

Starting: Tests: run Hello-NativeAOTFromJNI
==============================================================================
Task         : PowerShell
Description  : Run a PowerShell script on Linux, macOS, or Windows
Version      : 2.274.0
Author       : Microsoft Corporation
Help         : https://docs.microsoft.com/azure/devops/pipelines/tasks/utility/powershell
==============================================================================
Generating script.
========================== Starting Command Output ===========================
"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe" -NoLogo -NoProfile -NonInteractive -ExecutionPolicy Unrestricted -Command ". 'D:\a\_temp\17d5be39-eee5-4b16-844f-de695c101e3a.ps1'"
  Determining projects to restore...
  Restored D:\a\1\s\samples\Hello-NativeAOTFromJNI\Hello-NativeAOTFromJNI.csproj (in 203 ms).
  19 of 20 projects are up-to-date for restore.
  Hello from Java!
  C# init()
  Hello from .NET NativeAOT!
  String returned to Java: Hello from .NET NativeAOT!
  Unhandled exception. System.NotSupportedException: Could not find System.Type corresponding to Java type JniTypeSignature(TypeName=net/dot/jni/internal/JavaProxyThrowable ArrayRank=0 Keyword=False) .
     at Java.Interop.JniEnvironment.Types.TryLoadClassWithFallback(JniEnvironmentInfo, IntPtr, JniObjectReference, Boolean, JniObjectReference&) + 0x315
     at Java.Interop.JniEnvironment.Types.TryFindClass(String, Boolean) + 0x182
     at Java.Interop.JniPeerMembers.JniInstanceMethods..ctor(Type) + 0x132
     at Java.Interop.JniPeerMembers.JniInstanceMethods.GetConstructorsForType(Type) + 0x9b
     at Java.Interop.JniPeerMembers.JniInstanceMethods.StartCreateInstance(String, Type, JniArgumentValue*) + 0x1a
     at Java.Interop.JavaException..ctor(String message) + 0xb3
     at Java.Interop.JniEnvironment.Exceptions.Throw(Exception) + 0x44
     at Java.Interop.ManagedPeer.Construct(IntPtr, IntPtr, IntPtr, IntPtr, IntPtr) + 0x4f2
     at Hello-NativeAOTFromJNI!<BaseAddress>+0x12624e
D:\a\1\s\samples\Hello-NativeAOTFromJNI\Hello-NativeAOTFromJNI.targets(77,5): error MSB3073: The command ""C:\hostedtoolcache\windows\Java_Temurin-Hotspot_jdk\17.0.19-10\x64\bin\java.EXE" -classpath "hello-from-java.jar;java-interop.jar" net/dot/jni/hello/App" exited with code -1073740791. [D:\a\1\s\samples\Hello-NativeAOTFromJNI\Hello-NativeAOTFromJNI.csproj]

Build FAILED.

D:\a\1\s\samples\Hello-NativeAOTFromJNI\Hello-NativeAOTFromJNI.targets(77,5): error MSB3073: The command ""C:\hostedtoolcache\windows\Java_Temurin-Hotspot_jdk\17.0.19-10\x64\bin\java.EXE" -classpath "hello-from-java.jar;java-interop.jar" net/dot/jni/hello/App" exited with code -1073740791. [D:\a\1\s\samples\Hello-NativeAOTFromJNI\Hello-NativeAOTFromJNI.csproj]
    0 Warning(s)
    1 Error(s)

Time Elapsed 00:00:02.82
##[error]PowerShell exited with code '1'.
Finishing: Tests: run Hello-NativeAOTFromJNI

@simonrozsival

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

…etBuiltInType

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot AI commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

@copilot fix test failure:

Tests: run Hello-NativeAOTFromJNI

...

Fixed in commit Fix NativeAOT test failure: handle built-in types in JniTypeManager.GetBuiltInType.

Root cause: ManagedPeer.RegisterNativeMembers calls GetTypeFromSignature()typeManager.GetType(), but the NativeAOT sample's GetTypeForSimpleReference didn't know about net/dot/jni/internal/JavaProxyThrowable (an internal type in Java.Interop).

Changes:

  • Extended GetBuiltInType in the base JniTypeManager to handle JavaProxyObject.JniTypeName, JavaProxyThrowable.JniTypeName, and ManagedPeer.JniTypeName — these framework-internal types are now always resolved at the base class level, before the abstract GetTypeForSimpleReference is called.
  • Changed TryRegisterBuiltInNativeMembers from internal static to protected static so NativeAOT sample managers can call it.
  • Updated both NativeAOT samples (Hello-NativeAOTFromJNI and Hello-NativeAOTFromAndroid) to call TryRegisterBuiltInNativeMembers at the top of RegisterNativeMembers, so JavaProxyObject's native methods (equals/hashCode/toString) get registered correctly.
  • Added [UnconditionalSuppressMessage] for IL2026/IL2111 on GetBuiltInType — ILLink's MethodsConstructors DAM annotation caused it to analyze JavaProxyObject's nested delegate types whose constructors inherit Delegate.Delegate(Object, String) (marked RequiresUnreferencedCode). These are false positives: the delegate construction uses compile-time-known static method references, not string-based reflection.

Copilot AI and others added 2 commits June 9, 2026 16:22
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
… in base JniTypeManager

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot AI commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved the merge conflicts in commit Resolve merge conflicts: remove #if NET guards, keep abstract methods in base JniTypeManager.

The two new commits in main since the last merge (Remove Java.Interop.GenericMarshaler (#1450) and Remove legacy NET preprocessor paths (#1451)) conflicted with our branch in three files:

  • JniRuntime.JniTypeManager.cs (two conflicts): Kept the abstract method declarations (GetInvokerTypeCore, GetStaticMethodFallbackTypesCore, RegisterNativeMembers) from our branch while adopting the #if NET guard removals from main. The implementation helpers (TryRegisterNativeMembers, TryLoadJniMarshalMethods, etc.) remain in DynamicJniTypeManager where our PR moved them.
  • ManagedPeer.cs: Removed the #if NET guard that was wrapping the ReadOnlySpan<char> methods block.
  • JreRuntime.cs: Kept our branch's structure (TypeManager default set in the [RequiresDynamicCode]-annotated CreateJreVM, not in CreateJreVMCore), discarding main's addition of builder.TypeManager ??= new JreTypeManager(...) to CreateJreVMCore which would have bypassed the AOT warning.

Also removed the remaining #if NET guards from JniRuntime.ReflectionJniTypeManager.cs and JreRuntime.cs that were in our branch but no longer needed after the main merge.

@simonrozsival

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

Java.Interop PR Reviewer completed successfully!

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — Clean architectural split

Summary: This PR cleanly separates the reflection-based JniTypeManager / JniValueManager behavior into dedicated ReflectionJniTypeManager and ReflectionJniValueManager subclasses, making the base classes abstract and properly annotating the reflection-based paths with [RequiresDynamicCode] / [RequiresUnreferencedCode]. The NativeAOT samples are updated to implement the abstract base directly, and tests/downstream types are updated accordingly.

What looks good:

  • The split is well-structured — base classes contain only the contract and built-in type handling; all reflection goes into the Reflection* subclasses.
  • [RequiresDynamicCode] and [RequiresUnreferencedCode] annotations are applied consistently throughout the chain (samples, tests, JRE runtime types).
  • Public API baselines are updated correctly.
  • TryRegisterBuiltInNativeMembers is a nice extraction that NativeAOT consumers can call without needing the full reflection machinery.
  • The JreRuntimeOptions.CreateJreVM(JniTypeManager) overload is a good ergonomic addition.

Issues by severity:

Severity Count
⚠️ Warning 1
💡 Suggestion 3

Notes:

  • CI only shows license/cla — no build/test results visible yet. The PR is not mergeable until CI validates the full build.
  • The large abstract surface area for NativeAOT consumers (~12 methods) may be worth revisiting — see inline suggestion about making unused extension points virtual with default null returns.

Generated by Java.Interop PR Reviewer for issue #1441 · ● 14.3M

Comment thread samples/Hello-NativeAOTFromAndroid/NativeAotTypeManager.cs
Comment thread samples/Hello-NativeAOTFromJNI/NativeAotTypeManager.cs
Comment thread src/Java.Interop/Java.Interop/JniRuntime.ReflectionJniTypeManager.cs Outdated
simonrozsival and others added 2 commits June 9, 2026 22:20
Map JNI primitive and boxed primitive signatures in the NativeAOT sample type managers so constructor signature matching can resolve primitive parameters such as I to System.Int32.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
To minimize migration pain for callers moving from .NET 10 to .NET 11,
revert JniTypeManager from `abstract` back to a regular class. Methods
that were turned into `abstract` overrides now have `virtual` defaults
so existing subclasses keep compiling:

  * Type lookup methods return null/empty defaults.
  * RegisterNativeMembers is a no-op by default; reflection-based
    behavior remains in ReflectionJniTypeManager (override to opt-in).

Restore PublicAPI.Shipped.txt to its main-branch state (per the
`Microsoft.CodeAnalysis.PublicApiAnalyzers` convention, never edit
shipped entries in place) and track every breaking change in
PublicAPI.Unshipped.txt:

  * 5 new JniTypeManager.* APIs are now `virtual` (not `abstract`).
  * `*REMOVED*` entries for the two `JniTypeManager.TryRegisterNativeMembers`
    overloads that moved to `ReflectionJniTypeManager`.
  * `*REMOVED*virtual` + `abstract` pairs for the two virtual->abstract
    transitions on `JniValueManager` (CreatePeer, GetValueMarshalerCore(Type)).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jonathanpeppers jonathanpeppers merged commit 0244c10 into main Jun 9, 2026
2 checks passed
@jonathanpeppers jonathanpeppers deleted the dev/simonrozsival/fix-aot-warnings-jni-type-manager branch June 9, 2026 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-review This PR is ready to review/merge, thanks!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants