Repository navigation
Root [Export] callback delegates to prevent GC on CoreCLR - #10997
Conversation
…CoreCLR Root dynamically-created [Export] callback delegates in a static list to prevent CoreCLR's GC from collecting them between JNI registration and first invocation. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/android/sessions/28ca1465-aa32-4295-9366-82c2a12b6a9b
|
@copilot add a new test scenario which will allow us to verify that this fix solves the problem |
Verifies [Export] delegates survive GC on CoreCLR by forcing aggressive GC cycles between delegate registration and JNI invocation, then calling the method through JNI (Java -> native delegate -> C#) to exercise the exact path that crashes when the delegate is collected. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/android/sessions/9c9c2212-3532-4407-b394-a1b28d2a2c62
Added
Without the fix, the JNI call at step 3 would crash with |
There was a problem hiding this comment.
Pull request overview
This PR addresses a CoreCLR-specific crash where dynamically-created [Export] JNI callback delegates can be garbage-collected after registration but before first invocation, leading to SIGABRT. It does so by holding strong references to these delegates in the runtime, and adds a device test to exercise the Java → native → managed callback path after forced GC.
Changes:
- Root dynamically created
[Export]delegates inAndroidRuntime.AndroidTypeManagerto prevent GC collection on CoreCLR. - Add a new MSBuild device integration test that forces aggressive GC and then invokes an
[Export]method through JNI to verify stability.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/Mono.Android/Android.Runtime/AndroidRuntime.cs |
Adds a static delegate root list and stores each dynamically-created [Export] callback delegate. |
tests/MSBuildDeviceIntegration/Tests/MonoAndroidExportTest.cs |
Adds a new device test that forces GC and calls an [Export] method via JNI, validating no crash and expected log output. |
| // [Export] callback delegates are created dynamically via DynamicCallbackCodeGenerator and are not | ||
| // cached in static fields (unlike non-[Export] connector delegates). Without rooting them here, | ||
| // CoreCLR's GC can collect them between JNI registration and first invocation, causing a crash. | ||
| static List<Delegate> prevent_delegate_gc = new List<Delegate> (); | ||
| static List<JniNativeMethodRegistration> sharedRegistrations = new List<JniNativeMethodRegistration> (); |
There was a problem hiding this comment.
prevent_delegate_gc is a shared mutable List<Delegate> which is appended to from RegisterNativeMembers(). List<T> is not thread-safe; RegisterNativeMembers() can be invoked concurrently for different types, and concurrent Add() calls can corrupt the list or throw, leading to app crashes. Consider protecting the collection with a lock, or switching to a thread-safe collection (e.g., ConcurrentBag<Delegate>), and ensure the chosen approach matches expected access patterns.
| proj.AndroidManifest = $@"<?xml version=""1.0"" encoding=""utf-8""?> | ||
| <manifest xmlns:android=""http://schemas.android.com/apk/res/android"" android:versionCode=""1"" android:versionName=""1.0"" package=""${proj.PackageName}""> | ||
| <uses-sdk android:targetSdkVersion=""{apiLevel}"" /> | ||
| <application android:label=""${{PROJECT_NAME}}""> | ||
| </application > | ||
| </manifest>"; |
There was a problem hiding this comment.
The manifest string uses package="${proj.PackageName}" inside an interpolated string, which will emit a leading $ in the package name (e.g., $com.example...). This produces an invalid manifest package and can cause build/install failures. Use package="{proj.PackageName}" (no $ inside), consistent with other device tests (e.g., DebuggingTest.cs).
|
@copilot address all review comments |
…t package name - Protect prevent_delegate_gc with a dedicated lock object since RegisterNativeMembers can be called concurrently for different types - Fix manifest package attribute in ExportedMembersSurviveGarbageCollection test: remove erroneous $ prefix that produced invalid package name Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/android/sessions/d78cdf0b-7890-4fe2-b5b3-8b10f5d6f0f4
Both review comments addressed in
|
Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/android/sessions/b3afee17-6668-4279-a4cd-28aa337d19ac
…reCLR (#10997) * Fix [Export] callback delegates not rooted after JNI registration on CoreCLR Root dynamically-created [Export] callback delegates in a static list to prevent CoreCLR's GC from collecting them between JNI registration and first invocation. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/android/sessions/28ca1465-aa32-4295-9366-82c2a12b6a9b * Add test: ExportedMembersSurviveGarbageCollection Verifies [Export] delegates survive GC on CoreCLR by forcing aggressive GC cycles between delegate registration and JNI invocation, then calling the method through JNI (Java -> native delegate -> C#) to exercise the exact path that crashes when the delegate is collected. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>
…reCLR (#10997) * Fix [Export] callback delegates not rooted after JNI registration on CoreCLR Root dynamically-created [Export] callback delegates in a static list to prevent CoreCLR's GC from collecting them between JNI registration and first invocation. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/android/sessions/28ca1465-aa32-4295-9366-82c2a12b6a9b * Add test: ExportedMembersSurviveGarbageCollection Verifies [Export] delegates survive GC on CoreCLR by forcing aggressive GC cycles between delegate registration and JNI invocation, then calling the method through JNI (Java -> native delegate -> C#) to exercise the exact path that crashes when the delegate is collected. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>
[Export]callback delegates created byCreateDynamicCallback()are only referenced by a localJniNativeMethodRegistration[]array passed toRegisterNatives(). Once the array goes out of scope, CoreCLR's GC collects the delegates, causingSIGABRTon first invocation. MonoVM was unaffected because it uses native trampolines rather than managed delegates.Non-
[Export]delegates are safe because generated connector code caches them in static fields (e.g.cb_onCreate).[Export]delegates have no such caching — they're created dynamically viaMono.Android.Export.dlland returned into a local.Fix
List<Delegate>inAndroidTypeManagerto hold strong references to dynamically-created[Export]delegatesCreateDynamicCallback()returnsGrowth is bounded by the number of unique
[Export]methods in the app (registered once per type).Test
ExportedMembersSurviveGarbageCollectiondevice test inMonoAndroidExportTest.csthat verifies[Export]delegates survive GC on all runtimes by:[Export]methods (triggers delegate registration viaRegisterNativeMembers)GC.Collect+GC.WaitForPendingFinalizers)[Export]method through JNI (GetObjectClass→GetMethodID→CallVoidMethod) to exercise the Java → native delegate → C# pathWithout the fix, the JNI call at step 3 crashes with
SIGABRTon CoreCLR because the delegate backingn_Exportedwas collected.đź’ˇ You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.