diff --git a/src/LightningDB.Tests/FinalizerSafetyTests.cs b/src/LightningDB.Tests/FinalizerSafetyTests.cs new file mode 100644 index 0000000..329902e --- /dev/null +++ b/src/LightningDB.Tests/FinalizerSafetyTests.cs @@ -0,0 +1,49 @@ +using System.Diagnostics; +using System.IO; +using Shouldly; + +namespace LightningDB.Tests; + +// These scenarios run in a child process because the bug they reproduce is an +// unhandled exception on the finalizer thread, which is fatal to the process: +// there is no way to observe it in-process other than the whole test process dying. +// Before the fix, the child process aborts (exit code 134) instead of printing "OK". +public class FinalizerSafetyTests : TestBase +{ + private static (int exitCode, string output) RunSecondProcess(string mode, string path) + { + var otherProcessPath = Path.GetFullPath("SecondProcess.dll"); + using var process = new Process + { + StartInfo = new ProcessStartInfo + { + FileName = "dotnet", + Arguments = $"{otherProcessPath} {mode} {path}", + RedirectStandardError = true, + RedirectStandardOutput = true, + UseShellExecute = false, + CreateNoWindow = true, + WorkingDirectory = Directory.GetCurrentDirectory() + } + }; + + process.Start(); + var output = process.StandardOutput.ReadToEnd(); + process.WaitForExit(); + return (process.ExitCode, output); + } + + public void cursor_constructor_failure_must_not_crash_finalizer_thread() + { + var (exitCode, output) = RunSecondProcess("cursor-bad-txn", TempPath()); + exitCode.ShouldBe(0); + output.ShouldContain("OK"); + } + + public void transaction_constructor_failure_must_not_crash_finalizer_thread() + { + var (exitCode, output) = RunSecondProcess("txn-readers-full", TempPath()); + exitCode.ShouldBe(0); + output.ShouldContain("OK"); + } +} diff --git a/src/LightningDB/LightningCursor.cs b/src/LightningDB/LightningCursor.cs index 47467c6..9e274f9 100644 --- a/src/LightningDB/LightningCursor.cs +++ b/src/LightningDB/LightningCursor.cs @@ -41,7 +41,18 @@ internal LightningCursor(LightningDatabase db, LightningTransaction txn) if (txn == null) throw new ArgumentNullException(nameof(txn)); - mdb_cursor_open(txn._handle, db._handle, out _handle).ThrowOnError(); + try + { + mdb_cursor_open(txn._handle, db._handle, out _handle).ThrowOnError(); + } + catch + { + // A failed constructor must not leave a live finalizer: Database and + // Transaction are not yet assigned, so Dispose(false) would dereference + // them on the finalizer thread and abort the process. + GC.SuppressFinalize(this); + throw; + } Database = db; Transaction = txn; diff --git a/src/LightningDB/LightningTransaction.cs b/src/LightningDB/LightningTransaction.cs index be5c5c9..6ece32c 100644 --- a/src/LightningDB/LightningTransaction.cs +++ b/src/LightningDB/LightningTransaction.cs @@ -49,7 +49,19 @@ internal LightningTransaction(LightningEnvironment environment, LightningTransac State = LightningTransactionState.Ready; var parentHandle = parent?._handle ?? default(nint); - mdb_txn_begin(environment._handle, parentHandle, flags, out _handle).ThrowOnError(); + try + { + mdb_txn_begin(environment._handle, parentHandle, flags, out _handle).ThrowOnError(); + } + catch + { + // A failed constructor must not leave a live finalizer: mdb_txn_begin + // never allocated a transaction, so Dispose(false) would run against + // an environment that may already be closed by then, throwing on the + // finalizer thread and aborting the process. + GC.SuppressFinalize(this); + throw; + } _originalHandle = _handle; } diff --git a/src/SecondProcess/Program.cs b/src/SecondProcess/Program.cs index ae173cd..b9bc55f 100644 --- a/src/SecondProcess/Program.cs +++ b/src/SecondProcess/Program.cs @@ -1,4 +1,5 @@ using System; +using System.IO; using System.Linq; using System.Text; using LightningDB; @@ -9,6 +10,16 @@ class Program { static void Main(string[] args) { + switch (args.First()) + { + case "cursor-bad-txn": + ReproduceCursorConstructorFinalizerCrash(args[1]); + return; + case "txn-readers-full": + ReproduceTransactionConstructorFinalizerCrash(args[1]); + return; + } + var name = args.First(); using var env = new LightningEnvironment(name); env.Open(EnvironmentOpenFlags.ReadOnly); @@ -23,4 +34,84 @@ static void Main(string[] args) Console.WriteLine(Encoding.UTF8.GetString(results)); } -} \ No newline at end of file + + // Poisons a write transaction with MDB_MAP_FULL, then calls CreateCursor on it so + // mdb_cursor_open fails with MDB_BAD_TXN inside the LightningCursor constructor. + // Before the fix, the half-built LightningCursor is still finalizer-registered and + // its finalizer dereferences the never-assigned Database field, throwing a + // NullReferenceException on the finalizer thread and aborting the process. + static void ReproduceCursorConstructorFinalizerCrash(string path) + { + Directory.CreateDirectory(path); + using var env = new LightningEnvironment(path) { MapSize = 64 * 1024 }; + env.Open(); + + using (var setup = env.BeginTransaction()) + { + using var setupDb = setup.OpenDatabase(configuration: new DatabaseConfiguration { Flags = DatabaseOpenFlags.Create }); + setup.Commit(); + } + + using var tx = env.BeginTransaction(); + using var db = tx.OpenDatabase(); + + var value = new byte[4096]; + MDBResultCode result; + var i = 0; + do + { + result = tx.Put(db, BitConverter.GetBytes(i++), value); + } while (result == MDBResultCode.Success); + + if (result != MDBResultCode.MapFull) + throw new InvalidOperationException($"Expected MDB_MAP_FULL while filling the map, got {result}"); + + try + { + tx.CreateCursor(db); + throw new InvalidOperationException("Expected CreateCursor to throw on the poisoned transaction"); + } + catch (LightningException) + { + // Expected: mdb_cursor_open returns MDB_BAD_TXN for a poisoned transaction. + } + + GC.Collect(); + GC.WaitForPendingFinalizers(); + + Console.WriteLine("OK"); + } + + // Exhausts the reader lock table (MaxReaders = 1) and then attempts a second + // read-only BeginTransaction, so mdb_txn_begin fails with MDB_READERS_FULL inside + // the LightningTransaction constructor. Before the fix, the half-built + // LightningTransaction is still finalizer-registered; after the environment is + // disposed and GC runs, its finalizer throws "A transaction must be disposed + // before closing the environment" on the finalizer thread and aborts the process. + static void ReproduceTransactionConstructorFinalizerCrash(string path) + { + Directory.CreateDirectory(path); + var env = new LightningEnvironment(path, new EnvironmentConfiguration { MaxReaders = 1 }); + env.Open(); + + using (env.BeginTransaction(TransactionBeginFlags.ReadOnly)) + { + try + { + env.BeginTransaction(TransactionBeginFlags.ReadOnly); + throw new InvalidOperationException("Expected BeginTransaction to throw with the reader lock table full"); + } + catch (LightningException) + { + // Expected: mdb_txn_begin returns MDB_READERS_FULL. + } + } + + env.Dispose(); + + GC.Collect(); + GC.WaitForPendingFinalizers(); + + Console.WriteLine("OK"); + } +}