[Mono.Android] Prevent premature JNI handle collection - #12315
Open
jonathanpeppers wants to merge 4 commits into
Open
[Mono.Android] Prevent premature JNI handle collection#12315jonathanpeppers wants to merge 4 commits into
jonathanpeppers wants to merge 4 commits into
Conversation
Keep managed JNI handle owners alive until native calls have consumed their handles, and return stable local references for temporary wrappers. Add concurrent collection coverage for ToLocalJniHandle. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e6105414-1c2c-4696-8d01-6507bda3e4be
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a JNI lifetime hazard where managed handle owners can be collected after a .Handle/borrowed-handle access but before the corresponding native/JNI operation consumes it, leading to invalid handles. The change set hardens several managed→JNI call sites with GC.KeepAlive(...), updates conversions to prefer stable local references, and adds a regression test that stresses JNIEnv.ToLocalJniHandle() under concurrent GC/finalization.
Changes:
- Add
GC.KeepAlive(...)at a number of JNI boundary call sites to prevent premature collection of managed handle owners. - Route legacy
Android.Runtime.JavaObject.GetHandle(object)throughJavaConvert.ToLocalJniHandle(object)and adjustJavaConvert’s JavaObject wrapping to useToLocalJniHandle. - Add a GC-pressure regression test for
JNIEnv.ToLocalJniHandle().
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs | Adds concurrent-GC regression coverage for JNIEnv.ToLocalJniHandle() using a finalizable handle owner. |
| src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs | Keeps certificate object alive across JNI handle consumption. |
| src/Mono.Android/Java.Interop/Runtime.cs | Ensures IJavaObject passed to IsGCUserPeer stays alive through handle usage. |
| src/Mono.Android/Java.Interop/JavaObjectExtensions.cs | Keeps cast source instance alive across GetObject(..., Handle, ...) conversions. |
| src/Mono.Android/Java.Interop/JavaConvert.cs | Makes temporary Android.Runtime.JavaObject wrapper conversion return a stable local ref via JNIEnv.ToLocalJniHandle. |
| src/Mono.Android/Android.Widget/TextView.cs | Keeps listener/receiver alive across CallVoidMethod in event hookup. |
| src/Mono.Android/Android.Widget/ArrayAdapter.cs | Adds GC.KeepAlive(context) after constructor/static JNI calls and keeps comparator/this alive in JNI calls. |
| src/Mono.Android/Android.Widget/AdapterViewAnimator.cs | Adds GC.KeepAlive(this/value) around adapter getter/setter JNI calls. |
| src/Mono.Android/Android.Widget/AdapterView.cs | Keeps constructor parameters alive across StartCreateInstance/FinishCreateInstance calls. |
| src/Mono.Android/Android.Widget/AbsListView.cs | Keeps adapter/receiver alive across SetAdapter JNI calls. |
| src/Mono.Android/Android.Runtime/XmlReaderPullParser.cs | Keeps parser-reader wrappers alive across NewLocalRef(handle) creation. |
| src/Mono.Android/Android.Runtime/JNIEnv.cs | Keeps IJavaObject alive across local-ref creation; keeps array elements alive during copy/set into Java arrays. |
| src/Mono.Android/Android.Runtime/JavaObject.cs | Consolidates legacy GetHandle(object) conversion through JavaConvert.ToLocalJniHandle. |
| src/Mono.Android/Android.Runtime/AndroidEnvironment.cs | Keeps throwable alive across JNIEnv.Throw(Handle). |
Use an explicit marshaling failure when ArrayAdapter creation returns null. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e6105414-1c2c-4696-8d01-6507bda3e4be
Use a derived Java peer because Java.Lang.Object does not expose a public constructor. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e6105414-1c2c-4696-8d01-6507bda3e4be
Derive the finalizable owner directly from Java.Lang.Object so Android packaging accepts it as an IJavaObject. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e6105414-1c2c-4696-8d01-6507bda3e4be
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
JNIEnv.ToLocalJniHandle()with concurrent GC pressureJavaObject.GetHandle()conversion throughJavaConvertFixes #5405
Testing