Skip to content

Commit f020909

Browse files
authored
Heal tracking database if it can't open (#5724)
## Change When we fail to open the tracking database, rename it and create a new one. The initial `Open` requires two things to succeed: 1. SQLite considers this a valid database file 2. The metadata table exists and contains a schema version that we can read If an exception is thrown, we move the file (and the auxiliary journals if they exist) to new names (`installed.db` -> `installed-corrupted.db`) and create a new database.
1 parent fc36248 commit f020909

File tree

6 files changed

+141
-7
lines changed

6 files changed

+141
-7
lines changed

.github/actions/spelling/expect.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,7 @@ vns
582582
vsconfig
583583
vstest
584584
waitable
585+
wal
585586
wcex
586587
WDAG
587588
webpages

src/AppInstallerCLITests/PackageTrackingCatalog.cpp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,3 +233,49 @@ TEST_CASE("TrackingCatalog_Overlapping_ARP_Range", "[tracking_catalog]")
233233
REQUIRE(resultAfter.Matches[0].Package->GetAvailable()[0]->GetLatestVersion()->GetProperty(PackageVersionProperty::Version) ==
234234
manifest.Version);
235235
}
236+
237+
TEST_CASE("TrackingCatalog_Corrupt", "[tracking_catalog]")
238+
{
239+
TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
240+
INFO("Using temporary file named: " << tempFile.GetPath());
241+
242+
SourceDetails details;
243+
Manifest manifest;
244+
std::string relativePath;
245+
auto source = SimpleTestSetup(tempFile, details, manifest, relativePath);
246+
247+
SearchRequest request;
248+
request.Filters.emplace_back(PackageMatchField::Id, MatchType::Exact, manifest.Id);
249+
250+
std::filesystem::path catalogFile;
251+
252+
{
253+
// Add data to initial database
254+
PackageTrackingCatalog catalog = CreatePackageTrackingCatalogForSource(source);
255+
256+
SearchResult resultBefore = catalog.Search(request);
257+
REQUIRE(resultBefore.Matches.size() == 0);
258+
259+
catalog.RecordInstall(manifest, manifest.Installers[0], false);
260+
261+
SearchResult resultAfter = catalog.Search(request);
262+
REQUIRE(resultAfter.Matches.size() == 1);
263+
REQUIRE(resultAfter.Matches[0].Package->GetAvailable().size() == 1);
264+
265+
catalogFile = catalog.GetFilePath();
266+
}
267+
268+
{
269+
std::ofstream file{ catalogFile, std::ios_base::trunc };
270+
file << "Corrupted!";
271+
}
272+
273+
{
274+
// Open database again after "corruption"
275+
PackageTrackingCatalog catalog = CreatePackageTrackingCatalogForSource(source);
276+
277+
// Should not find anything in new database
278+
SearchResult resultBefore = catalog.Search(request);
279+
REQUIRE(resultBefore.Matches.size() == 0);
280+
}
281+
}

src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ namespace AppInstaller::Repository
1717
namespace
1818
{
1919
constexpr std::string_view c_PackageTrackingFileName = "installed.db";
20+
constexpr std::string_view c_PackageTrackingCorruptedFileName = "installed-corrupted.db";
2021

2122
std::string CreateNameForCPL(const std::string& pathName)
2223
{
@@ -31,19 +32,39 @@ namespace AppInstaller::Repository
3132
return result;
3233
}
3334

35+
// Call while holding the CrossProcessLock
36+
SQLiteIndex CreateOnlyTrackingIndex(const std::filesystem::path& trackingDB)
37+
{
38+
return SQLiteIndex::CreateNew(trackingDB.u8string(), SQLite::Version::Latest(), SQLiteIndex::CreateOptions::SupportPathless | SQLiteIndex::CreateOptions::DisableDependenciesSupport);
39+
}
40+
3441
// Call while holding the CrossProcessLock
3542
SQLiteIndex CreateOrOpenTrackingIndex(const std::filesystem::path& trackingDB)
3643
{
3744
if (!std::filesystem::exists(trackingDB))
3845
{
3946
std::filesystem::create_directories(trackingDB.parent_path());
40-
return SQLiteIndex::CreateNew(trackingDB.u8string(), SQLite::Version::Latest(), SQLiteIndex::CreateOptions::SupportPathless | SQLiteIndex::CreateOptions::DisableDependenciesSupport);
47+
return CreateOnlyTrackingIndex(trackingDB);
4148
}
4249
else
4350
{
44-
// TODO: Check schema version and upgrade as necessary when there is a relevant new schema.
45-
// Could write this all now but it will be better tested when there is a new schema.
46-
return SQLiteIndex::Open(trackingDB.u8string(), SQLiteIndex::OpenDisposition::ReadWrite);
51+
try
52+
{
53+
// TODO: Check schema version and upgrade as necessary when there is a relevant new schema.
54+
// Could write this all now but it will be better tested when there is a new schema.
55+
return SQLiteIndex::Open(trackingDB.u8string(), SQLiteIndex::OpenDisposition::ReadWrite);
56+
}
57+
catch(...)
58+
{
59+
LOG_CAUGHT_EXCEPTION_MSG("Exception opening tracking catalog");
60+
}
61+
62+
// Move existing database and create a new one
63+
std::filesystem::path destination{ trackingDB };
64+
destination.replace_filename(c_PackageTrackingCorruptedFileName);
65+
SQLite::SQLiteStorageBase::RenameSQLiteDatabase(trackingDB, destination, true);
66+
67+
return CreateOnlyTrackingIndex(trackingDB);
4768
}
4869
}
4970

@@ -205,11 +226,11 @@ namespace AppInstaller::Repository
205226
PackageTrackingCatalog::Version::~Version() = default;
206227

207228
PackageTrackingCatalog::Version::Version(PackageTrackingCatalog& catalog, std::shared_ptr<implementation>&& value) :
208-
m_catalog(catalog), m_implementation(std::move(value)) {}
229+
m_catalog(&catalog), m_implementation(std::move(value)) {}
209230

210231
void PackageTrackingCatalog::Version::SetMetadata(PackageVersionMetadata metadata, const Utility::NormalizedString& value)
211232
{
212-
auto& index = m_catalog.m_implementation->Source->GetIndex();
233+
auto& index = m_catalog->m_implementation->Source->GetIndex();
213234
index.SetMetadataByManifestId(m_implementation->Id, metadata, value);
214235
}
215236

@@ -287,6 +308,13 @@ namespace AppInstaller::Repository
287308
}
288309
}
289310

311+
#ifndef AICLI_DISABLE_TEST_HOOKS
312+
std::filesystem::path PackageTrackingCatalog::GetFilePath() const
313+
{
314+
return m_implementation->Source->GetIndex().GetContextData().Get<Schema::Property::DatabaseFilePath>();
315+
}
316+
#endif
317+
290318
std::unique_ptr<ISourceFactory> PackageTrackingCatalogSourceFactory::Create()
291319
{
292320
return std::make_unique<PackageTrackingCatalogSourceFactoryImpl>();

src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66

77
#include <memory>
88

9+
#ifndef AICLI_DISABLE_TEST_HOOKS
10+
#include <filesystem>
11+
#endif
912

1013
namespace AppInstaller::Repository
1114
{
@@ -55,7 +58,7 @@ namespace AppInstaller::Repository
5558
struct implementation;
5659
Version(PackageTrackingCatalog& catalog, std::shared_ptr<implementation>&& value);
5760
std::shared_ptr<implementation> m_implementation;
58-
PackageTrackingCatalog& m_catalog;
61+
PackageTrackingCatalog* m_catalog;
5962
};
6063

6164
// Records an installation of the given package.
@@ -64,6 +67,11 @@ namespace AppInstaller::Repository
6467
// Records an uninstall of the given package.
6568
void RecordUninstall(const Utility::LocIndString& packageIdentifier);
6669

70+
#ifndef AICLI_DISABLE_TEST_HOOKS
71+
// Gets the path to the database file.
72+
std::filesystem::path GetFilePath() const;
73+
#endif
74+
6775
protected:
6876
// Creates or opens the tracking catalog for the given source.
6977
static PackageTrackingCatalog CreateForSource(const Source& source);

src/AppInstallerSharedLib/Public/winget/SQLiteStorageBase.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <winget/SQLiteVersion.h>
66
#include <winget/ManagedFile.h>
77

8+
#include <filesystem>
89
#include <mutex>
910

1011
namespace AppInstaller::SQLite
@@ -32,6 +33,11 @@ namespace AppInstaller::SQLite
3233
// Gets the schema version of the database.
3334
const Version& GetVersion() const { return m_version; }
3435

36+
// Renames the database file and any auxiliary files given the inputs.
37+
// Should only be used on an inactive database.
38+
// If overwrite is given, existing destination files will be removed first.
39+
static void RenameSQLiteDatabase(const std::filesystem::path& source, const std::filesystem::path& destination, bool overwrite = false);
40+
3541
protected:
3642
SQLiteStorageBase(const std::string& target, const Version& version);
3743

src/AppInstallerSharedLib/SQLiteStorageBase.cpp

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,20 @@ namespace AppInstaller::SQLite
2323
return "Unknown";
2424
}
2525
}
26+
27+
std::filesystem::path AddSuffix(const std::filesystem::path& source, std::wstring_view suffix)
28+
{
29+
std::filesystem::path result{ source };
30+
31+
if (!suffix.empty())
32+
{
33+
std::wstring filename = result.filename().wstring();
34+
filename += suffix;
35+
result.replace_filename(std::move(filename));
36+
}
37+
38+
return result;
39+
}
2640
}
2741

2842
// One method for converting open disposition to proper open disposition
@@ -45,6 +59,37 @@ namespace AppInstaller::SQLite
4559
return MetadataTable::TryGetNamedValue<std::string>(m_dbconn, s_MetadataValueName_DatabaseIdentifier).value_or(std::string{});
4660
}
4761

62+
void SQLiteStorageBase::RenameSQLiteDatabase(const std::filesystem::path& source, const std::filesystem::path& destination, bool overwrite)
63+
{
64+
auto fileSuffixes = { L"", L"-journal", L"-wal" };
65+
66+
THROW_WIN32_IF(ERROR_FILE_NOT_FOUND, !std::filesystem::exists(source));
67+
THROW_WIN32_IF(ERROR_DIRECTORY, std::filesystem::is_directory(source));
68+
69+
if (overwrite)
70+
{
71+
for (const auto& suffix : fileSuffixes)
72+
{
73+
std::filesystem::path target = AddSuffix(destination, suffix);
74+
75+
if (std::filesystem::exists(target))
76+
{
77+
std::filesystem::remove_all(target);
78+
}
79+
}
80+
}
81+
82+
for (const auto& suffix : fileSuffixes)
83+
{
84+
std::filesystem::path target = AddSuffix(source, suffix);
85+
86+
if (std::filesystem::exists(target))
87+
{
88+
std::filesystem::rename(target, AddSuffix(destination, suffix));
89+
}
90+
}
91+
}
92+
4893
SQLiteStorageBase::SQLiteStorageBase(const std::string& filePath, OpenDisposition disposition, Utility::ManagedFile&& file) :
4994
m_indexFile(std::move(file))
5095
{

0 commit comments

Comments
 (0)