Suppress finalization when LightningCursor/LightningTransaction construction fails

[elvogel]

Fix: constructor failures in LightningCursor / LightningTransaction crash the finalizer thread

Problem

LightningCursor and LightningTransaction are both IDisposable types with a
finalizer (~LightningCursor() / ~LightningTransaction()) that calls
Dispose(false). Their constructors call a native LMDB function and then call
.ThrowOnError() on the result:

// LightningCursor
mdb_cursor_open(txn._handle, db._handle, out _handle).ThrowOnError();

Database = db;
Transaction = txn;

// LightningTransaction
mdb_txn_begin(environment._handle, parentHandle, flags, out _handle).ThrowOnError();
_originalHandle = _handle;

If the native call fails, ThrowOnError() throws before the fields below it are
assigned. By that point the object has already been allocated and — because it
has a finalizer — is already finalizer-registered by the runtime, even though the
constructor never returns and the caller never receives a reference to it. The
object becomes eligible for collection as soon as the failed constructor call's
stack frame is gone, and its finalizer will still run later, operating on
partially-initialized state:

• LightningCursor.Dispose(bool) starts with Database.Environment.IsOpened.
  If construction failed, Database was never assigned and is null, so the
  finalizer throws a NullReferenceException.
• LightningTransaction.Dispose(bool) starts with
  if (!Environment.IsOpened) throw new InvalidOperationException("A transaction must be disposed before closing the environment");. Environment is assigned
  earlier in the constructor, so this doesn't NRE, but if the environment has
  since been closed (a very plausible sequence — the failed BeginTransaction
  call is usually followed by disposing the environment), the finalizer throws
  that InvalidOperationException instead.

An exception thrown on the finalizer thread is unhandled by definition — there is
no application code on that call stack that could catch it — and terminates the
process. In this specific case the practical symptom is that a try/catch around
a failed CreateCursor/BeginTransaction call looks like it handled the error
correctly, but the process later aborts anyway (or hangs, if a test host is
waiting on a process that has already died on its finalizer thread) once the GC
happens to collect the discarded object.

This was found in a downstream project's test suite, where it appeared as
process exit code 134 during otherwise-unrelated test runs (or a hang, if the
test runner has GC/finalization interleaved with output capture).

Reproduction

Two independent repros trigger the same class of bug through the two affected
constructors:

1. LightningCursor: open a small-MapSize environment, fill a write
   transaction until a Put returns MDB_MAP_FULL (poisoning the transaction),
   then call CreateCursor on that poisoned transaction. mdb_cursor_open
   returns MDB_BAD_TXN, ThrowOnError() throws, and the half-built
   LightningCursor is left finalizer-registered with Database == null.
   Forcing GC.Collect(); GC.WaitForPendingFinalizers(); afterward crashes the
   process.

2. LightningTransaction: open an environment with MaxReaders = 1, begin
   one read-only transaction to hold the only reader slot, then attempt a second
   read-only BeginTransaction. mdb_txn_begin returns MDB_READERS_FULL,
   ThrowOnError() throws, and the half-built LightningTransaction is left
   finalizer-registered. Disposing the environment and forcing
   GC.Collect(); GC.WaitForPendingFinalizers(); afterward crashes the process,
   because the finalizer's Environment.IsOpened check now fails.

Both are exercised by the included tests (see below), run in a child process
because an unhandled exception on the finalizer thread is fatal to the process
that hits it — there's no way to assert on it in-process other than observing
the whole process die.

Root cause

The constructors allocate an object with a finalizer, make a native call that can
fail, and only assign the rest of the object's fields — including the fields the
finalizer's Dispose(false) path depends on — after that call succeeds. On
failure, the exception propagates out of the constructor, but the object is
already subject to finalization with those fields left at their default null/
unassigned values, which the finalizer logic does not guard against.

Notably, LightningEnvironment's two constructors already guard against exactly
this: if EnvironmentConfiguration.Configure throws after mdb_env_create
succeeds, they close the native handle and call GC.SuppressFinalize(this)
before rethrowing, with a comment explaining why:

catch
{
    // A failed constructor must not leave a live finalizer (it would throw
    // on the finalizer thread and abort the process) or a native handle leak.
    mdb_env_close(_handle);
    _handle = 0;
    _disposed = true;
    GC.SuppressFinalize(this);
    throw;
}

LightningCursor and LightningTransaction never received the same treatment.

Fix

Wrap the native call plus ThrowOnError() in each constructor in a try/catch
that calls GC.SuppressFinalize(this) before rethrowing, matching the existing
LightningEnvironment pattern. Neither constructor allocates a native resource
that needs explicit cleanup on this failure path (a failed mdb_cursor_open /
mdb_txn_begin does not hand back a handle that needs closing/aborting), so no
handle-cleanup logic is needed — only preventing the finalizer from ever running
against the half-built object.

See fix.patch.

Tests

See tests.patch. It adds:

• Two new modes to src/SecondProcess/Program.cs (the existing helper process
  used by MultiProcessTests) that reproduce each scenario above end-to-end,
  including the GC.Collect(); GC.WaitForPendingFinalizers(); call, and print
  OK if the process survives.
• src/LightningDB.Tests/FinalizerSafetyTests.cs, with two tests
  (cursor_constructor_failure_must_not_crash_finalizer_thread,
  transaction_constructor_failure_must_not_crash_finalizer_thread) that launch
  that helper process for each mode and assert exit code 0 and OK in
  stdout.

Both tests were run against this repo (net8.0, Linux x64):

• Before the fix (library constructors reverted, test harness kept): both
  new tests fail, each with the child process exiting with code 134,
  reproducing the reported crash exactly.
• After the fix: both new tests pass; the rest of the existing suite is
  unaffected by the change (147/161 passing locally, matching the pre-existing
  baseline — the 14 unrelated failures are EntryPointNotFoundExceptions from
  substituting an older native liblmdb.so build in this sandbox that predates
  some newer entry points such as mdb_txn_prepare/mdb_env_rollback, not a
  regression from this change).

Affected versions

0.21.0, 0.22.0, and 0.23.0 all contain this pattern in both constructors
(verified by decompiling the shipped LightningDB.dll for each version); 0.23.0
was additionally confirmed against upstream source at commit dc732e3
("Bumping to v0.23.0").