-
Notifications
You must be signed in to change notification settings - Fork 578
Root [Export] callback delegates to prevent GC on CoreCLR #10997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9d61050
6678c28
414b356
abccc78
f18729b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -106,5 +106,94 @@ protected override void OnCreate (Bundle bundle) | |
| Assert.True (b.Uninstall (proj), "Project should have uninstalled."); | ||
| } | ||
| } | ||
|
|
||
| [Test] | ||
| public void ExportedMembersSurviveGarbageCollection ( | ||
| [Values] bool isRelease, | ||
| [Values] AndroidRuntime runtime) | ||
| { | ||
| if (runtime == AndroidRuntime.NativeAOT) { | ||
| Assert.Ignore ("NativeAOT does not support Mono.Android.Export"); | ||
| } | ||
| if (IgnoreUnsupportedConfiguration (runtime, release: isRelease)) { | ||
| return; | ||
| } | ||
|
|
||
| AssertCommercialBuild (); | ||
| var proj = new XamarinAndroidApplicationProject (packageName: PackageUtils.MakePackageName (runtime)) { | ||
| IsRelease = isRelease, | ||
| References = { | ||
| new BuildItem.Reference ("Mono.Android.Export"), | ||
| }, | ||
| }; | ||
| proj.SetRuntime (runtime); | ||
| proj.Sources.Add (new BuildItem.Source ("ContainsExportedMethods.cs") { | ||
| TextContent = () => @"using System; | ||
| using Java.Interop; | ||
|
|
||
| namespace UnnamedProject { | ||
| class ContainsExportedMethods : Java.Lang.Object { | ||
| [Export] | ||
| public void Exported () | ||
| { | ||
| Console.WriteLine (""# ExportedCallbackInvoked""); | ||
| } | ||
| } | ||
| } | ||
| ", | ||
| }); | ||
| proj.MainActivity = @"using System; | ||
| using Android.App; | ||
| using Android.OS; | ||
| using Android.Runtime; | ||
|
|
||
| namespace UnnamedProject | ||
| { | ||
| [Activity (Label = ""UnnamedProject"", MainLauncher = true, Icon = ""@drawable/icon"")] | ||
| public class MainActivity : Activity { | ||
| protected override void OnCreate (Bundle bundle) | ||
| { | ||
| base.OnCreate (bundle); | ||
|
|
||
| var foo = new ContainsExportedMethods (); | ||
|
|
||
| // Force GC to collect any unrooted delegates | ||
| for (int i = 0; i < 10; i++) { | ||
| GC.Collect (); | ||
| GC.WaitForPendingFinalizers (); | ||
| } | ||
|
|
||
| // Invoke the [Export] method through JNI (Java -> native delegate -> C#) | ||
| // This path crashes with SIGABRT if the delegate was garbage collected | ||
| IntPtr klass = JNIEnv.GetObjectClass (foo.Handle); | ||
| IntPtr methodId = JNIEnv.GetMethodID (klass, ""Exported"", ""()V""); | ||
| JNIEnv.CallVoidMethod (foo.Handle, methodId); | ||
|
|
||
| Console.WriteLine (""# ExportCallbackSurvivedGC""); | ||
| } | ||
| } | ||
| }"; | ||
| proj.SetAndroidSupportedAbis (DeviceAbi); | ||
| proj.SetDefaultTargetDevice (); | ||
| using (var b = CreateApkBuilder ()) { | ||
| b.LatestTargetFrameworkVersion (out string apiLevel); | ||
| proj.SupportedOSPlatformVersion = "24.0"; | ||
| 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>"; | ||
|
Comment on lines
+181
to
+186
|
||
| Assert.True (b.Install (proj), "Project should have installed."); | ||
| RunProjectAndAssert (proj, b, doNotCleanupOnUpdate: true); | ||
| Assert.True (WaitForActivityToStart (proj.PackageName, "MainActivity", | ||
| Path.Combine (Root, b.ProjectDirectory, "logcat.log"), 30), "Activity should have started."); | ||
| string expectedLogcatOutput = "ExportCallbackSurvivedGC"; | ||
| Assert.IsTrue (MonitorAdbLogcat ((line) => { | ||
| return line.Contains (expectedLogcatOutput); | ||
| }, Path.Combine (Root, b.ProjectDirectory, "startup-logcat.log"), 45), $"Output did not contain {expectedLogcatOutput}!"); | ||
| Assert.True (b.Uninstall (proj), "Project should have uninstalled."); | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prevent_delegate_gcis a shared mutableList<Delegate>which is appended to fromRegisterNativeMembers().List<T>is not thread-safe;RegisterNativeMembers()can be invoked concurrently for different types, and concurrentAdd()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.