From 11d1653dbdf173362993309a8823ac3f87f01148 Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Wed, 3 Jun 2026 10:58:18 -0700 Subject: [PATCH] Fix repair of corrupt BlobSizes.sql and mount tolerance The repair job for BlobSizes.sql was unable to delete the corrupt database file on Windows because SQLite connection pooling kept the file handle open after the integrity check in HasIssue(). Two fixes: 1. SqliteDatabase.HasIssue: Use Pooling=False for integrity check connections so file handles are released immediately on dispose, allowing repair to delete the corrupt file. 2. BlobSizes.Initialize: Tolerate corrupt databases by catching SQLITE_CORRUPT and SQLITE_NOTADB errors, deleting the corrupt file (and WAL/SHM sidecars), and recreating a fresh database. This provides defense-in-depth since BlobSizes is a cache. Also remove SkipInCI from RepairFixesCorruptBlobSizesDatabase and add an assertion that repair actually cleans up the corrupt folder. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella --- GVFS/GVFS.Common/Database/SqliteDatabase.cs | 2 +- GVFS/GVFS.Common/Database/SqliteErrorCodes.cs | 15 ++++ .../MultiEnlistmentTests/SharedCacheTests.cs | 8 +- .../GVFS.Virtualization/BlobSize/BlobSizes.cs | 79 ++++++++++++------- 4 files changed, 73 insertions(+), 31 deletions(-) create mode 100644 GVFS/GVFS.Common/Database/SqliteErrorCodes.cs diff --git a/GVFS/GVFS.Common/Database/SqliteDatabase.cs b/GVFS/GVFS.Common/Database/SqliteDatabase.cs index 0416cec80..8cd9ac6c8 100644 --- a/GVFS/GVFS.Common/Database/SqliteDatabase.cs +++ b/GVFS/GVFS.Common/Database/SqliteDatabase.cs @@ -21,7 +21,7 @@ public static bool HasIssue(string databasePath, PhysicalFileSystem filesystem, try { - string sqliteConnectionString = CreateConnectionString(databasePath); + string sqliteConnectionString = $"data source={databasePath};Pooling=False"; using (SqliteConnection integrityConnection = new SqliteConnection(sqliteConnectionString)) { integrityConnection.Open(); diff --git a/GVFS/GVFS.Common/Database/SqliteErrorCodes.cs b/GVFS/GVFS.Common/Database/SqliteErrorCodes.cs new file mode 100644 index 000000000..2ed11d79a --- /dev/null +++ b/GVFS/GVFS.Common/Database/SqliteErrorCodes.cs @@ -0,0 +1,15 @@ +namespace GVFS.Common.Database +{ + /// + /// SQLite result codes used for error classification. + /// See https://www.sqlite.org/rescode.html + /// + public static class SqliteErrorCodes + { + /// SQLITE_CORRUPT (11) — database disk image is malformed + public const int Corrupt = 11; + + /// SQLITE_NOTADB (26) — file is not a database + public const int NotADatabase = 26; + } +} diff --git a/GVFS/GVFS.FunctionalTests/Tests/MultiEnlistmentTests/SharedCacheTests.cs b/GVFS/GVFS.FunctionalTests/Tests/MultiEnlistmentTests/SharedCacheTests.cs index 8837c6660..afd235bae 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/MultiEnlistmentTests/SharedCacheTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/MultiEnlistmentTests/SharedCacheTests.cs @@ -60,7 +60,6 @@ public void SecondCloneDoesNotDownloadAdditionalObjects() } [TestCase] - [SkipInCI("Product bug: repair does not fully restore corrupt BlobSizes.sql — mount crashes after repair")] public void RepairFixesCorruptBlobSizesDatabase() { GVFSFunctionalTestEnlistment enlistment = this.CloneAndMountEnlistment(); @@ -74,9 +73,12 @@ public void RepairFixesCorruptBlobSizesDatabase() blobSizesDbPath.ShouldBeAFile(this.fileSystem); this.fileSystem.WriteAllText(blobSizesDbPath, "0000"); - // GVFS now tolerates corrupt blob sizes DB on mount (recreates - // in-memory), but repair should still fix the underlying file. + // Repair should detect and fix the corrupt database enlistment.Repair(confirm: true); + + // Verify repair actually cleaned up the corrupt file + blobSizesRoot.ShouldNotExistOnDisk(this.fileSystem); + enlistment.MountGVFS(); } diff --git a/GVFS/GVFS.Virtualization/BlobSize/BlobSizes.cs b/GVFS/GVFS.Virtualization/BlobSize/BlobSizes.cs index a4d59f316..d4eb621a0 100644 --- a/GVFS/GVFS.Virtualization/BlobSize/BlobSizes.cs +++ b/GVFS/GVFS.Virtualization/BlobSize/BlobSizes.cs @@ -54,6 +54,54 @@ public virtual void Initialize() string folderPath = Path.GetDirectoryName(this.databasePath); this.fileSystem.CreateDirectory(folderPath); + try + { + this.InitializeDatabase(); + } + catch (SqliteException ex) when (ex.SqliteErrorCode == SqliteErrorCodes.Corrupt || ex.SqliteErrorCode == SqliteErrorCodes.NotADatabase) + { + EventMetadata metadata = this.CreateEventMetadata(ex); + metadata.Add("SqliteErrorCode", ex.SqliteErrorCode); + this.tracer.RelatedWarning(metadata, $"{nameof(BlobSizes)}.{nameof(this.Initialize)}: database corrupt, deleting and recreating"); + + SqliteConnection.ClearAllPools(); + this.DeleteDatabaseFiles(); + this.InitializeDatabase(); + } + + this.flushDataThread = new Thread(this.FlushDbThreadMain); + this.flushDataThread.IsBackground = true; + this.flushDataThread.Start(); + } + + public virtual void Shutdown() + { + this.isStopping = true; + this.wakeUpFlushThread.Set(); + this.flushDataThread.Join(); + } + + public virtual void AddSize(Sha1Id sha, long size) + { + this.queuedSizes.Enqueue(new BlobSize(sha, size)); + } + + public virtual void Flush() + { + this.wakeUpFlushThread.Set(); + } + + public void Dispose() + { + if (this.wakeUpFlushThread != null) + { + this.wakeUpFlushThread.Dispose(); + this.wakeUpFlushThread = null; + } + } + + private void InitializeDatabase() + { using (SqliteConnection connection = new SqliteConnection(this.sqliteConnectionString)) { connection.Open(); @@ -125,36 +173,13 @@ public virtual void Initialize() createTableCommand.ExecuteNonQuery(); } } - - this.flushDataThread = new Thread(this.FlushDbThreadMain); - this.flushDataThread.IsBackground = true; - this.flushDataThread.Start(); } - public virtual void Shutdown() + private void DeleteDatabaseFiles() { - this.isStopping = true; - this.wakeUpFlushThread.Set(); - this.flushDataThread.Join(); - } - - public virtual void AddSize(Sha1Id sha, long size) - { - this.queuedSizes.Enqueue(new BlobSize(sha, size)); - } - - public virtual void Flush() - { - this.wakeUpFlushThread.Set(); - } - - public void Dispose() - { - if (this.wakeUpFlushThread != null) - { - this.wakeUpFlushThread.Dispose(); - this.wakeUpFlushThread = null; - } + this.fileSystem.TryDeleteFile(this.databasePath); + this.fileSystem.TryDeleteFile(this.databasePath + "-wal"); + this.fileSystem.TryDeleteFile(this.databasePath + "-shm"); } private void FlushDbThreadMain()