fix(secrets): make concurrent SQLite store initialisation safe; bump to 0.6.4 (unpublished) (scadaproj#25)

Several setups migrating one SQLite secret store at once (mxaccessgw's parallel test hosts on the
per-process store) failed intermittently on Windows with "cannot start a transaction within a
transaction" from SqliteSecretsStoreMigrator.MigrateAsync. That message means the BEGIN ran on a
handle that was already in a transaction: the pinned Microsoft.Data.Sqlite 10.0.7 can hand one
pooled handle to two owners (dotnet/efcore#39009, fixed in 10.0.13) and no 10.0.x refuses to
re-pool a handle whose rollback failed (dotnet/efcore#39082).

The migrator now serialises migrations of one store per process behind a gate keyed on the
normalised path, runs each attempt on a dedicated unpooled connection inside BEGIN IMMEDIATE, and
retries SQLITE_BUSY/SQLITE_LOCKED (another process past the busy timeout) with equal-jitter
exponential backoff. Store operations and the Microsoft.Data.Sqlite pin are unchanged.

Tests: concurrent-init (16 hosts x 20 rounds on fresh and existing stores, two path spellings,
16 AddZbSecrets containers running the pre-host migrator + hosted service), gate sharing/waiting,
BUSY retry past the 5 s busy timeout, transient-code classification.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
Joseph Doherty
2026-11-09 13:57:53 -05:00
parent 01c6dc1d02
commit f21f2e551d
7 changed files with 420 additions and 9 deletions
@@ -24,12 +24,21 @@ public sealed class SecretsSqliteConnectionFactory
_sqlitePath = sqlitePath;
}
/// <summary>
/// The store path made absolute and canonical (<c>..</c> segments resolved), so two factories
/// built from different spellings of one file agree on its identity. Used to key the
/// process-wide migration gate.
/// </summary>
internal string NormalizedPath => Path.GetFullPath(_sqlitePath);
/// <summary>
/// Creates an unopened SQLite connection (Mode=ReadWriteCreate). Prefer
/// <see cref="OpenConnectionAsync"/>, which also applies WAL journaling and the
/// busy timeout.
/// </summary>
public SqliteConnection CreateConnection()
public SqliteConnection CreateConnection() => CreateConnection(pooling: true);
private SqliteConnection CreateConnection(bool pooling)
{
string? directory = Path.GetDirectoryName(_sqlitePath);
@@ -42,7 +51,7 @@ public sealed class SecretsSqliteConnectionFactory
{
DataSource = _sqlitePath,
Mode = SqliteOpenMode.ReadWriteCreate,
Pooling = true,
Pooling = pooling,
DefaultTimeout = (int)BusyTimeout.TotalSeconds,
};
@@ -56,9 +65,25 @@ public sealed class SecretsSqliteConnectionFactory
/// </summary>
/// <param name="cancellationToken">Cancellation token for the operation.</param>
/// <returns>An opened and configured SQLite connection.</returns>
public async Task<SqliteConnection> OpenConnectionAsync(CancellationToken cancellationToken)
public Task<SqliteConnection> OpenConnectionAsync(CancellationToken cancellationToken) =>
OpenAsync(pooling: true, cancellationToken);
/// <summary>
/// Opens a connection that never comes from, and never returns to, the connection pool. The
/// migrator uses it so schema creation cannot be handed a pooled handle another owner is still
/// using, nor leave one behind (scadaproj#25). Microsoft.Data.Sqlite before 10.0.13 can hand one
/// pooled handle to two owners at once (dotnet/efcore#39009), and no 10.0.x release refuses to
/// pool a handle whose rollback failed mid-transaction (dotnet/efcore#39082); either surfaces as
/// "cannot start a transaction within a transaction" on the next <c>BEGIN</c>.
/// </summary>
/// <param name="cancellationToken">Cancellation token for the operation.</param>
/// <returns>An opened, configured, unpooled SQLite connection.</returns>
internal Task<SqliteConnection> OpenUnpooledConnectionAsync(CancellationToken cancellationToken) =>
OpenAsync(pooling: false, cancellationToken);
private async Task<SqliteConnection> OpenAsync(bool pooling, CancellationToken cancellationToken)
{
SqliteConnection connection = CreateConnection();
SqliteConnection connection = CreateConnection(pooling);
try
{
await connection.OpenAsync(cancellationToken).ConfigureAwait(false);
@@ -1,3 +1,4 @@
using System.Collections.Concurrent;
using System.Globalization;
using Microsoft.Data.Sqlite;
using ZB.MOM.WW.Secrets.Abstractions;
@@ -9,9 +10,33 @@ namespace ZB.MOM.WW.Secrets.Sqlite;
/// safe to run repeatedly. Refuses to run against a database whose on-disk version is
/// newer than this build supports.
/// </summary>
/// <remarks>
/// Safe when several initialisations of one store run at once (scadaproj#25 — mxaccessgw's parallel
/// test hosts migrating one per-process store failed on Windows with
/// "cannot start a transaction within a transaction"):
/// <list type="bullet">
/// <item>Migrations of one store inside one process are serialised by a process-wide gate keyed on the
/// normalised store path, however many factories, migrators or DI containers point at it.</item>
/// <item>Each attempt runs on a dedicated, unpooled connection inside <c>BEGIN IMMEDIATE</c>, so it takes the
/// write lock up front and can neither inherit nor leave behind a pooled handle in a transaction.</item>
/// <item>Contention with another process (<c>SQLITE_BUSY</c> / <c>SQLITE_LOCKED</c> after the busy timeout)
/// is retried on a fresh connection with jittered exponential backoff.</item>
/// </list>
/// </remarks>
public sealed class SqliteSecretsStoreMigrator(SecretsSqliteConnectionFactory connectionFactory)
: ISecretsStoreMigrator
{
/// <summary>Total attempts (first try plus retries) on SQLITE_BUSY / SQLITE_LOCKED.</summary>
internal const int MaxAttempts = 6;
private const int SqliteBusy = 5;
private const int SqliteLocked = 6;
private static readonly ConcurrentDictionary<string, SemaphoreSlim> Gates = new(
OperatingSystem.IsWindows() || OperatingSystem.IsMacOS()
? StringComparer.OrdinalIgnoreCase
: StringComparer.Ordinal);
/// <summary>Applies the schema migration to the secret store.</summary>
/// <param name="cancellationToken">Cancellation token.</param>
/// <exception cref="SecretStoreMigrationException">
@@ -19,11 +44,56 @@ public sealed class SqliteSecretsStoreMigrator(SecretsSqliteConnectionFactory co
/// </exception>
public async Task MigrateAsync(CancellationToken cancellationToken)
{
await using SqliteConnection connection =
await connectionFactory.OpenConnectionAsync(cancellationToken).ConfigureAwait(false);
SemaphoreSlim gate = GateFor(connectionFactory.NormalizedPath);
await gate.WaitAsync(cancellationToken).ConfigureAwait(false);
try
{
for (int attempt = 1; ; attempt++)
{
try
{
await MigrateOnceAsync(cancellationToken).ConfigureAwait(false);
return;
}
catch (SqliteException ex) when (IsTransient(ex) && attempt < MaxAttempts)
{
await Task.Delay(Backoff(attempt), cancellationToken).ConfigureAwait(false);
}
}
}
finally
{
gate.Release();
}
}
await using SqliteTransaction transaction =
(SqliteTransaction)await connection.BeginTransactionAsync(System.Data.IsolationLevel.Serializable, cancellationToken).ConfigureAwait(false);
/// <summary>
/// The gate serialising migrations of the store at <paramref name="normalizedPath"/>; exposed so a
/// test can prove two spellings of one path share it.
/// </summary>
internal static SemaphoreSlim GateFor(string normalizedPath) =>
Gates.GetOrAdd(normalizedPath, static _ => new SemaphoreSlim(1, 1));
internal static bool IsTransient(SqliteException ex) =>
(ex.SqliteErrorCode & 0xFF) is SqliteBusy or SqliteLocked;
// Equal-jitter exponential backoff: 100, 200, 400, 800, 1600 ms ceilings, at least half of each,
// so concurrent processes that collided once do not retry in lockstep.
private static TimeSpan Backoff(int attempt)
{
int ceiling = 100 * (1 << Math.Min(attempt - 1, 4));
return TimeSpan.FromMilliseconds((ceiling / 2) + Random.Shared.Next(ceiling / 2 + 1));
}
private async Task MigrateOnceAsync(CancellationToken cancellationToken)
{
await using SqliteConnection connection =
await connectionFactory.OpenUnpooledConnectionAsync(cancellationToken).ConfigureAwait(false);
// deferred: false => BEGIN IMMEDIATE: take the write lock before reading the version, so two
// migrators can never both read "no schema" and then race to create it.
await using SqliteTransaction transaction = connection.BeginTransaction(
System.Data.IsolationLevel.Serializable, deferred: false);
int existingVersion =
await ReadExistingSchemaVersionAsync(connection, transaction, cancellationToken).ConfigureAwait(false);