AdvisoryLock strands a handle acquired after DisposeAsync — permanent 'idle in transaction' advisory-xact-lock session

[jeremydmiller]

AdvisoryLock.TryAttainLockAsync checks _disposed only at method entry, then stores the winning handle with no re-check (src/Weasel.Postgresql/AdvisoryLock.cs):

if (_disposed) return false;                      // entry check only (weasel#349)

var locker = _distributedLockProviders[lockId];
var handle = await locker.TryAcquireAsync(cancellationToken: token).ConfigureAwait(false);
if (handle is not null)
{
    _handles[lockId] = handle;                    // <-- may land after DisposeAsync drained _handles
    return true;
}

DisposeAsync sets _disposed and drains _handles. An acquire already past the entry check stores its handle into the drained dictionary, where nothing will ever dispose it. With TransactionalLockEnabled = true (Marten's default Events.UseAdvisoryLockTransaction), Medallion takes the lock via pg_try_advisory_xact_lock and holds a transaction open for the handle's lifetime, so the stranded handle leaves a backend idle in transaction with a granted advisory lock until the process exits. The connection is leased by the handle, so NpgsqlConnection.ClearAllPools() does not reclaim it, and disposing the NpgsqlDataSource only reclaims it when the consumer owns the data source.

Reported downstream as JasperFx/marten#5090, where the zombie transaction then permanently defeats Marten's high-water gap-liveness check (a pre-gap open transaction that never ends), stalling every subsequent daemon in the process.

Reproduction

Sweeping the dispose timing across the whole acquire, 120 iterations each, against AdvisoryLock directly:

probe	result
DisposeAsync racing an in-flight TryAttainLockAsync	120/120 stranded
cancelling an in-flight TryAttainLockAsync	0/120 — Medallion cleans up correctly

Each stranded backend shows state = idle in transaction, query = SELECT pg_catalog.pg_try_advisory_xact_lock($1), and one granted advisory lock in pg_locks.

var advisoryLock = new AdvisoryLock(dataSource, NullLogger.Instance, "probe",
    new AdvisoryLockOptions { TransactionalLockEnabled = true });

var task = advisoryLock.TryAttainLockAsync(lockId, CancellationToken.None);
await Task.Delay(TimeSpan.FromMicroseconds(micros));   // sweep 0 .. ~6ms
await advisoryLock.DisposeAsync();
await task;                                            // returns true -> handle stranded

The cancellation probe is worth recording as a negative result: the shutdown token is not the leak path, so a fix does not need to change how the token is passed.

Suggested fix

Serialize the handle store against the drain rather than relying on a pre-acquire flag check:

• take a lock around every _handles mutation, and set _disposed inside that same lock in DisposeAsync;
• after an acquire completes, store the handle under the lock — if _disposed is already set, dispose the handle locally instead and return false.

Then a handle either lands before the drain (and is drained) or observes the disposal and disposes itself. No window.

Related: _handles is a plain Dictionary<int, PostgresDistributedLockHandle> mutated concurrently by the caller's poll loop, ReleaseLockAsync, and DisposeAsync with no synchronization at all, and read unsynchronized by HasLock. The same lock fixes that too.

Adjacent to the shutdown races in weasel#349 / weasel#353 / marten#4915, but distinct: those churn on an already-disposed data source, this one strands the winning handle.