Repository navigation
[Unsafe][Java.Interop] Opt into updated C# memory-safety rules - #13192
Open
simonrozsival wants to merge 1 commit into
Open
simonrozsival wants to merge 1 commit into
simonrozsival wants to merge 1 commit into
Conversation
Enable project-local caller enforcement after the focused preparation layers. Keep address getters and managed acquisition safe, enforce borrowed setters and raw ABI contracts, and preserve the warning-obsolete RC2 IntPtr APIs. Retain compiler and metadata regressions for runtime creation, borrowed-pointer publication, primitive-array references, and safe legacy compatibility boundaries. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Public JNI contract activation across compiler modes and runtime packs warrants final human review of compatibility and validation evidence.
No open issues 🎉
What changed in this PR
Activates updated C# memory-safety rules in Java.Interop, completing the prepared JNI caller-contract migration without enabling them in Mono.Android or customer projects.
Changes:
- Enforces unsafe caller obligations while preserving safe address getters and managed array acquisition.
- Updates API baselines, paired generated output, and architecture documentation.
- Adds compiler and metadata regression tests for safety contracts and legacy compatibility.
| File | Description |
|---|---|
| tests/Mono.Android-Tests/Java.Interop-Tests/Java.Interop/JniCallerSafetyTests.cs | Tests caller-contract metadata and legacy compatibility. |
| tests/Mono.Android-Tests/Java.Interop-Tests/Java.Interop/JavaPrimitiveArraySafetyTests.cs | Tests unsafe ref indexers and safe acquisition/getters. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/JniEnvMemorySafetyTests.cs | Adds consumer compilation checks. |
| external/Java.Interop/src/Java.Interop/PublicAPI.Unshipped.txt | Records pending unsafe API contracts. |
| external/Java.Interop/src/Java.Interop/PublicAPI.Shipped.txt | Updates existing API safety annotations. |
| external/Java.Interop/src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs | Restricts unsafe requirements to pointer setters. |
| external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.cs | Removes pointer-field unsafe modifier. |
| external/Java.Interop/src/Java.Interop/Java.Interop/JniMethodInfo.cs | Updates diagnostic pointer-field declarations. |
| external/Java.Interop/src/Java.Interop/Java.Interop/JavaPrimitiveArrays.tt | Keeps generated address getters safe. |
| external/Java.Interop/src/Java.Interop/Java.Interop/JavaPrimitiveArrays.cs | Synchronizes generated address getters. |
| external/Java.Interop/src/Java.Interop/Java.Interop/JavaArray.cs | Updates pointer storage and address getter declarations. |
| external/Java.Interop/src/Java.Interop/Java.Interop.csproj | Enables updated memory-safety rules locally. |
| external/Java.Interop/Documentation/Architecture.md | Documents activated contracts and compatibility boundaries. |
Review effort: Balanced
This was referenced Oct 19, 2026
simonrozsival
added a commit
that referenced
this pull request
Oct 22, 2026
Replace `ref *InvalidJniObjectReference` constructor sentinels with local default references and the options-only forwarding chain without invoking the allocating default base constructor. Keep all 11 managed constructors caller-safe: parameterless `Java.Interop.JavaObject` and `Java.Interop.JavaException`, `Java.Interop.JavaObjectArray<T>(int)`, and eight primitive-array length constructors. Pointer/native initialization stays inside explicit local unsafe blocks. Keep `JavaPrimitiveArrays.tt` and its canonically regenerated `.cs` together. The audit adds concrete `// SAFETY:` proofs and documents actual raw-JNI ABI, thread, lifetime, aliasing, transfer, and failure obligations while preserving genuine caller-unsafe declarations. It does not redesign ownership, change allocations, or introduce new accepted-call restrictions. **User-directed constructor layout:** the public parameterless `JavaException` remains first and delegates to a private options constructor below it. Its original `if (PeerReference.IsValid) return;` is preserved. The private constructor conditionally captures an already-installed peer's Java stack; fresh creation keeps its existing JNI sequence and capture. Each constructor has one capture site and each path captures once. This private forwarding change was explicitly requested in review; it adds no public API. **Separate bug coverage:** the pinned extraction source omitted stack capture on the preinstalled-peer path; preserving the original delegating-constructor behavior avoids that regression. `JavaSideExceptionConstructorCapturesStackFromPreinstalledPeer` exercises real Java-origin activation. `Transition_RejectsNullEnvironmentPointer` covers zero-JNIEnv rejection before TLS mutation/native dispatch. These are the only two new construction bug tests; the added ordinary managed-construction parity test was removed. Existing baseline tests remain intact. `Features` stays off in this preparation layer. Final **Java.Interop-only** compiler activation/metadata assertions are in [#13192](#13192). **Mono.Android product feature activation is outside this stack.**
simonrozsival
added a commit
that referenced
this pull request
Oct 23, 2026
Prepare UTF-8 remapping metadata for the updated C# memory-safety rules without breaking the APIs shipped in .NET 11 RC2. Related to #11467. Store borrowed JNI type names, method names, and signatures as `byte*`, while keeping JNI handles and method IDs as opaque `IntPtr` values. Add typed remapping properties and the `GetReplacementTypeInfoUnsafe` / `GetReplacementMethodInfoUnsafe` hooks, and use them throughout peer-member lookup and the runtime remapping consumers. Document the NUL-termination and lifetime requirements for retained buffers, preserving managed-string alternatives, lazy caching, pointer precedence, and zero-copy UTF-8 literals. Keep the six released native-int accessors and both legacy virtual hooks safe-callable and warning-obsolete. The typed hooks continue to dispatch through the legacy virtual slots so existing overrides and base calls still work. Make remapping diagnostics format pointer addresses rather than dereference borrowed memory. Include coverage for diagnostic formatting, legacy compatibility, override dispatch, and borrowed metadata reads. This PR does not enable the updated memory-safety rules; activation is handled separately in [#13192](#13192).
This branch has not been deployed
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.
Enable the updated C# memory-safety rules in
Java.Interop, separatingcaller obligations from implementation-local unsafe operations. Raw buffers,
borrowed lifetimes, and JNI callback ABIs need explicit caller contracts;
managed alternatives must not become caller-unsafe merely because they use JNI.
Related: #11467; this PR does not close that issue.
Published source versus pending local revision
Published: this PR still contains exactly one activation commit,
cd9e12f2d482a0ed9260c7e866b2cb73bbf5e212, on runtime preparation88a2829eb7d3d91fa27ebfd085c7d1be2c176c62: 13 files, +483/-213.Its original 20-commit preparation/activation chain starts at
31001bbbfc2cc684d6491ab059e66d7ed57cedda. The validation below remainsevidence for that exact published source and the separately identified
legacy preparation source, not for a revised integration.
Native stack #13185 retains ten review layers. Its root has since moved to
0074a9bb763f0cad7a9cf1ac88c18c9436cba5d9, but this PR has not beenrebased onto that root or the nine upper-layer audit revisions.
There is no claim that the currently revised full stack has built or passed.
Pending and uncommitted: a local audit proposal is 12 files, +293/-10;
none of it is in this PR's published diff. It makes address-only
JniNativeMethodconstruction, owned control-blockAlloc,StartCreateInstancewith its ignored argument pointer, and a privateaddress getter caller-safe.
Free,FreeIfInvalid,FinishCreateInstance,and consuming registration APIs remain caller-unsafe. The two public API
baseline edits remove only
unsafemarkers; signatures, layout,implementation expressions, allocations, and cleanup are unchanged.
The proposal adds concrete
// SAFETY:comments, lifetime/alias/failuredocumentation, focused compiler/metadata assertions, and a safe consumer
that calls all nine array-acquisition methods directly.
The local proposal passed 52 static checks and two actual canonical
T4 renders whose complete outputs match the generated array source
byte-for-byte. Its new compiler fixtures, actual revised-mode PE audits,
SDK builds, and runtime/device suites have not run. The older results
below do not validate those proposed contracts or assertions. Source edits
and reintegration are paused while the overall audit's code churn is reassessed.
Caller contracts and deferred work
All nine
GetElements()acquisition methods are safe-callable, independentlyof the safe typed
Elementsaddress getters. All eight ref-returningprimitive-array indexers remain caller-unsafe. Typed remapping address
getters remain safe; their borrowed-storage setters remain caller-unsafe.
The six released native-int accessors and two legacy virtual hooks retain
their signatures, override/base dispatch, safe calls, and warning-only
obsolescence. These eight RC2 compatibility exceptions are intentional.
Only
Java.Interopenablesupdated-memory-safety-rules; Mono.Androidand customer-project activation remain separate work. The exact original
31-line derived-runtime initializer test and both API records are preserved.
Correcting the address-only descriptor constructor does not authorize new
raw-peer constructor enforcement: the user deferred the non-Throwable
raw-peer enforcement gap requiring broader initializer redesign.
Existing approved Object/Throwable contracts remain unchanged.
Partial JNI registration ownership changes are also user-deferred.
The revised root retains original-array ownership only after successful
registration; callers must preserve callback roots after failed or partial
registration. That behavior is not yet integrated here. An
unregistration-failure test requiring a new hook remains blocked; no new
test hook or ownership redesign is claimed.
Local validation
Historical validation of the exact published chain, not the pending audit
proposal or current revised root. Activated SDK/compiler/PE results below
belong to
cd9e12f2d482a0ed9260c7e866b2cb73bbf5e212; unactivatedpreparation-suite results belong to
bf917a5d38c36e87e4b8e2ced7739ccdc8246d4a.These were actual local SDK outputs, not CI or unrelated standalone APKs.
DOTNET_PROCESSOR_COUNT=2 CMAKE_BUILD_PARALLEL_LEVEL=2 MSBUILDDISABLENODEREUSE=1 make prepare CONFIGURATION=Debug MSBUILD_ARGS='-m:1 -p:UseSharedCompilation=false':passed, 0 warnings/errors.
make preparecommand withCONFIGURATION=Release:passed, 0 warnings/errors.
DOTNET_PROCESSOR_COUNT=2 CMAKE_BUILD_PARALLEL_LEVEL=2 MSBUILDDISABLENODEREUSE=1 make leeroy CONFIGURATION=Debug MSBUILD_ARGS='-m:1 -p:UseSharedCompilation=false':passed primary SDK and APIs 37/37.1/37.2, API compatibility, and workload
setup; 39 existing primary and 62 extra-API warnings, 0 errors.
make leeroycommand withCONFIGURATION=Release:passed the same outputs; 57 existing primary and 62 extra-API warnings,
0 errors. Warnings were retained, not suppressed.
NUNIT_MSBUILD_ARGSset to newline-separated-m:1,-p:UseSharedCompilation=false, and-nr:false, followed by./dotnet-local.sh test bin/TestDebug/net10.0/Xamarin.Android.Build.Tests.dll --filter 'FullyQualifiedName~JniEnvMemorySafetyTests':6 passed, 0 failed/skipped. The corresponding
bin/TestReleaserunalso passed all 6: 12 executed host test cases in total. These covered
actual SDK consumers, negative CS9362, and positive unsafe-initializer/
setter cases; they are not runs of the newly added local fixtures.
three API levels in both configurations: all 18 passed the rules-2,
caller-contract, safe acquisition/address getter, ref-return ABI,
and released-RC2-signature audits for the published source.
passed. The unchanged consumer compiled against the actual
.NET 11 RC2 reference pack bound all eight legacy MemberRefs and
preserved old/new override dispatch.
byte-for-byte parity passed. Compiler probes accepted safe address
getters and explicit unsafe callers, rejected exactly seven unsafe
boundary violations with CS9362, and accepted the eight legacy calls
without unsafe with exactly eight deprecation warnings.
Device validation
Historical exact-source runs only: the preparation rows used the legacy
source identified above; the activated rows used the published
cd9e12f2d4source, not the uncommitted revision or current root. Full suites used owned
API 35 arm64 emulators with automatic bounded cleanup. Normal category
exclusions were unchanged.
The three additional activated CoreCLR skips are the existing diagnostic
method-metadata cases against a Release-built Java.Interop library;
the tests gate on the loaded library's capability, not the APK's DEBUG
symbol. NativeAOT used an actual AArch64 native app library with no
CoreCLR payload. CoreCLR's deployed managed fast-deploy payload was
verified, not inferred from its APK alone.
Canonical fixture builds used
./dotnet-local.sh build tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj -t:SignAndroidPackage -p:RuntimeIdentifier=android-arm64 -p:IncludeCategories= -p:UseSharedCompilation=false -m:1,with
-c Debug -p:PublishAot=false -p:Optimize=falsefor CoreCLR and-c Release -p:PublishAot=true -p:Optimize=truefor NativeAOT. Both passed;CoreCLR had 0 warnings/errors and NativeAOT retained 11 existing
EventSource/trimming/AOT warnings with 0 errors.
The activated suites used canonical MTP
dotnet test --no-buildfromthe test-project directory with matching runtime/configuration properties
and an exact owned
--device. The preparation suites used the sameSDK Android test adapter through the host-verified
dotnet test --test-modules Microsoft.Android.Run.dll --root-directory <owned-SDK-tools> -- --adb=<adb> '--adb-target=-s <owned-serial>' --package=Mono.Android.NET_Tests --instrument=xamarin.android.runtimetests.TestInstrumentation --report-trx --results-directory <results>entry point. This avoids this pinned CLI's unexpected project-mode
DeployToDevicecall even with--no-build; it does not replace theadapter, filter tests, or suppress failures. This was a harness workaround,
not a product fix.
Earlier deployment/identity attempts were preserved as failed harness
attempts, not counted as passing tests. Final installed APKs and managed
payloads were verified before and after the successful runs. All owned
fixtures, QEMU processes, AVDs, homes, and port listeners were cleaned up
within their respective absolute deadlines. No shared or physical
devices were used. No new execution or CI-green claim accompanies this
description-only update.
Pull Request
title and
description
should follow the
commit-messages.mdworkflow documentation, and in particular should include: