Register JNI natives via blittable JniNativeMethod#1474
Conversation
The default native-method registration funnel invoked the JNI RegisterNatives function pointer typed as `delegate* unmanaged<nint, nint, JniNativeMethodRegistration[], int, int>`, passing a non-blittable managed array and relying on a runtime-synthesized marshalling stub to convert it to JNINativeMethod*. crossgen2 miscompiles that stub under composite ReadyToRun + PGO (MIBC): it degrades to a raw struct blit, so the native `name`/`signature` pointers end up referencing the managed string objects instead of marshalled UTF-8 data. The registered method names are corrupted, producing NoSuchMethodError at startup (e.g. MauiApplication, net.dot.jni.ManagedPeer). Marshal JniNativeMethodRegistration[] into blittable JniNativeMethod values and dispatch to the existing RegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>) overload, eliminating the non-blittable `delegate* unmanaged<>` call site. This matches the trimmable type-map path, which was already immune. Fixes dotnet/android#11633 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address PR review: the generated `_RegisterNatives` wrapper checked `ExceptionOccurred()` after the native call and rethrew/cleared any pending Java exception (e.g. NoSuchMethodError). Routing registration through the blittable `RegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>)` overload dropped that check, which could leave a pending exception in the JNIEnv and cause subsequent JNI calls to fail or hide the real error. Add the pending-exception check (and a `type.IsValid` guard) to the blittable overload so both the array-based registration path and the trimmable type-map path surface JNI registration failures correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates Java.Interop’s JNI native-method registration to avoid passing a non-blittable JniNativeMethodRegistration[] through a delegate* unmanaged<> call path, instead marshalling into blittable JniNativeMethod entries and using the existing RegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>) overload. It also improves AOT analyzer signaling for APIs that rely on dynamic code and clarifies test suppressions.
Changes:
- Reworked
JniEnvironment.Types.RegisterNatives(JniObjectReference, JniNativeMethodRegistration[], int)to marshal names/signatures to unmanaged UTF-8 and dispatch via the blittable overload. - Added pending-Java-exception surfacing/clearing to the blittable
RegisterNativesoverload and addedRequiresDynamicCode/IL3050suppressions where appropriate. - Updated tests and
ManagedPeerto suppress IL3050 where they intentionally exercise non-AOT-compatible registration APIs.
Show a summary per file
| File | Description |
|---|---|
| tests/Java.Interop-Tests/Java.Interop/JniTypeTest.cs | Adds IL3050 suppressions for tests that intentionally call non-AOT-compatible native registration APIs. |
| src/Java.Interop/Java.Interop/ManagedPeer.cs | Suppresses IL3050 on ManagedPeer static ctor due to non-AOT-compatible native registration usage. |
| src/Java.Interop/Java.Interop/JniType.cs | Marks RegisterNativeMethods(JniNativeMethodRegistration[]) as RequiresDynamicCode for clearer AOT analyzer behavior. |
| src/Java.Interop/Java.Interop/JniEnvironment.Types.cs | Implements marshalling to blittable JniNativeMethod and improves exception handling for RegisterNatives. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 2
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…nally block Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
… helper Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
|
/review |
|
✅ Java.Interop PR Reviewer completed successfully! |
There was a problem hiding this comment.
🤖
Thorough, well-documented fix. The root-cause analysis (crossgen2 emitting the blittable Memmove fallback of the StructureMarshaler<__Canon> intrinsic when it precompiles the hot, non-blittable array marshaller for a delegate* unmanaged<> call) is convincing, and rerouting through the blittable RegisterNatives (ReadOnlySpan<JniNativeMethod>) overload is the right call. I independently verified:
- ✅ The hand-rolled marshalling is correct:
StringToCoTaskMemUTF8↔ZeroFreeCoTaskMemUTF8are correctly paired,unmanagedStrings.Clear ()+ thefinallycorrectly free partially-allocated buffers if an allocation fails mid-loop, andGC.KeepAlive (methods)is correctly placed after the native call to keep the marshaler delegates rooted across it (long-term rooting stays withJniType.methods/ManagedPeer._members, unchanged). - ✅
[RequiresDynamicCode]coverage is complete —ManagedPeer, both tests, andReflectionJniTypeManager(class-level annotation) are all covered, so no new IL3050 escapes the build. - ✅
GetExceptionForLastThrowable ()does callExceptionClear (), so the new "surface and clear" behavior matches the comment and is a genuine improvement for the trimmable type-map path too. - ✅ CI (
dotnet.java-interop) is green;mergeable_state: blockedreflects review requirements, not a failing check.
Main ask: this fixes a method-name corruption bug (refs dotnet/android#11633) but adds no test that exercises the reworked JniNativeMethodRegistration[] marshalling loop directly — the existing tests either pass zero methods or hit the already-correct blittable overload. A targeted regression test would lock in the fix (details inline).
Findings: 1 Marshaler behavioral change). No correctness or safety defects found in the new code.
Generated by Java.Interop PR Reviewer for issue #1474 · 585.1 AIC · ⌖ 47.3 AIC · ⊞ 34.6K
Comment /review to run again
|
@copilot address code review |
…r JniNativeMethodRegistration[] path Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
…rNatives Upstream dotnet/java-interop#1474 added [RequiresDynamicCode] to the JniNativeMethodRegistration[] overload of JniEnvironment.Types.RegisterNatives, which makes ManagedTypeManager.RegisterNativeMembers emit IL3050 under NativeAOT. Marshal directly into blittable JniNativeMethod values (UTF-8 strings via Marshal.StringToCoTaskMemUTF8 + Marshal.GetFunctionPointerForDelegate) and call the blittable RegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>) overload to eliminate the warning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Upstream dotnet/java-interop#1474 added [RequiresDynamicCode] to: - JniRuntime.ReflectionJniTypeManager (the class), surfacing IL3050 at ManagedTypeManager..ctor() since we derive from it. - JniEnvironment.Types.RegisterNatives(.., JniNativeMethodRegistration[], int), surfacing IL3050 at ManagedTypeManager.RegisterNativeMembers. Suppress both for now; the JniNativeMethodRegistration[] registration path will be migrated to the blittable RegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>) overload in a future change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
### Bump external/Java.Interop from `8d54473` to `6ec1345` Bumps [external/Java.Interop](https://github.com/dotnet/java-interop) from `8d54473` to `6ec1345`. - [Commits](dotnet/java-interop@8d54473...6ec1345) --- updated-dependencies: - dependency-name: external/Java.Interop dependency-version: 6ec1345165fa7385c935f16ccaa8cc38be50a080 dependency-type: direct:production ... ### Remove Java.Runtime.Environment project from solution The Java.Runtime.Environment project was removed upstream in dotnet/java-interop#1447 (commit 4e142449), so the reference in Xamarin.Android.sln no longer resolves and breaks restore. ### Suppress new IL3050 warnings in ManagedTypeManager Upstream dotnet/java-interop#1474 added [RequiresDynamicCode] to: - JniRuntime.ReflectionJniTypeManager (the class), surfacing IL3050 at ManagedTypeManager..ctor() since we derive from it. - JniEnvironment.Types.RegisterNatives(.., JniNativeMethodRegistration[], int), surfacing IL3050 at ManagedTypeManager.RegisterNativeMembers. Suppress for now; the JniNativeMethodRegistration[] registration path will be migrated to the blittable RegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>) overload in a future change. Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary Register JNI native methods by marshalling into the **blittable** `JniNativeMethod` struct and calling the existing `RegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>)` overload, instead of invoking the JNI `RegisterNatives` function pointer with a **non-blittable managed array** (`JniNativeMethodRegistration[]`). This avoids relying on the runtime to marshal an array of a non-blittable struct across a `delegate* unmanaged<>` call — a path that is currently miscompiled by crossgen2 under composite ReadyToRun + PGO and corrupts the registered method names. Refs dotnet/android#11633 ## Background: what breaks The default (non-trimmable / llvm-ir typemap) registration funnels through: ```csharp // JniEnvironment.Types public static void RegisterNatives (JniObjectReference type, JniNativeMethodRegistration [] methods, int numMethods) => _RegisterNatives (type, methods, numMethods); // generated invoker: // delegate* unmanaged<IntPtr, jobject, JniNativeMethodRegistration[], int, int> RegisterNatives ``` `JniNativeMethodRegistration` is non-blittable (`string Name; string Signature; Delegate Marshaler`). Passing `JniNativeMethodRegistration[]` through the `delegate* unmanaged<>` requires the runtime to marshal the array element-by-element. `ManagedPeer..cctor`, `AndroidTypeManager`, `ManagedTypeManager`, and every `JniType.RegisterNativeMethods` caller funnel through this single method (`_RegisterNatives` has exactly one caller), so fixing it here fixes all of them. ## Root cause (a crossgen2 / runtime regression) dotnet/runtime **#126911** ("Move built-in array marshalling to managed", 2026-05-01) moved array-of-struct marshalling from native C++ into the managed generic `System.StubHelpers.StructureMarshaler<T> : IArrayElementMarshaler<T, StructureMarshaler<T>>`. Its element converter is an intrinsic with a **blittable-only fallback body**: ```csharp // src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs // "Non-blittable structs should have a custom IL body generated with the marshaling logic." [Intrinsic] private static void ConvertToUnmanagedCore (ref T managed, byte* unmanaged, ref CleanupWorkListElement? cleanupWorkList) => SpanHelpers.Memmove (ref *unmanaged, ref Unsafe.As<T,byte>(ref managed), (nuint)sizeof(T)); ``` For non-blittable `T`, the VM generates a real per-field marshalling body at JIT time (`StructMarshalStubs::TryGenerateStructMarshallingMethod`, `dllimport.cpp`). **crossgen2 has no equivalent**, so when PGO/MIBC marks the marshaller hot and crossgen2 precompiles the shared canonical (`__Canon`) instantiation, it emits the literal `Memmove` fallback — a raw blit of the managed object references into the native struct. Disassembly of the precompiled canonical converter from a composite-R2R + MIBC image: ```asm StructureMarshaler`1<__Canon>.ConvertToUnmanagedCore: ldr x0, [x1] ; first 8 bytes of the managed struct = the Name string REFERENCE str x0, [x2] ; stored straight into the native JNINativeMethod.name ← no string→char* marshalling ret ``` So JNI receives a managed `string` object pointer where it expects a UTF-8 `char*`, the method name is garbage, and registration fails with `NoSuchMethodError` during startup (e.g. `MauiApplication`, `net.dot.jni.ManagedPeer`). This only manifests when crossgen2 precompiles the marshaller (MIBC marks it hot). Without a profile the JIT compiles it and registration is correct — which is why the same app works without a startup MIBC. ## Why this is the right fix - It **eliminates the non-blittable array marshalling** at this call site entirely. The blittable `RegisterNatives(ReadOnlySpan<JniNativeMethod>)` path marshals names/signatures to UTF-8 ourselves (`Marshal.StringToCoTaskMemUTF8`) and passes a blittable `JniNativeMethod*` — there is no `StructureMarshaler<T>` involved, so it is correct regardless of the crossgen2 bug. - It matches the **trimmable type-map path**, which already used the blittable overload and was therefore never affected. - It is the single registration chokepoint, so it fixes `ManagedPeer`, `AndroidTypeManager`, and `ManagedTypeManager` together. We do **not** want to marshal arrays of non-blittable types across `delegate* unmanaged<>` / P/Invoke boundaries given this runtime limitation. A scan of the shipped runtime path (Java.Interop + Mono.Android) shows `RegisterNatives` was the only such site: it is the only invoker function pointer with an array parameter, the only non-blittable-array native call; all `JValue[]` call sites already pin (`fixed (JValue* …)`) and pass a blittable pointer. ## Changes - Rework `JniEnvironment.Types.RegisterNatives(JniObjectReference, JniNativeMethodRegistration[], int)` to marshal `Name`/`Signature` to unmanaged UTF-8 and a function pointer, then dispatch to the blittable `RegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>)` overload (no more `_RegisterNatives` / non-blittable `delegate* unmanaged<>` call). - Preserve JNI error behavior in the blittable overload: it now observes and rethrows (clearing) any pending Java exception from `JNIEnv::RegisterNatives()` — e.g. `NoSuchMethodError` — and guards `type.IsValid`, matching the generated `_RegisterNatives` wrapper it replaces. This benefits both the array-based path and the trimmable type-map path (which previously could leave a pending exception in the JNIEnv). - Add regression tests for the `JniNativeMethodRegistration[]` path to validate correct UTF-8 marshaling and function pointer conversion, covering both stack-allocated (≤32 methods) and heap-allocated (>32 methods) code paths. ## Implementation notes - UTF-8 name/signature buffers are allocated with `Marshal.StringToCoTaskMemUTF8` and freed in a `finally` block with `Marshal.ZeroFreeCoTaskMemUTF8`, guarding against `IntPtr.Zero` to prevent crashes with uninitialized entries. `GC.KeepAlive (methods)` keeps the marshaler delegates alive across the native call. - Null `Marshaler` delegates are explicitly checked with an actionable error message that includes the array index and method signature, rather than relying on the generic `ArgumentNullException` from `Marshal.GetFunctionPointerForDelegate`. - `Marshal.GetFunctionPointerForDelegate` is called inline within the already-`RequiresDynamicCode`-annotated method. This `JniNativeMethodRegistration[]` path runs only on JIT-capable runtimes (MonoVM/CoreCLR); NativeAOT registers through the trimmable type map with statically-compiled function pointers and never reaches it. ## Verification Reproduced and fixed in an isolated `net11.0` console app (no Java.Interop/Android types): a non-blittable `struct[]` passed through a `delegate* unmanaged<>` to a real native function corrupts only under composite R2R + MIBC (when the call site is hot); the blittable equivalent is correct under all configurations (JIT, plain R2R, composite R2R, composite R2R + MIBC). ## Tracking - Underlying runtime regression introduced by dotnet/runtime#126911; durable fix belongs in crossgen2 (expand the marshalling intrinsic for non-blittable `__Canon`, or defer it to the JIT). This PR is the Java.Interop-side fix and is correct independent of the runtime change.
…veMembers dotnet/java-interop#1474 (pulled in by the Java.Interop bump in this PR) adds [RequiresDynamicCode] to JniEnvironment.Types.RegisterNatives(.., JniNativeMethodRegistration[], int), surfacing a new IL3050 AOT analysis warning at ManagedTypeManager.RegisterNativeMembers under NativeAOT. That broke BuildHasNoWarnings(True,False,"apk",NativeAOT), which asserts an exact warning count. Suppress for now; the JniNativeMethodRegistration[] registration path will be migrated to the blittable RegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>) overload in a future change. Mirrors the companion change made on main in #11782. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
) ### Bump external/Java.Interop from `7049364` to `c24a8e9b` Fixes a Release-mode startup crash in MAUI apps on preview.6: System.TypeInitializationException: ... Java.Interop.ManagedPeer ---> Java.Lang.IncompatibleClassChangeError: no static or non-static method "Lnet/dot/jni/ManagedPeer;.????" Picks up dotnet/java-interop#1474 "Register JNI natives via blittable JniNativeMethod", which reworks the RegisterNatives chokepoint to marshal method names/signatures to UTF-8 ourselves and dispatch through the blittable RegisterNatives(ReadOnlySpan<JniNativeMethod>) overload. The old path passed a non-blittable JniNativeMethodRegistration[] through a delegate* unmanaged<> call. dotnet/runtime#126911 moved array-of-struct marshalling into a managed StructureMarshaler<T> whose blittable-only fallback is miscompiled by crossgen2 under composite R2R + startup MIBC, blitting a managed string reference into the native char* name and corrupting the registered method names (the "????" above). This only reproduces in Release (composite R2R + PGO), which is why Debug and `dotnet new android` were unaffected while MAUI Release crashed. Refs: #11633 ### [Mono.Android] Suppress new IL3050 in ManagedTypeManager.RegisterNativeMembers dotnet/java-interop#1474 (pulled in by the Java.Interop bump in this PR) adds [RequiresDynamicCode] to JniEnvironment.Types.RegisterNatives(.., JniNativeMethodRegistration[], int), surfacing a new IL3050 AOT analysis warning at ManagedTypeManager.RegisterNativeMembers under NativeAOT. That broke BuildHasNoWarnings(True,False,"apk",NativeAOT), which asserts an exact warning count. Suppress for now; the JniNativeMethodRegistration[] registration path will be migrated to the blittable RegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>) overload in a future change. Mirrors the companion change made on main in #11782. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Summary
Register JNI native methods by marshalling into the blittable
JniNativeMethodstruct and calling the existingRegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>)overload, instead of invoking the JNIRegisterNativesfunction pointer with a non-blittable managed array (JniNativeMethodRegistration[]).This avoids relying on the runtime to marshal an array of a non-blittable struct across a
delegate* unmanaged<>call — a path that is currently miscompiled by crossgen2 under composite ReadyToRun + PGO and corrupts the registered method names.Refs dotnet/android#11633
Background: what breaks
The default (non-trimmable / llvm-ir typemap) registration funnels through:
JniNativeMethodRegistrationis non-blittable (string Name; string Signature; Delegate Marshaler). PassingJniNativeMethodRegistration[]through thedelegate* unmanaged<>requires the runtime to marshal the array element-by-element.ManagedPeer..cctor,AndroidTypeManager,ManagedTypeManager, and everyJniType.RegisterNativeMethodscaller funnel through this single method (_RegisterNativeshas exactly one caller), so fixing it here fixes all of them.Root cause (a crossgen2 / runtime regression)
dotnet/runtime #126911 ("Move built-in array marshalling to managed", 2026-05-01) moved array-of-struct marshalling from native C++ into the managed generic
System.StubHelpers.StructureMarshaler<T> : IArrayElementMarshaler<T, StructureMarshaler<T>>. Its element converter is an intrinsic with a blittable-only fallback body:For non-blittable
T, the VM generates a real per-field marshalling body at JIT time (StructMarshalStubs::TryGenerateStructMarshallingMethod,dllimport.cpp). crossgen2 has no equivalent, so when PGO/MIBC marks the marshaller hot and crossgen2 precompiles the shared canonical (__Canon) instantiation, it emits the literalMemmovefallback — a raw blit of the managed object references into the native struct.Disassembly of the precompiled canonical converter from a composite-R2R + MIBC image:
So JNI receives a managed
stringobject pointer where it expects a UTF-8char*, the method name is garbage, and registration fails withNoSuchMethodErrorduring startup (e.g.MauiApplication,net.dot.jni.ManagedPeer).This only manifests when crossgen2 precompiles the marshaller (MIBC marks it hot). Without a profile the JIT compiles it and registration is correct — which is why the same app works without a startup MIBC.
Why this is the right fix
RegisterNatives(ReadOnlySpan<JniNativeMethod>)path marshals names/signatures to UTF-8 ourselves (Marshal.StringToCoTaskMemUTF8) and passes a blittableJniNativeMethod*— there is noStructureMarshaler<T>involved, so it is correct regardless of the crossgen2 bug.ManagedPeer,AndroidTypeManager, andManagedTypeManagertogether.We do not want to marshal arrays of non-blittable types across
delegate* unmanaged<>/ P/Invoke boundaries given this runtime limitation. A scan of the shipped runtime path (Java.Interop + Mono.Android) showsRegisterNativeswas the only such site: it is the only invoker function pointer with an array parameter, the only non-blittable-array native call; allJValue[]call sites already pin (fixed (JValue* …)) and pass a blittable pointer.Changes
JniEnvironment.Types.RegisterNatives(JniObjectReference, JniNativeMethodRegistration[], int)to marshalName/Signatureto unmanaged UTF-8 and a function pointer, then dispatch to the blittableRegisterNatives(JniObjectReference, ReadOnlySpan<JniNativeMethod>)overload (no more_RegisterNatives/ non-blittabledelegate* unmanaged<>call).JNIEnv::RegisterNatives()— e.g.NoSuchMethodError— and guardstype.IsValid, matching the generated_RegisterNativeswrapper it replaces. This benefits both the array-based path and the trimmable type-map path (which previously could leave a pending exception in the JNIEnv).JniNativeMethodRegistration[]path to validate correct UTF-8 marshaling and function pointer conversion, covering both stack-allocated (≤32 methods) and heap-allocated (>32 methods) code paths.Implementation notes
Marshal.StringToCoTaskMemUTF8and freed in afinallyblock withMarshal.ZeroFreeCoTaskMemUTF8, guarding againstIntPtr.Zeroto prevent crashes with uninitialized entries.GC.KeepAlive (methods)keeps the marshaler delegates alive across the native call.Marshalerdelegates are explicitly checked with an actionable error message that includes the array index and method signature, rather than relying on the genericArgumentNullExceptionfromMarshal.GetFunctionPointerForDelegate.Marshal.GetFunctionPointerForDelegateis called inline within the already-RequiresDynamicCode-annotated method. ThisJniNativeMethodRegistration[]path runs only on JIT-capable runtimes (MonoVM/CoreCLR); NativeAOT registers through the trimmable type map with statically-compiled function pointers and never reaches it.Verification
Reproduced and fixed in an isolated
net11.0console app (no Java.Interop/Android types): a non-blittablestruct[]passed through adelegate* unmanaged<>to a real native function corrupts only under composite R2R + MIBC (when the call site is hot); the blittable equivalent is correct under all configurations (JIT, plain R2R, composite R2R, composite R2R + MIBC).Tracking
__Canon, or defer it to the JIT). This PR is the Java.Interop-side fix and is correct independent of the runtime change.