Make JniTypeManager reflection fallback safe#1430
Open
simonrozsival wants to merge 1 commit into
Open
Conversation
Avoid reflection failures when static method fallback types are unavailable and update NativeAOT type manager samples accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates JniRuntime.JniTypeManager to avoid reflection-based type construction in its fallback paths (improving trimming/NativeAOT safety), updates derived type managers (tests/samples) to use the new safe lookup hook, and adds targeted tests plus a PublicAPI entry.
Changes:
- Add
JniRuntime.JniTypeManager.GetTypeForSimpleReference()and migrateGetType()to use it (and explicitly reject array signatures for the DAM-annotatedGetType()path). - Replace runtime array/generic construction with a precomputed “known array types” map.
- Update
JreTypeManager, test type managers, and NativeAOT samples to override the new hook; add/adjust tests and PublicAPI tracking.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Java.Interop-Tests/Java.Interop/JniTypeManagerTests.cs | Adjusts GetType() tests to validate new behavior (incl. generic holder mapping). |
| tests/Java.Interop-Tests/Java.Interop/JniRuntime.JniTypeManagerTests.cs | Adds a NET-only test asserting generic invoker type construction now throws. |
| tests/Java.Interop-Tests/Java.Interop/JavaVMFixture.cs | Updates fixture type manager to override GetTypeForSimpleReference(); removes debug output. |
| src/Java.Runtime.Environment/Java.Interop/JreTypeManager.cs | Overrides GetTypeForSimpleReference() to integrate typeMappings without reflection fallback. |
| src/Java.Interop/PublicAPI.Unshipped.txt | Tracks the newly added protected virtual API. |
| src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs | Core change: new safe lookup hook + known array-type mapping, removes reflection-based construction paths. |
| samples/Hello-NativeAOTFromJNI/NativeAotTypeManager.cs | Overrides GetTypeForSimpleReference() for safe lookups in the NativeAOT sample. |
| samples/Hello-NativeAOTFromAndroid/NativeAotTypeManager.cs | Removes diagnostic output and adds GetTypeForSimpleReference() override for safe lookups. |
Comments suppressed due to low confidence (1)
src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs:375
GetTypeForSimpleReference()only handles primitives/boxed primitives/string/void, butGetTypesForSimpleReference()can return additional built-in types (e.g., JavaProxyObject/JavaProxyThrowable/ManagedPeer viaJniBuiltinSimpleReferenceToType). As a result,GetType()will now return null for those built-in simple references. Consider either delegating to the same built-in map here or adding explicit cases for the remaining built-ins soGetType()stays consistent withGetTypes()for built-in types.
[return: DynamicallyAccessedMembers (MethodsConstructors)]
protected virtual Type? GetTypeForSimpleReference (string jniSimpleReference)
{
AssertValid ();
AssertSimpleReference (jniSimpleReference);
return jniSimpleReference switch {
"java/lang/String" => TypeOf<string> (),
"V" => TypeOfVoid (),
"Z" => TypeOf<Boolean> (),
"java/lang/Boolean" => TypeOf<Boolean?> (),
"B" => TypeOf<SByte> (),
"java/lang/Byte" => TypeOf<SByte?> (),
"C" => TypeOf<Char> (),
"java/lang/Character" => TypeOf<Char?> (),
"S" => TypeOf<Int16> (),
"java/lang/Short" => TypeOf<Int16?> (),
"I" => TypeOf<Int32> (),
"java/lang/Integer" => TypeOf<Int32?> (),
"J" => TypeOf<Int64> (),
"java/lang/Long" => TypeOf<Int64?> (),
"F" => TypeOf<Single> (),
"java/lang/Float" => TypeOf<Single?> (),
"D" => TypeOf<Double> (),
"java/lang/Double" => TypeOf<Double?> (),
_ => null,
};
Comment on lines
+279
to
+295
| static KnownArrayTypesInfo InitKnownArrayTypes () | ||
| { | ||
| var arrayTypes = new Dictionary<Type, Type> (); | ||
| var javaObjectArrayTypes = new Dictionary<Type, Type> (); | ||
|
|
||
| AddKnownArrayTypes<string> (arrayTypes, javaObjectArrayTypes); | ||
|
|
||
| AddKnownPrimitiveArrayTypes<Boolean, JavaBooleanArray> (arrayTypes, javaObjectArrayTypes); | ||
| AddKnownPrimitiveArrayTypes<SByte, JavaSByteArray> (arrayTypes, javaObjectArrayTypes); | ||
| AddKnownPrimitiveArrayTypes<Char, JavaCharArray> (arrayTypes, javaObjectArrayTypes); | ||
| AddKnownPrimitiveArrayTypes<Int16, JavaInt16Array> (arrayTypes, javaObjectArrayTypes); | ||
| AddKnownPrimitiveArrayTypes<Int32, JavaInt32Array> (arrayTypes, javaObjectArrayTypes); | ||
| AddKnownPrimitiveArrayTypes<Int64, JavaInt64Array> (arrayTypes, javaObjectArrayTypes); | ||
| AddKnownPrimitiveArrayTypes<Single, JavaSingleArray> (arrayTypes, javaObjectArrayTypes); | ||
| AddKnownPrimitiveArrayTypes<Double, JavaDoubleArray> (arrayTypes, javaObjectArrayTypes); | ||
|
|
||
| return new KnownArrayTypesInfo (arrayTypes, javaObjectArrayTypes); |
Comment on lines
+321
to
+325
| static bool TryMakeArrayType (Type type, out Type? arrayType) => | ||
| KnownArrayTypes.Value.ArrayTypes.TryGetValue (type, out arrayType); | ||
|
|
||
| static bool TryMakeJavaObjectArrayType (Type type, out Type? arrayType) => | ||
| KnownArrayTypes.Value.JavaObjectArrayTypes.TryGetValue (type, out arrayType); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
git diff --checkpasseddotnet test tests/Java.Interop-Tests/Java.Interop-Tests.csproj -v minimal --filter "Name~JniTypeManager"blocked locally by missingbin/BuildDebug/Java.Interop.BootstrapTasks.dlland empty javac command exiting with code 127