Split reflection-based JniTypeManager and JniValueManager behavior by simonrozsival · Pull Request #1441 · dotnet/java-interop

simonrozsival · 2026-06-04T14:21:46Z

Draft PR to let CI validate the JniTypeManager/JniValueManager split and AOT warning work.

Summary:

Keep JniTypeManager as the abstract base and move reflection-backed behavior into DynamicJniTypeManager.
Keep JniValueManager as the abstract base and move reflection-backed behavior into ReflectionJniValueManager.
Update JRE/test managers that extend the default behavior to inherit from DynamicJniTypeManager or ReflectionJniValueManager.
Keep NativeAOT sample managers on the abstract bases with explicit mappings instead of reflection-backed base calls.
Update public API baselines for the split.

Local validation:

dotnet build src/Java.Interop/Java.Interop.csproj -p:JavaCPath="$JAVA_HOME/bin/javac" -p:JarPath="$JAVA_HOME/bin/jar"
dotnet build tests/Java.Interop-Tests/Java.Interop-Tests.csproj -p:JavaCPath="$JAVA_HOME/bin/javac" -p:JarPath="$JAVA_HOME/bin/jar" -p:NativeToolchainSupported=false

Notes:

Full local prepare/build is still blocked in this workspace by local build setup/toolchain issues, so this is intentionally draft for CI coverage.

simonrozsival · 2026-06-04T14:33:41Z

/azp run

azure-pipelines · 2026-06-04T14:33:48Z

No pipelines are associated with this pull request.

Copilot

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

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>

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 · 2026-06-09T09:56:59Z

@copilot resolve the merge conflicts in this pull request

…-aot-warnings-jni-type-manager # Conflicts: # src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot · 2026-06-09T10:01:17Z

@copilot resolve the merge conflicts in this pull request

Resolved the merge conflict in JniRuntime.JniValueManager.cs — the FEATURE_JNIOBJECTREFERENCE_SAFEHANDLES block was removed in the merge commit, consistent with the upstream removal in #1446.

simonrozsival · 2026-06-09T10:10:47Z

/review

github-actions · 2026-06-09T10:11:25Z

✅ Java.Interop PR Reviewer completed successfully!

github-actions

⚠️ Needs Changes

The architectural split of JniTypeManager → JniTypeManager (abstract) + ReflectionJniTypeManager and JniValueManager → JniValueManager (abstract) + ReflectionJniValueManager is well-structured. The approach of pushing reflection-based behavior into opt-in subclasses while keeping the abstract bases AOT-safe is sound. The built-in type signature/lookup helpers in the base class, the ReflectionConstructibleType wrapper for DAM annotation flow, and the KnownArrayTypes lookup table to avoid MakeArrayType/MakeGenericType calls are all good improvements.

Issues by severity

Severity	Count	Summary
❌ Error	2	Runtime crash in `GetObjectArrayMarshaler`; placeholder "TODO" in user-facing attributes
⚠️ Warning	1	Indentation mismatch in `CreateValueCore`
💡 Suggestion	1	Unused `EmptyStringArray` field

Critical: `GetObjectArrayMarshaler` runtime crash

The lambda refactor at ReflectionJniValueManager.cs:481-486 replaced a named generic helper method with a non-generic lambda. Calling GetGenericMethodDefinition() on the resulting compiler-generated method will throw InvalidOperationException at runtime whenever a non-primitive IList<T> value is marshaled.

CI Status

Only license/cla and .github/dependabot.yml checks are present — no build/test CI checks appear to have run on this PR. The build issues mentioned in the PR description may explain this, but the GetObjectArrayMarshaler bug above would cause a runtime failure if tested.

Positive callouts

Clean separation of concerns between AOT-safe base and reflection-based derived classes
Good use of [RequiresDynamicCode]/[RequiresUnreferencedCode] on ReflectionJniTypeManager
The KnownArrayTypes dictionary approach for avoiding MakeArrayType/MakeGenericType in the type manager is a solid AOT-safe alternative
NativeAOT sample updates are thorough and demonstrate the intended usage pattern
Public API baseline updates are comprehensive

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

simonrozsival · 2026-06-09T10:21:28Z

@copilot address code review

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

Copilot · 2026-06-09T10:32:34Z

@copilot address code review

Addressed all code review issues in commit c6bfba0:

Runtime crash fix (ReflectionJniValueManager.cs:481): Restored the named generic helper method pattern so GetGenericMethodDefinition() is called on an actual generic method (GetObjectArrayMarshalerHelper<object>), not a non-generic lambda.
TODO placeholder messages (ReflectionJniValueManager.cs:19-20): Updated [RequiresDynamicCode]/[RequiresUnreferencedCode] attribute messages to match the pattern used on ReflectionJniTypeManager.
Indentation (ReflectionJniValueManager.cs:256): Fixed CreateValueCore indentation to use 3 tabs consistent with the rest of the class.
Unused field (ReflectionJniTypeManager.cs:129): Removed unused EmptyStringArray field.

simonrozsival · 2026-06-09T10:49:02Z

/review

github-actions · 2026-06-09T10:49:30Z

✅ Java.Interop PR Reviewer completed successfully!

github-actions

🤖 Review Summary — ⚠️ Needs Minor Changes

Overview: This PR cleanly splits the reflection-backed behavior from JniTypeManager and JniValueManager into dedicated subclasses (ReflectionJniTypeManager, ReflectionJniValueManager), making the abstract bases NativeAOT-safe. The architecture is sound and the code movement is faithful to the original.

Issues by severity

Severity	Count
⚠️ Warning	2
💡 Suggestion	2

Positive callouts

Clean separation of concerns: abstract bases carry no reflection, [RequiresDynamicCode]/[RequiresUnreferencedCode] correctly annotate reflection subclasses
NativeAOT sample managers properly implement the abstract methods without reflection
ReflectionConstructibleType wrapper preserves [DynamicallyAccessedMembers] flow annotations correctly through the type hierarchy
JreTypeManager composition pattern (inheriting ReflectionJniTypeManager and overriding specific methods) is well-designed
Public API baseline updates look complete and consistent

CI Note

Only the license/cla check is visible — no build/test results were found at review time. Please confirm CI is green before merging.

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

simonrozsival · 2026-06-09T10:58:05Z

@copilot address review comments

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

Copilot · 2026-06-09T11:06:11Z

@copilot address review comments

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

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

simonrozsival · 2026-06-09T11:20:49Z

/review

github-actions · 2026-06-09T11:21:18Z

🚀 Java.Interop PR Reviewer has started processing this issue comment

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

WIP

58f0dbd

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

simonrozsival added 4 commits June 4, 2026 16:45

Remove unnecessary suppression

fd4c112

Move DynamicJniTypeManager to its own separate file

f2246b3

Update NativeAOT sample

6d52b54

Simplify

fa49c9a

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 started reviewing on behalf of simonrozsival June 8, 2026 11:56 View session

Copilot AI reviewed Jun 8, 2026

View reviewed changes

Comment thread samples/Hello-NativeAOTFromJNI/NativeAotTypeManager.cs Outdated

Comment thread samples/Hello-NativeAOTFromAndroid/NativeAotTypeManager.cs Outdated

Comment thread src/Java.Interop/Java.Interop/JniRuntime.ReflectionJniTypeManager.cs

Fix dynamic JniTypeManager test regressions

17e87cd

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

simonrozsival marked this pull request as draft June 8, 2026 14:24

simonrozsival and others added 9 commits June 8, 2026 16:32

Use annotations for dynamic type manager tests

7f9798e

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

Keep NativeAOT samples off dynamic type manager

6429366

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

Revert extra JniTypeManager test assertions

c18bdc7

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

Move dynamic test annotations to classes

a19e758

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

Remove extra generic invoker test

cd635d8

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

Use direct type references in NativeAOT samples

6af232f

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

Separate default JRE type manager from AOT path

906e8dd

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

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

Split JniValueManager and ReflectionJniValueManager

5877194

simonrozsival changed the title ~~Split reflection-based JniTypeManager behavior~~ Split reflection-based JniTypeManager and JniValueManager behavior Jun 9, 2026

simonrozsival added 2 commits June 9, 2026 11:44

Undo unnecessary changes

65e6c60

Remove unnecessary changes

32f4f4f

Suppress warnigns in tests

8f644e5

Copilot started work on behalf of simonrozsival June 9, 2026 09:57 View session

Merge remote-tracking branch 'origin/main' into dev/simonrozsival/fix…

00d3a9d

…-aot-warnings-jni-type-manager # Conflicts: # src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot finished work on behalf of simonrozsival June 9, 2026 10:01

github-actions Bot requested changes Jun 9, 2026

View reviewed changes

Copilot started work on behalf of simonrozsival June 9, 2026 10:21 View session

Fix code review issues in ReflectionJniValueManager and ReflectionJni…

48fbe33

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

Copilot finished work on behalf of simonrozsival June 9, 2026 10:32

github-actions Bot reviewed Jun 9, 2026

View reviewed changes

Copilot started work on behalf of simonrozsival June 9, 2026 10:58 View session

Address review comments: EnsureNotDisposed, spacing, Array.Empty, nul…

5dbaa73

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

Copilot finished work on behalf of simonrozsival June 9, 2026 11:06

Conversation

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

Uh oh!

Uh oh!

simonrozsival commented Jun 4, 2026

Uh oh!

azure-pipelines Bot commented Jun 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simonrozsival commented Jun 9, 2026

Uh oh!

Copilot AI commented Jun 9, 2026

Uh oh!

simonrozsival commented Jun 9, 2026

Uh oh!

github-actions Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

⚠️ Needs Changes

Issues by severity

Critical: GetObjectArrayMarshaler runtime crash

CI Status

Positive callouts

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simonrozsival commented Jun 9, 2026

Uh oh!

Copilot AI commented Jun 9, 2026

Uh oh!

simonrozsival commented Jun 9, 2026

Uh oh!

github-actions Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

🤖 Review Summary — ⚠️ Needs Minor Changes

Issues by severity

Positive callouts

CI Note

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simonrozsival commented Jun 9, 2026

Uh oh!

Copilot AI commented Jun 9, 2026

Uh oh!

simonrozsival commented Jun 9, 2026

Uh oh!

github-actions Bot commented Jun 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

simonrozsival commented Jun 4, 2026 •

edited

Loading

github-actions Bot commented Jun 9, 2026 •

edited

Loading

Critical: `GetObjectArrayMarshaler` runtime crash

github-actions Bot commented Jun 9, 2026 •

edited

Loading