Fix transaction finalizer causing LiteDB.LiteException/System.ObjectDisposedException (#2721)

* Updated the transaction finalizer to not try to release a lock that can never be successful

* Added test to ensure that finalizing a transaction does not throw an exception

Author: ejdre-vestas

LiteDB.Tests/Engine/Transactions_Tests.cs

@@ -17,7 +17,7 @@ namespace LiteDB.Tests.Engine
     public class Transactions_Tests
     {
         const int MIN_CPU_COUNT = 2;
-        
+
         [CpuBoundFact(MIN_CPU_COUNT)]
         public async Task Transaction_Write_Lock_Timeout()
         {
@@ -75,7 +75,7 @@ public async Task Transaction_Write_Lock_Timeout()
             }
         }
 
-        
+
         [CpuBoundFact(MIN_CPU_COUNT)]
         public async Task Transaction_Avoid_Dirty_Read()
         {
@@ -135,7 +135,7 @@ public async Task Transaction_Avoid_Dirty_Read()
                 await Task.WhenAll(ta, tb);
             }
         }
-       
+
 
         [CpuBoundFact(MIN_CPU_COUNT)]
         public async Task Transaction_Read_Version()
@@ -233,6 +233,19 @@ public void Test_Transaction_States()
             }
         }
 
+        [CpuBoundFact(MIN_CPU_COUNT)]
+        public void Test_Transaction_Finalizer()
+        {
+            var db = new LiteDatabase(new MemoryStream());
+            db.BeginTrans();
+
+            GC.Collect(0, GCCollectionMode.Forced);
+
+            // Finalizer should not throw exception
+            // If it does, it will be an unhandled exception
+            GC.WaitForPendingFinalizers();
+        }
+
 #if DEBUG || TESTING
         [Fact]
         public void Transaction_Rollback_Should_Skip_ReadOnly_Buffers_From_Safepoint()
@@ -339,9 +352,9 @@ public void Transaction_Rollback_Should_Discard_Writable_Dirty_Pages()
 
         private class BlockingStream : MemoryStream
         {
-            public readonly AutoResetEvent   Blocked       = new AutoResetEvent(false);
+            public readonly AutoResetEvent Blocked = new AutoResetEvent(false);
             public readonly ManualResetEvent ShouldUnblock = new ManualResetEvent(false);
-            public          bool             ShouldBlock;
+            public bool ShouldBlock;
 
             public override void Write(byte[] buffer, int offset, int count)
             {
@@ -358,10 +371,10 @@ public override void Write(byte[] buffer, int offset, int count)
         [CpuBoundFact(MIN_CPU_COUNT)]
         public void Test_Transaction_ReleaseWhenFailToStart()
         {
-            var    blockingStream             = new BlockingStream();
-            var    db                         = new LiteDatabase(blockingStream);
+            var blockingStream = new BlockingStream();
+            var db = new LiteDatabase(blockingStream);
             SetEngineTimeout(db, TimeSpan.FromMilliseconds(50));
-            Thread lockerThread               = null;
+            Thread lockerThread = null;
             try
             {
                 lockerThread = new Thread(() =>
@@ -432,11 +445,11 @@ private static void SetEngineTimeout(LiteDatabase database, TimeSpan timeout)
             var engine = GetLiteEngine(database);
 
             var headerField = typeof(LiteEngine).GetField("_header", BindingFlags.Instance | BindingFlags.NonPublic);
-            var header      = headerField?.GetValue(engine) ?? throw new InvalidOperationException("LiteEngine header not available.");
+            var header = headerField?.GetValue(engine) ?? throw new InvalidOperationException("LiteEngine header not available.");
             var pragmasProp = header.GetType().GetProperty("Pragmas", BindingFlags.Instance | BindingFlags.Public) ?? throw new InvalidOperationException("Engine pragmas not accessible.");
-            var pragmas     = pragmasProp.GetValue(header) ?? throw new InvalidOperationException("Engine pragmas not available.");
+            var pragmas = pragmasProp.GetValue(header) ?? throw new InvalidOperationException("Engine pragmas not available.");
             var timeoutProp = pragmas.GetType().GetProperty("Timeout", BindingFlags.Instance | BindingFlags.Public) ?? throw new InvalidOperationException("Timeout property not found.");
-            var setter      = timeoutProp.GetSetMethod(true) ?? throw new InvalidOperationException("Timeout setter not accessible.");
+            var setter = timeoutProp.GetSetMethod(true) ?? throw new InvalidOperationException("Timeout setter not accessible.");
 
             setter.Invoke(pragmas, new object[] { timeout });
         }

LiteDB/Engine/Services/TransactionMonitor.cs

@@ -1,8 +1,6 @@
 using System;
-using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
-using System.Runtime.InteropServices;
 using System.Threading;
 using static LiteDB.Constants;
 
@@ -105,9 +103,10 @@ public TransactionService GetTransaction(bool create, bool queryOnly, out bool i
         }
 
         /// <summary>
-        /// Release current thread transaction
+        /// Dispose and remove transaction from monitor
+        /// without releasing thread lock
         /// </summary>
-        public void ReleaseTransaction(TransactionService transaction)
+        public bool RemoveTransaction(TransactionService transaction)
         {
             // dispose current transaction
             transaction.Dispose();
@@ -123,8 +122,16 @@ public void ReleaseTransaction(TransactionService transaction)
                 _freePages += transaction.MaxTransactionSize;
 
                 // check if current thread contains more query transactions
-                keepLocked = _transactions.Values.Any(x => x.ThreadID == Environment.CurrentManagedThreadId);
+                return keepLocked = _transactions.Values.Any(x => x.ThreadID == Environment.CurrentManagedThreadId);
             }
+        }
+
+        /// <summary>
+        /// Release current thread transaction
+        /// </summary>
+        public void ReleaseTransaction(TransactionService transaction)
+        {
+            var keepLocked = RemoveTransaction(transaction);
 
             // unlock thread-transaction only if there is no more transactions
             if (keepLocked == false)
@@ -150,7 +157,7 @@ public TransactionService GetThreadTransaction()
         {
             lock (_transactions)
             {
-                return 
+                return
                     _slot.Value ??
                     _transactions.Values.FirstOrDefault(x => x.ThreadID == Environment.CurrentManagedThreadId);
             }
@@ -191,7 +198,7 @@ private int GetInitialSize()
         /// </summary>
         private bool TryExtend(TransactionService trans)
         {
-            lock(_transactions)
+            lock (_transactions)
             {
                 if (_freePages >= _initialSize)
                 {
@@ -211,7 +218,7 @@ private bool TryExtend(TransactionService trans)
         /// </summary>
         public bool CheckSafepoint(TransactionService trans)
         {
-            return 
+            return
                 trans.Pages.TransactionSize >= trans.MaxTransactionSize &&
                 this.TryExtend(trans) == false;
         }
@@ -232,4 +239,4 @@ public void Dispose()
             }
         }
     }
-}
+}

LiteDB/Engine/Services/TransactionService.cs

@@ -1,9 +1,6 @@
 using System;
-using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
-using System.Runtime.InteropServices;
-using System.Threading;
 using static LiteDB.Constants;
 
 namespace LiteDB.Engine
@@ -444,7 +441,7 @@ protected virtual void Dispose(bool dispose)
             if (!dispose)
             {
                 // Remove transaction monitor's dictionary
-                _monitor.ReleaseTransaction(this);
+                _monitor.RemoveTransaction(this);
             }
         }
     }