From 4d8d5a28a676abb953550b98aefcaeaa717adda7 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 3 Jun 2026 14:01:18 -0500 Subject: [PATCH] Add GC.KeepAlive to JNI bridges 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> --- .github/copilot-instructions.md | 27 ++++ src/ComposeNet.Compose/ComposeBridges.cs | 167 +++++++++++++++++++++-- 2 files changed, 180 insertions(+), 14 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index ee28b5a6..a078077f 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -122,6 +122,33 @@ Pattern, copied throughout the file: `try`/`finally` that calls `DeleteLocalRef`. 6. Pass managed objects as `((Java.Lang.Object)obj).Handle`. Use `IntPtr.Zero` for `null`. +7. **Always wrap `JNIEnv.CallStatic*Method` in `try { … } finally { … }` + and call `GC.KeepAlive(...)` on every managed parameter whose + `.Handle` was read into a `JValue` (lambdas, the composer, optional + slot wrappers — `null` is a no-op so unconditional `KeepAlive` on + nullable params is fine).** This matches what + `dotnet/java-interop` generates for bound members. Without it, the + JIT considers each managed wrapper dead the instant `.Handle` is + read, and a GC during the JNI call can finalize the wrapper and + invalidate the underlying handle. Combine with the `DeleteLocalRef` + `finally` for string args — one `try`/`finally` doing both is the + normal shape. + +Pattern: +```csharp +JValue* args = stackalloc JValue[N]; +// fill args ... +try +{ + JNIEnv.CallStaticVoidMethod(s_cls, s_method, args); +} +finally +{ + GC.KeepAlive(onClick); + GC.KeepAlive(content); + GC.KeepAlive(composer); +} +``` Keep these methods `internal` — user code never touches JNI directly. **Do not add new JNI bridges if the binding already exposes the diff --git a/src/ComposeNet.Compose/ComposeBridges.cs b/src/ComposeNet.Compose/ComposeBridges.cs index 70713235..c23dcc60 100644 --- a/src/ComposeNet.Compose/ComposeBridges.cs +++ b/src/ComposeNet.Compose/ComposeBridges.cs @@ -70,6 +70,7 @@ public static unsafe void Text(string text, IComposer composer) finally { JNIEnv.DeleteLocalRef(textRef); + GC.KeepAlive(composer); } } @@ -112,7 +113,16 @@ public static unsafe void Button(IFunction0 onClick, IFunction3 content, ICompos args[10] = new JValue(((Java.Lang.Object)composer).Handle); args[11] = new JValue(0); args[12] = new JValue(defaults); - JNIEnv.CallStaticVoidMethod(s_buttonClass, s_buttonMethod, args); + try + { + JNIEnv.CallStaticVoidMethod(s_buttonClass, s_buttonMethod, args); + } + finally + { + GC.KeepAlive(onClick); + GC.KeepAlive(content); + GC.KeepAlive(composer); + } } // androidx.compose.material3.IconButtonKt.IconButton(onClick, modifier, @@ -144,7 +154,16 @@ public static unsafe void IconButton(IFunction0 onClick, IFunction2 content, ICo args[6] = new JValue(((Java.Lang.Object)composer).Handle); args[7] = new JValue(0); args[8] = new JValue((int)IconButtonDefault.All); - JNIEnv.CallStaticVoidMethod(s_iconButtonClass, s_iconButtonMethod, args); + try + { + JNIEnv.CallStaticVoidMethod(s_iconButtonClass, s_iconButtonMethod, args); + } + finally + { + GC.KeepAlive(onClick); + GC.KeepAlive(content); + GC.KeepAlive(composer); + } } // androidx.compose.material3.FloatingActionButtonKt.FloatingActionButton-X-z6DiA( @@ -180,7 +199,16 @@ public static unsafe void FloatingActionButton(IFunction0 onClick, IFunction2 co args[8] = new JValue(((Java.Lang.Object)composer).Handle); args[9] = new JValue(0); args[10] = new JValue((int)FloatingActionButtonDefault.All); - JNIEnv.CallStaticVoidMethod(s_fabClass, s_fabMethod, args); + try + { + JNIEnv.CallStaticVoidMethod(s_fabClass, s_fabMethod, args); + } + finally + { + GC.KeepAlive(onClick); + GC.KeepAlive(content); + GC.KeepAlive(composer); + } } // androidx.compose.material3.SurfaceKt.Surface-T9BRK9s (non-interactive): @@ -214,7 +242,15 @@ public static unsafe void Surface(IFunction2 content, IComposer composer) args[8] = new JValue(((Java.Lang.Object)composer).Handle); args[9] = new JValue(0); args[10] = new JValue((int)SurfaceDefault.All); - JNIEnv.CallStaticVoidMethod(s_surfaceClass, s_surfaceMethod, args); + try + { + JNIEnv.CallStaticVoidMethod(s_surfaceClass, s_surfaceMethod, args); + } + finally + { + GC.KeepAlive(content); + GC.KeepAlive(composer); + } } // androidx.compose.material3.TextFieldKt.TextField (String overload): @@ -321,7 +357,20 @@ public static unsafe void AlertDialog( args[15] = new JValue(0); // $changed args[16] = new JValue(0); // $changed1 args[17] = new JValue(defaults); // $default - JNIEnv.CallStaticVoidMethod(s_alertDialogClass, s_alertDialogMethod, args); + try + { + JNIEnv.CallStaticVoidMethod(s_alertDialogClass, s_alertDialogMethod, args); + } + finally + { + GC.KeepAlive(onDismissRequest); + GC.KeepAlive(confirmButton); + GC.KeepAlive(dismissButton); + GC.KeepAlive(icon); + GC.KeepAlive(title); + GC.KeepAlive(text); + GC.KeepAlive(composer); + } } static unsafe void InvokeTextField(IntPtr cls, IntPtr method, string value, IFunction1 onValueChange, IComposer composer, int defaults) @@ -363,6 +412,8 @@ static unsafe void InvokeTextField(IntPtr cls, IntPtr method, string value, IFun finally { JNIEnv.DeleteLocalRef(valueRef); + GC.KeepAlive(onValueChange); + GC.KeepAlive(composer); } } @@ -397,7 +448,15 @@ public static unsafe void Card(IFunction3 content, IComposer composer) args[6] = new JValue(((Java.Lang.Object)composer).Handle); args[7] = new JValue(0); args[8] = new JValue((int)CardDefault.All); - JNIEnv.CallStaticVoidMethod(s_cardClass, s_cardMethod, args); + try + { + JNIEnv.CallStaticVoidMethod(s_cardClass, s_cardMethod, args); + } + finally + { + GC.KeepAlive(content); + GC.KeepAlive(composer); + } } // androidx.compose.material3.ChipKt.AssistChip: @@ -449,7 +508,18 @@ public static unsafe void AssistChip( args[12] = new JValue(0); // $changed args[13] = new JValue(0); // $changed1 args[14] = new JValue(defaults); // $default - JNIEnv.CallStaticVoidMethod(s_assistChipClass, s_assistChipMethod, args); + try + { + JNIEnv.CallStaticVoidMethod(s_assistChipClass, s_assistChipMethod, args); + } + finally + { + GC.KeepAlive(onClick); + GC.KeepAlive(label); + GC.KeepAlive(leadingIcon); + GC.KeepAlive(trailingIcon); + GC.KeepAlive(composer); + } } // androidx.compose.material3.ChipKt.FilterChip: @@ -503,7 +573,18 @@ public static unsafe void FilterChip( args[13] = new JValue(0); args[14] = new JValue(0); args[15] = new JValue(defaults); - JNIEnv.CallStaticVoidMethod(s_filterChipClass, s_filterChipMethod, args); + try + { + JNIEnv.CallStaticVoidMethod(s_filterChipClass, s_filterChipMethod, args); + } + finally + { + GC.KeepAlive(onClick); + GC.KeepAlive(label); + GC.KeepAlive(leadingIcon); + GC.KeepAlive(trailingIcon); + GC.KeepAlive(composer); + } } // androidx.compose.material3.ChipKt.InputChip: @@ -559,7 +640,19 @@ public static unsafe void InputChip( args[14] = new JValue(0); args[15] = new JValue(0); args[16] = new JValue(defaults); - JNIEnv.CallStaticVoidMethod(s_inputChipClass, s_inputChipMethod, args); + try + { + JNIEnv.CallStaticVoidMethod(s_inputChipClass, s_inputChipMethod, args); + } + finally + { + GC.KeepAlive(onClick); + GC.KeepAlive(label); + GC.KeepAlive(leadingIcon); + GC.KeepAlive(avatar); + GC.KeepAlive(trailingIcon); + GC.KeepAlive(composer); + } } // androidx.compose.material3.ChipKt.SuggestionChip: @@ -605,7 +698,17 @@ public static unsafe void SuggestionChip( args[10] = new JValue(((Java.Lang.Object)composer).Handle); args[11] = new JValue(0); args[12] = new JValue(defaults); - JNIEnv.CallStaticVoidMethod(s_suggestionChipClass, s_suggestionChipMethod, args); + try + { + JNIEnv.CallStaticVoidMethod(s_suggestionChipClass, s_suggestionChipMethod, args); + } + finally + { + GC.KeepAlive(onClick); + GC.KeepAlive(label); + GC.KeepAlive(icon); + GC.KeepAlive(composer); + } } // androidx.compose.material3.NavigationBarKt.NavigationBar-HsRjFd4: @@ -638,7 +741,15 @@ public static unsafe void NavigationBar(IFunction3 content, IComposer composer) args[6] = new JValue(((Java.Lang.Object)composer).Handle); args[7] = new JValue(0); args[8] = new JValue((int)NavigationBarDefault.All); - JNIEnv.CallStaticVoidMethod(s_navBarClass, s_navBarMethod, args); + try + { + JNIEnv.CallStaticVoidMethod(s_navBarClass, s_navBarMethod, args); + } + finally + { + GC.KeepAlive(content); + GC.KeepAlive(composer); + } } // androidx.compose.material3.NavigationBarKt.NavigationBarItem: @@ -691,7 +802,17 @@ public static unsafe void NavigationBarItem( args[10] = new JValue(((Java.Lang.Object)composer).Handle); args[11] = new JValue(0); args[12] = new JValue(defaults); - JNIEnv.CallStaticVoidMethod(s_navBarItemClass, s_navBarItemMethod, args); + try + { + JNIEnv.CallStaticVoidMethod(s_navBarItemClass, s_navBarItemMethod, args); + } + finally + { + GC.KeepAlive(onClick); + GC.KeepAlive(icon); + GC.KeepAlive(label); + GC.KeepAlive(composer); + } } // androidx.compose.material3.NavigationRailKt.NavigationRail-qi6gXK8: @@ -726,7 +847,15 @@ public static unsafe void NavigationRail(IFunction3 content, IComposer composer) args[6] = new JValue(((Java.Lang.Object)composer).Handle); args[7] = new JValue(0); args[8] = new JValue((int)NavigationRailDefault.All); - JNIEnv.CallStaticVoidMethod(s_navRailClass, s_navRailMethod, args); + try + { + JNIEnv.CallStaticVoidMethod(s_navRailClass, s_navRailMethod, args); + } + finally + { + GC.KeepAlive(content); + GC.KeepAlive(composer); + } } // androidx.compose.material3.NavigationRailKt.NavigationRailItem: @@ -773,6 +902,16 @@ public static unsafe void NavigationRailItem( args[9] = new JValue(((Java.Lang.Object)composer).Handle); args[10] = new JValue(0); args[11] = new JValue(defaults); - JNIEnv.CallStaticVoidMethod(s_navRailItemClass, s_navRailItemMethod, args); + try + { + JNIEnv.CallStaticVoidMethod(s_navRailItemClass, s_navRailItemMethod, args); + } + finally + { + GC.KeepAlive(onClick); + GC.KeepAlive(icon); + GC.KeepAlive(label); + GC.KeepAlive(composer); + } } }