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.