Skip to content

[Unsafe] Type-check JNI callback delegates - #13042

Merged
simonrozsival merged 3 commits into
mainfrom
simonrozsival-jni-delegate-type-dispatch
Oct 9, 2026
Merged

simonrozsival merged 3 commits into
mainfrom
simonrozsival-jni-delegate-type-dispatch

Conversation

@simonrozsival

@simonrozsival simonrozsival commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

Pull Request
title and
description
should follow the
commit-messages.md workflow documentation, and in particular should include:

  • Useful description of why the change is necessary.
  • Links to issues fixed
  • Unit tests

Summary

JNINativeWrapper.CreateBuiltInDelegate dispatches on the runtime delegate type instead of its simple name. Unrelated custom delegates that collide with built-in names therefore use the existing Reflection.Emit fallback with their original delegate type and signature. All 40 built-in mappings and their wrappers remain unchanged. No new tests are included in this PR.

Context

#11467 is broader background only; this PR addresses built-in delegate dispatch and does not close that issue.

Validation

  • PASS — T4 regeneration and cmp verified the generated source matches the template; all 40 mappings remain.
  • PASS — ./dotnet-local.sh test external/Java.Interop/tests/Xamarin.SourceWriter-Tests/Xamarin.SourceWriter-Tests.csproj -v minimal (11 passed).
  • PASS — ./dotnet-local.sh test external/Java.Interop/tests/generator-Tests/generator-Tests.csproj -v minimal -p:JavaCPath=/Users/simon/Library/Java/JavaVirtualMachines/jdk-23.0.2+7/Contents/Home/bin/javac -p:JarPath=/Users/simon/Library/Java/JavaVirtualMachines/jdk-23.0.2+7/Contents/Home/bin/jar (526 passed).
  • BLOCKED — make prepare && make all: make prepare failed during restore with NU1102 because Microsoft.NETCore.App.Ref 10.0.13 is unavailable from the configured feeds; make all was not reached.
  • CI follow-up — Build 1628850 failed at IL3050 for the subsequently removed regression test. The follow-up commit has been pushed; no dotnet-android checks for that commit were listed at the time of this update.

Dispatch built-in JNI callbacks by concrete delegate type rather than their
simple names.  A custom delegate with a colliding name must use the
Reflection.Emit fallback with its original delegate type and signature.

The 40 built-in mappings and their callback wrappers remain unchanged.

Context: #11467

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 8, 2026 22:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The focused runtime regression test and SDK build were blocked, leaving the changed JNI callback path unvalidated.

1 open finding
What changed in this PR

Replaces unsafe name-based JNI delegate dispatch with exact runtime-type matching.

Changes:

  • Uses typed delegate patterns instead of Unsafe.As.
  • Adds regression coverage for same-name custom delegates.
  • Keeps generated source synchronized with its T4 template.
File Description
JNINativeWrapper.g.tt Updates delegate dispatch generation.
JNINativeWrapper.g.cs Applies generated type-safe mappings.
JnienvArrayMarshaling.cs Adds collision regression test.

🧠 Review effort: Balanced

Comment thread src/Mono.Android/Android.Runtime/JNINativeWrapper.g.tt
simonrozsival and others added 2 commits October 9, 2026 08:52
Remove the new CreateDelegate regression test as requested.  Its call to
CreateDelegate also triggers IL3050 during NativeAOT compilation, even
though the test is excluded from NativeAOT execution.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@simonrozsival simonrozsival changed the title [Mono.Android] Type-check JNI callback delegates [Unsafe] Type-check JNI callback delegates Oct 9, 2026
@simonrozsival

Copy link
Copy Markdown
Member Author

@dalexsoto review

@simonrozsival
simonrozsival enabled auto-merge (squash) October 9, 2026 10:24
@simonrozsival simonrozsival added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Oct 9, 2026

@dalexsoto dalexsoto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The typed dispatch preserves all 40 built-in mappings and their exception/GC-bridge behavior while routing unrelated same-named delegates through the existing fallback. The template, generated source, and updated caller are consistent; no blocking issue remains.

@simonrozsival
simonrozsival merged commit 036438e into main Oct 9, 2026
42 checks passed
@simonrozsival
simonrozsival deleted the simonrozsival-jni-delegate-type-dispatch branch October 9, 2026 12:45
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, I think any CI failures are just flaky (ignorable).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants