fix: Pin CallbackDelegate with GCHandle to prevent GC collection on .… - #1363
Open
w7rus wants to merge 1 commit into
Open
fix: Pin CallbackDelegate with GCHandle to prevent GC collection on .…#1363w7rus wants to merge 1 commit into
w7rus wants to merge 1 commit into
Conversation
…NET 10 Linux On .NET 10 Linux, Marshal.GetFunctionPointerForDelegate no longer internally pins the delegate, causing the GC to collect CallbackDelegate instances while native code still holds function pointers to them. This results in crashes with "callback was made on a garbage collected delegate". Add GCHandle.Alloc in FunctionReference constructor to explicitly prevent collection. Free the handle in Remove() during intentional cleanup. Also fix BasePlugin.Dispose() modifying dictionaries during foreach iteration by snapshotting values with .ToList() first.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Environment
./cs2 -dedicated -insecure1.41.7.0 (build 2000875)counterstrikesharp/dotnet/shared/Microsoft.NETCore.App/10.0.3/)Description
After the latest CounterStrikeSharp update, the CS2 dedicated server crashes with a garbage collected delegate error. The crash occurs when native code attempts to invoke a managed callback (
OnMapEndlistener) whose delegate has already been collected by the GC. This did not happen prior to the update — the same plugin configuration was stable before.Crash Log
Analysis
A managed dump was captured from the live process using the bundled
createdumptool before the crash occurred:The dump was analyzed with
dotnet-dump analyzeusingsetclrpathto point to the bundled runtime.CallbackDelegate instances
The first delegate (
7fcf4804d9a8) has 2660 unique GC roots — properly pinned. The remaining delegates have only 4 GC roots each, meaning they are weakly held and eligible for garbage collection while native code still holds pointers to them.Tracing the weakly-rooted delegate to its source
Step 1 —
dumpdelegateon a weakly-rooted CallbackDelegate:Step 2 —
dumpobjon the FunctionReference wrapper:Step 3 —
dumpdelegateon the underlying_targetMethod:Step 4 —
dumpobjon the display class to find the listener type:Step 5 —
dumpobjon<>4__thisreveals the plugin:Root Cause
FunctionReferencenever pins itsCallbackDelegate(_nativeCallback) with aGCHandle. It relies solely on managed references through staticConcurrentDictionarymaps to keep the delegate alive. WhenMarshal.GetFunctionPointerForDelegateis called, the native side receives a raw function pointer, but the GC has no knowledge of this unmanaged reference.On .NET 8 (Linux),
Marshal.GetFunctionPointerForDelegateinternally maintained an implicit GCHandle on the delegate, which kept it alive regardless of managed reference strength. On .NET 10 (Linux), this internal pinning was removed — the runtime now strictly follows the documented contract: "You must manually keep the delegate from being collected by the garbage collector from managed code."As a result, between callback registration and invocation, the GC collects the
CallbackDelegateeven though theFunctionReferenceholding it is still in the static maps. The managed reference chain (ConcurrentDictionary→FunctionReference→_nativeCallback) is not sufficient under .NET 10's more aggressive GC on Linux.This was not happening before the .NET 10 upgrade (
492727e8) because the runtime itself was keeping delegates alive. The issue is Linux-only because the native-to-managed thunk mechanism and GC behavior differ between platforms.Secondary issue:
BasePlugin.Dispose()corrupts cleanupBasePlugin.Dispose()iteratesHandlers.Values,Listeners.Values, etc. inforeachloops. Eachsubscriber.Dispose()call modifies the underlying dictionary (viaRemove), causingInvalidOperationExceptionon the next iteration. This means only the first subscriber in each dictionary gets properly cleaned up — remaining handlers/listeners are never unhooked from native and theirFunctionReferenceentries are never removed. While this doesn't directly cause the GC crash (it actually preventsFunctionReference.Removefrom running), it leaves orphaned native hooks and leakedFunctionReferenceentries.Fix
1. Pin
CallbackDelegatewithGCHandleinFunctionReferenceAdded
GCHandle.Alloc(_nativeCallback)in the constructor to explicitly prevent GC collection of the delegate while native code holds a pointer to it. The handle is freed inRemove()when the callback is intentionally unregistered.2. Fix
BasePlugin.Dispose()dictionary mutation during iterationSnapshot each dictionary's values with
.ToList()before iterating, preventingInvalidOperationExceptionand ensuring all subscribers are properly cleaned up.Steps to Reproduce
player_connect_full,OnMapEnd)A callback was made on a garbage collected delegate of type 'CounterStrikeSharp.API!CounterStrikeSharp.API.Core.FunctionReference+CallbackDelegate::Invoke'Additional Notes
execstackissue (cannot enable executable stack as shared object requires: Invalid argument) also appeared on Fedora 42 and was resolved separately withexecstack -con the shared object.CallbackDelegatein the heap (7fcf4804d9a8) has 2660 roots and is stable — the issue is specific to listener-registered delegates, not all delegates.