MySqlConnector.MySqlException in TransactionScope.Dispose caused by a deadlock creates a memory leak

[PatrickMNL]

Software versions
MySqlConnector version: 2.2.5
Server type (MySQL, MariaDB, Aurora, etc.) and version: 10.6.12-MariaDB-log
.NET version: 6.0

Describe the bug
When a deadlock is thrown by the SQL database, we catch and retry it as suggested by the exception. But before we retry, our using statement inside our try/catch block disposes of the then active TransactionScope. The dispose however is interrupted by another exception thrown by MySqlConnector, causing the TransactionScope to not be properly disposed and leaving behind a reference in the s_transactionConnections dictionary which will snowball into ever increasing memory leakage for each subsequently created transaction.

Exception

MySqlConnector.MySqlException (0x80004005): XA_RBDEADLOCK: Transaction branch was rolled back: deadlock was detected
   at MySqlConnector.Core.ResultSet.ReadResultSetHeaderAsync(IOBehavior ioBehavior) in /_/src/MySqlConnector/Core/ResultSet.cs:line 43
   at MySqlConnector.MySqlDataReader.ActivateResultSet(CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 130
   at MySqlConnector.MySqlDataReader.CreateAsync(CommandListPosition commandListPosition, ICommandPayloadCreator payloadCreator, IDictionary`2 cachedProcedures, IMySqlCommand command, CommandBehavior behavior, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 469
   at MySqlConnector.Core.CommandExecutor.ExecuteReaderAsync(IReadOnlyList`1 commands, ICommandPayloadCreator payloadCreator, CommandBehavior behavior, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/CommandExecutor.cs:line 56
   at MySqlConnector.MySqlCommand.ExecuteNonQueryAsync(IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlCommand.cs:line 296
   at MySqlConnector.MySqlCommand.ExecuteNonQuery() in /_/src/MySqlConnector/MySqlCommand.cs:line 107
   at MySqlConnector.Core.XaEnlistedTransaction.ExecuteXaCommand(String statement) in /_/src/MySqlConnector/Core/XaEnlistedTransaction.cs:line 46
   at MySqlConnector.Core.XaEnlistedTransaction.OnRollback(Enlistment enlistment) in /_/src/MySqlConnector/Core/XaEnlistedTransaction.cs:line 39
   at MySqlConnector.Core.EnlistedTransactionBase.System.Transactions.IEnlistmentNotification.Rollback(Enlistment enlistment) in /_/src/MySqlConnector/Core/EnlistedTransactionBase.cs:line 37
   at System.Transactions.VolatileEnlistmentAborting.EnterState(InternalEnlistment enlistment)
   at System.Transactions.TransactionStateAborted.EnterState(InternalTransaction tx)
   at System.Transactions.Transaction.Rollback()
   at System.Transactions.TransactionScope.InternalDispose()
   at System.Transactions.TransactionScope.Dispose()
<removed company specific part of the callstack>

Code sample
Not available, scenario should be easy enough to replicate and the exception holds all necessary information.

Expected behavior
The transaction dispose should preferably not be interrupted by exception, but in the case it has to, it should ensure that any static references bound to the transaction are still properly cleaned up.

Additional context
The reason why I consider this a 'severe' memory leak is the following;

Once the transaction is stuck inside the s_transactionConnections dictionary everything that's referenced by it is no longer eligible for garbage collection, this wouldn't be that bad if it was just the transaction, but in .NET adds data for every subsequently created transaction to those references which will then also never be eligible for garbage collection.

To be more specific, each transaction holds a reference to an InternalTransaction, which in term holds a reference to a Bucket, which has a reference to a BucketSet to which it belongs. Each BucketSet holds a weak-reference to it's previous BucketSet and a hard-reference to the next BucketSet. Any newly created Transaction will insert a new BucketSet into the chain of BucketSets ordered by an 'AbsoluteTimeout' value (hard referencing is done oldest->newest).

This means that our old BucketSet will keep newer BucketSets in memory forever, eventually causing an application that ran into a deadlock to inevitably run out of memory.

[bgrainger]

The transaction dispose should preferably not be interrupted by exception

Does #1318 seem like a reasonable fix for this issue?

[PatrickMNL]

Thanks for the quick reply @bgrainger! To answer your question, it does indeed seem like a reasonable fix to me 👍, it would definitely fix our scenario.

I'm just wondering if you want the try/catch in all scenario's and not just explicitly when calling the ExecuteXaCommand with "ROLLBACK" (or for simplicity only in the OnRollback method), but I'll leave that up to you to decide, I have too little understanding of the internals of the library/package.

[bgrainger]

Yes, that's a good callout; it probably makes more sense to have a targeted exception handler just in OnRollback. I'll make that change.

[PatrickMNL]

Sounds good @bgrainger, thanks 👍

[bgrainger]

Fixed in 2.2.6.

[kloarubeek]

Thanks! The coming days we'll test the new package to see if the memory leak is gone.

[kloarubeek]

Memory is really stable now, memory leak is solved. Thanks again!