fix(localdb): disable SQLite pooling on legacy reads - a Windows site node could not boot
SiteLocalDbLegacyMigrator opens each pre-Phase-2 file read-only, copies its rows into
the consolidated site database, then renames the file so a later boot skips it.
Microsoft.Data.Sqlite POOLS connections, so disposing one returns it to the pool and
leaves the underlying sqlite3 handle - and the OS file handle - open. On Windows the
subsequent File.Move throws IOException ("being used by another process"), the
migration faults out of AddZbLocalDb's factory, and the site node does not start at
all. There is no partial-migration path: it is a hard boot failure, once, on the first
upgrade past LocalDb Phase 2.
Why it shipped: POSIX rename ignores open handles, so this cannot reproduce on Linux or
macOS. Every existing rename assertion in SiteLocalDbLegacyMigratorTests - including
LegacyTrackingRows_AreCopiedAndTheFileIsRenamed - passes with the bug fully present, and
the docker rig migrates cleanly. It was found by pre-flighting the wonder-app-vd03
upgrade against COPIES of that box's real site databases; an earlier pre-flight pass
with empty data directories had nothing to drain and passed clean.
Fix: both legacy read-only connection strings now go through one
LegacyReadOnlyConnectionString helper carrying Pooling=False.
The accompanying test is deliberately white-box. A behavioural assertion cannot
discriminate here on the platform this suite runs on, so it pins the connection string
instead; it is red without the fix.
Verified: Host.Tests 440/440, 0 warnings. The build deployed to wonder-app-vd03 carries
this change (as the then-uncommitted fix) and has been running since 2026-08-05.
This commit is contained in:
@@ -415,7 +415,7 @@ public static class SiteLocalDbLegacyMigrator
|
||||
/// </summary>
|
||||
private static List<object?[]>? ReadAll(string legacyPath, string sql, int expectedColumns)
|
||||
{
|
||||
using var connection = new SqliteConnection($"Data Source={legacyPath};Mode=ReadOnly");
|
||||
using var connection = new SqliteConnection(LegacyReadOnlyConnectionString(legacyPath));
|
||||
connection.Open();
|
||||
|
||||
using var cmd = connection.CreateCommand();
|
||||
@@ -450,11 +450,34 @@ public static class SiteLocalDbLegacyMigrator
|
||||
|
||||
private static SqliteConnection OpenLegacyReadOnly(string legacyPath)
|
||||
{
|
||||
var connection = new SqliteConnection($"Data Source={legacyPath};Mode=ReadOnly");
|
||||
var connection = new SqliteConnection(LegacyReadOnlyConnectionString(legacyPath));
|
||||
connection.Open();
|
||||
return connection;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Read-only connection string for a legacy file, with connection pooling DISABLED.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// <b>Pooling=False is load-bearing, not a tuning choice.</b> Microsoft.Data.Sqlite pools
|
||||
/// connections by default, so disposing one returns it to the pool and leaves the
|
||||
/// underlying sqlite3 handle — and therefore the OS file handle — open. Every migrate step
|
||||
/// finishes by renaming its legacy file (<see cref="MarkMigrated"/>), and on Windows
|
||||
/// <see cref="File.Move(string, string)"/> against a file someone still holds open throws
|
||||
/// <see cref="IOException"/> ("being used by another process"). The whole migration then
|
||||
/// faults out of <c>AddZbLocalDb</c>'s factory and the node cannot boot at all.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// This never shows up on Linux or macOS: POSIX rename does not care about open handles,
|
||||
/// so the docker rig and developer machines migrate cleanly. It reproduces on the first
|
||||
/// Windows site node that boots with a pre-Phase-2 database beside it — which is every
|
||||
/// Windows install being upgraded, exactly once.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
internal static string LegacyReadOnlyConnectionString(string legacyPath)
|
||||
=> $"Data Source={legacyPath};Mode=ReadOnly;Pooling=False";
|
||||
|
||||
/// <summary>
|
||||
/// Returns the subset of <paramref name="wanted"/> that the legacy table actually has,
|
||||
/// in the caller's order. An absent table yields an empty list rather than throwing.
|
||||
|
||||
@@ -228,6 +228,29 @@ public class SiteLocalDbLegacyMigratorTests : IDisposable
|
||||
Assert.True(File.Exists(trackingPath + ".migrated"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void LegacyReadsDisablePooling_OrTheRenameFailsOnWindows()
|
||||
{
|
||||
// Deliberately white-box, and deliberately NOT a behavioural test.
|
||||
//
|
||||
// Microsoft.Data.Sqlite pools connections, so disposing one returns it to the pool
|
||||
// and leaves the underlying sqlite3 handle — and the OS file handle — open. Every
|
||||
// migrate step then renames its legacy file. On Windows, File.Move against a file
|
||||
// someone still holds open throws IOException, the migration faults out of
|
||||
// AddZbLocalDb's factory, and the site node cannot boot at all.
|
||||
//
|
||||
// It cannot be reproduced from a behavioural assertion off Windows: POSIX rename does
|
||||
// not care about open handles, so every rename test above — including
|
||||
// LegacyTrackingRows_AreCopiedAndTheFileIsRenamed — passes on Linux and macOS with the
|
||||
// pooling bug fully present. That is exactly how it shipped and reached a production
|
||||
// Windows box. Asserting on the connection string is the only guard that holds on the
|
||||
// platform this suite actually runs on.
|
||||
var cs = SiteLocalDbLegacyMigrator.LegacyReadOnlyConnectionString(Path_("whatever.db"));
|
||||
|
||||
Assert.Contains("Pooling=False", cs, StringComparison.OrdinalIgnoreCase);
|
||||
Assert.Contains("Mode=ReadOnly", cs, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void LegacyEvents_GetDeterministicIds_NotFreshGuids()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user