GC.KeepAlive doesn't keep local delegate alive in mono (Xamarin.Android)

[trampster]

EDITED by @stephentoub on 1/20/2023:
The issue at this point tracks an analyzer that would flag use of GC.KeepAlive(local) usage in the context of C# iterators, async methods, and anywhere else the C# compiler does a significant method rewrite such that a "local" isn't actually local, e.g. where that local is lifted to a state machine and the use of GC.KeepAlive has little to no impact, e.g.

object o = new object();
yield return o;
GC.KeepAlive(o);

---

Description

According to Eric Lippert there is no need to pin a local delegate or keep a GCHandle when passing a delegate via PInvoke as long as you use GC.KeepAlive to keep the delegate alive for as long as the native code could call it. https://stackoverflow.com/a/5465380/78561

However while this seems to be true for the dotnet runtime in mono this does not work and we are getting crashes when the callback happens.

We have found that in mono we need to keep a Normal GCHandle to the delegate GC.KeepAlive does not seem to be enough.

This behaviour needs to be consistent across the mono/dotnet runtimes.

This issue was found when using Xamarin.Android

Reproduction Steps

var taskCompletionSource = new TaskCompletionSource();

MyDelegateType myDelegate = () => taskCompletionSource.SetResult(true);

NativeMethods.MyNativeMethod(myDelegate);

await taskCompletionSource.Task;

GC.KeepAlive(myDelegate);

Expected behavior

GC.KeepAlive should prevent the delegate from being collected until the delegate callback has happened.

Actual behavior

Crash, usually with NullReferenceException

Regression?

No response

Known Workarounds

Get a normal GCHandle to the delegate instead of using GC.KeepAlive

var taskCompletionSource = new TaskCompletionSource();

MyDelegateType myDelegate = () => taskCompletionSource.SetResult(true);

var gcHandle = GCHandle.Alloc(myDelegate, GCHandleType.Normal);

NativeMethods.MyNativeMethod(myDelegate);

await taskCompletionSource.Task;

gcHandle.Free();

Configuration

No response

Other information

No response

Tagging subscribers to this area: @dotnet/area-system-threading-tasks
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

According to Eric Lippert there is no need to pin a local delegate or keep a GCHandle when passing a delegate via PInvoke as long as you use GC.KeepAlive to keep the delegate alive for as long as the native code could call it. https://stackoverflow.com/a/5465380/78561

However while this seems to be true for the dotnet runtime in mono this does not work and we are getting crashes when the callback happens.

We have found that in mono we need to keep a Normal GCHandle to the delegate GC.KeepAlive does not seem to be enough.

This behaviour need to be consistent across the mono/dotnet runtimes.

This issue was found when using Xamarin.Android

Reproduction Steps

var taskCompletionSource = new TaskCompletionSource();

var myDelegate = () => taskCompletionSource.SetResult(true);

var taskCompletionSource = new TaskCompletionSource();

var myDelegate = () => taskCompletionSource.SetResult(true);

NativeMethods.MyNativeMethod(myDelegate);

await taskCompletionSource.Task;

NativeMethods.MyNativeMethod(myDelegate);

await taskCompletionSource.Task;

GC.KeepAlive(myDelegate);

Expected behavior

GC.KeepAlive should prevent the delegate from being collected until the delegate callback has happened.

Actual behavior

Crash, usually with NullReferenceException

Regression?

No response

Known Workarounds

Get a normal GCHandle to the delegate instead of using GC.KeepAlive

var taskCompletionSource = new TaskCompletionSource();

MyDelegateType myDelegate = () => taskCompletionSource.SetResult(true);

var gcHandle = GCHandle.Alloc(myDelegate, GCHandleType.Normal);

var taskCompletionSource = new TaskCompletionSource();

var myDelegate = () => taskCompletionSource.SetResult(true);

NativeMethods.MyNativeMethod(myDelegate);

await taskCompletionSource.Task;

NativeMethods.MyNativeMethod(myDelegate);

await taskCompletionSource.Task;

gcHandle.Free();

Configuration

No response

Other information

No response

Author:	trampster
Assignees:	-
Labels:	area-System.Threading.Tasks, untriaged
Milestone:	-

Tagging subscribers to 'arch-android': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

According to Eric Lippert there is no need to pin a local delegate or keep a GCHandle when passing a delegate via PInvoke as long as you use GC.KeepAlive to keep the delegate alive for as long as the native code could call it. https://stackoverflow.com/a/5465380/78561

However while this seems to be true for the dotnet runtime in mono this does not work and we are getting crashes when the callback happens.

We have found that in mono we need to keep a Normal GCHandle to the delegate GC.KeepAlive does not seem to be enough.

This behaviour needs to be consistent across the mono/dotnet runtimes.

This issue was found when using Xamarin.Android

Reproduction Steps

var taskCompletionSource = new TaskCompletionSource();

var myDelegate = () => taskCompletionSource.SetResult(true);

var taskCompletionSource = new TaskCompletionSource();

var myDelegate = () => taskCompletionSource.SetResult(true);

NativeMethods.MyNativeMethod(myDelegate);

await taskCompletionSource.Task;

NativeMethods.MyNativeMethod(myDelegate);

await taskCompletionSource.Task;

GC.KeepAlive(myDelegate);

Expected behavior

GC.KeepAlive should prevent the delegate from being collected until the delegate callback has happened.

Actual behavior

Crash, usually with NullReferenceException

Regression?

No response

Known Workarounds

Get a normal GCHandle to the delegate instead of using GC.KeepAlive

var taskCompletionSource = new TaskCompletionSource();

MyDelegateType myDelegate = () => taskCompletionSource.SetResult(true);

var gcHandle = GCHandle.Alloc(myDelegate, GCHandleType.Normal);

var taskCompletionSource = new TaskCompletionSource();

var myDelegate = () => taskCompletionSource.SetResult(true);

NativeMethods.MyNativeMethod(myDelegate);

await taskCompletionSource.Task;

NativeMethods.MyNativeMethod(myDelegate);

await taskCompletionSource.Task;

gcHandle.Free();

Configuration

No response

Other information

No response

Author:	trampster
Assignees:	-
Labels:	area-System.Threading.Tasks, os-android, untriaged, runtime-mono
Milestone:	-

[steveisok]

/cc @SamMonoRT @BrzVlad

[BrzVlad]

@trampster I'm skeptical that this is a real issue. What makes you so sure that coreclr has different behavior ? In the post that you linked there is no mention of async methods, which completely change the apparent storage of locals so I expect GC.KeepAlive doesn't work because of this.

@trampster Also what happens if you put the keep alive before the await operation. I suspect the await is breaking the method so the keep alive probably no longer has any effect.

[stephentoub]

which completely change the apparent storage of locals so I expect GC.KeepAlive doesn't work because of this.

Right. The C# compiler rewrites iterators, async methods, etc., such that locals whose use spans a yield/await boundary are lifted to be fields, such that they're not actually locals. GC.KeepAlive will be a nop on such usage. It did not create a GC handle, just keeps an actual local referenced and alive until that point.

[trampster]

Thanks for your reply.

We run our integrations tests with dotnet 6 and GC.KeepAlive fixed a crash we where having there. The same code when run on Xamarin.Android crashes without a normal GCHandle. That particular example may not have had async in it, I can check when I'm back at work on monday.

If the locals are lifted to be fields then the field should keep the value alive (instance variables do no need GC.KeepAlive or GCHandles, again this is from Eric Lippert's post and therefore no Handle or keep alive should be required, However we know that with a normal GCHandle the code does not crash and without it it does.

Do guys know why a GCHandle would be required to keep the delegate alive with async in this situation on mono?

[stephentoub]

Async state machines are kept alive by the thing being awaited, e.g. the thing that will complete the awaited task. But in your case, that's a native call, which upon returning won't keep anything managed rooted, so as with most interop, you need to create a GC handle to keep that call's state alive, and that will in turn keep the state machine alive.