From 7caa8bfd996a3706c9fd9e8c7826e4bed6aa95b6 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 17:06:54 -0400 Subject: [PATCH] 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. --- .../SiteLocalDbLegacyMigrator.cs | 27 +++++++++++++++++-- .../SiteLocalDbLegacyMigratorTests.cs | 23 ++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbLegacyMigrator.cs b/src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbLegacyMigrator.cs index 29f0eddd..ce040715 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbLegacyMigrator.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbLegacyMigrator.cs @@ -415,7 +415,7 @@ public static class SiteLocalDbLegacyMigrator /// private static List? 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; } + /// + /// Read-only connection string for a legacy file, with connection pooling DISABLED. + /// + /// + /// + /// Pooling=False is load-bearing, not a tuning choice. 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 (), and on Windows + /// against a file someone still holds open throws + /// ("being used by another process"). The whole migration then + /// faults out of AddZbLocalDb's factory and the node cannot boot at all. + /// + /// + /// 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. + /// + /// + internal static string LegacyReadOnlyConnectionString(string legacyPath) + => $"Data Source={legacyPath};Mode=ReadOnly;Pooling=False"; + /// /// Returns the subset of that the legacy table actually has, /// in the caller's order. An absent table yields an empty list rather than throwing. diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbLegacyMigratorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbLegacyMigratorTests.cs index 329b1011..c8fc2e7b 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbLegacyMigratorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbLegacyMigratorTests.cs @@ -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() {