Add GC.KeepAlive to ComposeBridges JNI calls#11
Merged
Conversation
Wrap every JNIEnv.CallStaticVoidMethod in ComposeBridges.cs in a try/finally that calls GC.KeepAlive on each managed parameter whose .Handle was read into a JValue (lambdas, the composer, optional slot wrappers). This matches what dotnet/java-interop emits for bound members. Without it, once .Handle is read the JIT can consider the managed wrapper dead and a GC during the JNI call can finalize it and invalidate the underlying handle. Also document the pattern as rule #7 under the JNI-bridges section of .github/copilot-instructions.md so future bridges get it right. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the raw JNI bridge layer (ComposeBridges.cs) by ensuring managed wrapper objects (lambdas and the IComposer) remain alive across JNIEnv.CallStaticVoidMethod invocations, matching the lifetime-safety pattern emitted by dotnet/java-interop. This reduces the risk of stale/invalid JNI handles under GC pressure.
Changes:
- Wrapped each
JNIEnv.CallStaticVoidMethodintry/finallyand addedGC.KeepAlive(...)for every managed parameter whose.Handleis captured into aJValue. - Folded the
GC.KeepAlive(...)calls into existingtry/finallyblocks where stringDeleteLocalRefcleanup already existed (e.g.,Text,InvokeTextField). - Documented the required
try/finally+GC.KeepAliveJNI-bridge pattern in.github/copilot-instructions.mdto prevent future regressions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/ComposeNet.Compose/ComposeBridges.cs |
Adds try/finally wrappers and GC.KeepAlive(...) calls around JNI invocations to prevent premature finalization of managed wrappers during JNI calls. |
.github/copilot-instructions.md |
Documents the required JNI-bridge lifetime-safety pattern (matching dotnet/java-interop) for future bridge additions. |
jonathanpeppers
added a commit
that referenced
this pull request
Jun 3, 2026
Mirror the pattern from PR #11 (bfb108d) — wrap each JNIEnv.CallStatic*Method invocation in try/finally and GC.KeepAlive every managed wrapper whose .Handle was read into a JValue. Once the handle is in the JValue the JIT can consider the wrapper dead, and a GC mid-JNI-call would finalize it and invalidate the handle. Applied to: ModalBottomSheet, BottomSheetScaffold, DatePickerDialog, DatePicker, TimePicker, TimePickerDialog, TooltipBox, RememberDatePickerState, RememberTimePickerState, RememberTooltipState, RememberPlainTooltipPositionProvider. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
The raw-JNI bridges in
ComposeBridges.cswere missing theGC.KeepAlivepattern thatdotnet/java-interopemits for every bound member. Once((Java.Lang.Object)obj).Handleis read into aJValue, the JIT considers the managed wrapper dead, so a GC during the JNI call can finalize it and invalidate the underlying handle. In practice it usually doesn't crash because the lambda wrappers are referenced fromComposableLambda*/the call stack and Compose calls are short, but under GC pressure it's exactly the kind of bug that surfaces as a "stale local reference" JNI error.What changed
Every bridge (
Text,Button,IconButton,FloatingActionButton,Surface,TextField/OutlinedTextField,AlertDialog,Card,AssistChip/FilterChip/InputChip/SuggestionChip,NavigationBar/NavigationBarItem,NavigationRail/NavigationRailItem) now wrapsJNIEnv.CallStaticVoidMethodin atry { ... } finally { GC.KeepAlive(...); }block, with oneKeepAliveper managed parameter whose.Handlewas read into aJValue. The bridges that already had atry/finallytoDeleteLocalRefaJNIEnv.NewStringref (Text,InvokeTextField) fold the newKeepAlivecalls into that existing finally rather than nesting a second one.GC.KeepAlive(null)is a no-op so the optional slot wrappers inAlertDialog, the chips, and the nav items are kept alive unconditionally without a null check.Instructions update
Added rule #7 under the JNI-bridges section of
.github/copilot-instructions.mddocumenting thetry/finally+GC.KeepAliveshape, the rationale (matchesdotnet/java-interopoutput), and a copy-paste pattern, so future bridges get it right from the first commit.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com