-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Description
Description
Currently, the C# GameSDK wrapper uses PInvoke marshaling to pass delegates as callback function pointers to the native library. According to the documentation, the passed delegate mustn't be garbage collected while the native code still stores the function pointer:
However, if the unmanaged function stores the delegate to use after the call completes, you must manually prevent garbage collection until the unmanaged function finishes with the delegate.
The C# GameSDK wrapper implicitly converts the method group of an internal middle layer function (which then calls the user's callback) to a delegate, and doesn't create the delegate explicitly (as an example, UpdateActivity
):
public void UpdateActivity(Activity activity, UpdateActivityHandler callback)
{
GCHandle wrapped = GCHandle.Alloc(callback);
Methods.UpdateActivity(MethodsPtr, ref activity, GCHandle.ToIntPtr(wrapped), >>>> UpdateActivityCallbackImpl <<<<);
}
(note that the UpdateActivityHandler
callback is not relevant - the implicit conversion of UpdateActivityCallbackImpl
to an UpdateActivityCallback
is the issue)
However, prior to C#11, the compiler did not cache this implicitly converted delegate - as such it becomes unreferenced while the native code still references the function pointer (see here). This then causes RunCallbacks
to crash if the garbage collector has ran since the call of the original function, usually resulting in a SEGFAULT, a ExecutionEngineException
, or other fatal runtime errors.
This bug has been reported multiple times already (almost exclusively on the old issue tracker), but doesn't seem to have been addressed/fixed yet - it was encountered while adding GameSDK support to our project:
- C# Example app crashes after Garbage Collecting gamesdk-and-dispatch#36
- ExecutionEngineException on Discord.RunCallbacks in C# (WPF) gamesdk-and-dispatch#120
- Misuse of GCHandle causes fatal GC errors #2577
Steps to Reproduce
Any example involving callbacks when compiled with a version of C# prior to C#11 will result in the issue occurring. To reliably ensure the crash happens, the GC can be explicitly triggered by calling GC.Collect();
before invoking RunCallbacks
(this isn't required though, in sufficiently complex applications the GC will trigger by itself).
Expected Behavior
The callbacks should be invoked like expected, and not result in a fatal runtime error. This can be achieved by explicitly creating and caching the delegate like this (applied to all methods passing such callback delegates, not just UpdateActivity
):
private static UpdateActivityCallback UpdateActivityCB = UpdateActivityCallbackImpl;
public void UpdateActivity(Activity activity, UpdateActivityHandler callback)
{
GCHandle wrapped = GCHandle.Alloc(callback);
Methods.UpdateActivity(MethodsPtr, ref activity, GCHandle.ToIntPtr(wrapped), UpdateActivityCB);
}
Current Behavior
The runtime crashes using one of a variety of fatal runtime errors, as reported here:
- C# Example app crashes after Garbage Collecting gamesdk-and-dispatch#36
- Error: fatal runtime error (
CallbackOnCollectedDelegate
)
- Error: fatal runtime error (
- ExecutionEngineException on Discord.RunCallbacks in C# (WPF) gamesdk-and-dispatch#120
- Error:
ExecutionEngineException
- Error:
- Misuse of GCHandle causes fatal GC errors #2577
- Error: fatal runtime error (
A callback was made on a garbage collected delegate
)
- Error: fatal runtime error (
- Switch to Discord Game SDK + rework the presence itself EverestAPI/Everest#543 (comment)
- Error: SEGFAULT
Screenshots/Videos
No response
Client and System Information
The issue was reproduced on the following setup for us, however any C# version below C#11 should encounter the same issue (mono has been reported to not encounter the issue, likely because of implementation quirks):
- Windows 10 x64
- .NET Framework 4.5.2
- C#9
- latest GameSDK wrapper (v3.2.1)