Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
+41 −24
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca7037f
Updated the transaction finalizer to not try to release a lock that c…
ejdre-vestas Oct 30, 2025
3959a69
Added test to ensure that finalizing a transaction does not throw an …
ejdre-vestas Oct 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 24 additions & 11 deletions LiteDB.Tests/Engine/Transactions_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down Expand Up @@ -75,7 +75,7 @@ public async Task Transaction_Write_Lock_Timeout()
}
}


[CpuBoundFact(MIN_CPU_COUNT)]
public async Task Transaction_Avoid_Dirty_Read()
{
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
{
Expand All @@ -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(() =>
Expand Down Expand Up @@ -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 });
}
Expand Down
25 changes: 16 additions & 9 deletions LiteDB/Engine/Services/TransactionMonitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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();
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling transaction.Dispose() within RemoveTransaction creates infinite recursion. The finalizer calls Dispose(false), which calls RemoveTransaction, which calls Dispose() again. This will result in a stack overflow. The RemoveTransaction method should not call transaction.Dispose() since it's already being called from within the transaction's own Dispose(bool) method.

Suggested change
transaction.Dispose();
// transaction.Dispose(); // Removed to prevent infinite recursion

Copilot uses AI. Check for mistakes.
Copy link
Author

@ejdre-vestas ejdre-vestas Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not change this behavior, but I think this comment is wrong anyway.
Calling the Dispose here will do

this.Dispose(true);
GC.SuppressFinalize(this);

The dispose boolean is true, and the finalizer will never be called because that is being suppressed.

Expand All @@ -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);
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to keepLocked is useless, since its value is never read.

Copilot uses AI. Check for mistakes.
Copy link
Author

@ejdre-vestas ejdre-vestas Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used in the original ReleaseTransaction method.

}
}

/// <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)
Expand All @@ -150,7 +157,7 @@ public TransactionService GetThreadTransaction()
{
lock (_transactions)
{
return
return
_slot.Value ??
_transactions.Values.FirstOrDefault(x => x.ThreadID == Environment.CurrentManagedThreadId);
}
Expand Down Expand Up @@ -191,7 +198,7 @@ private int GetInitialSize()
/// </summary>
private bool TryExtend(TransactionService trans)
{
lock(_transactions)
lock (_transactions)
{
if (_freePages >= _initialSize)
{
Expand All @@ -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;
}
Expand All @@ -232,4 +239,4 @@ public void Dispose()
}
}
}
}
}
5 changes: 1 addition & 4 deletions LiteDB/Engine/Services/TransactionService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -444,7 +441,7 @@ protected virtual void Dispose(bool dispose)
if (!dispose)
{
// Remove transaction monitor's dictionary
_monitor.ReleaseTransaction(this);
_monitor.RemoveTransaction(this);
}
}
}
Expand Down
Loading