Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refetch local metadata cache if corruption is detected #31509

Merged
merged 4 commits into from
Jan 22, 2025
Merged

Refetch local metadata cache if corruption is detected #31509

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b0c0c98
Refetch local metadata cache if corruption is detected
bdach Jan 14, 2025
910c002
Adjust code style slightly
peppy Jan 22, 2025
26ef23c
Remove outdated ef related catch-when usage
peppy Jan 22, 2025
6ceb348
Adjust code again to avoid weird `throw` mishandling
peppy Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions osu.Game/Beatmaps/LocalCachedBeatmapMetadataSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,26 @@ public bool TryLookup(BeatmapInfo beatmapInfo, [NotNullWhen(true)] out OnlineBea
}
}
}
catch (SqliteException sqliteException)
{
// There have been cases where the user's local database is corrupt.
// Let's attempt to identify these cases and re-initialise the local cache.
switch (sqliteException.SqliteErrorCode)
{
case 26: // SQLITE_NOTADB
case 11: // SQLITE_CORRUPT
// only attempt purge & re-download if there is no other refetch in progress
if (cacheDownloadRequest != null)
throw;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a behavioural change? I think this should return false? If the intention was to rethrow to the catch (Exception ex) block below then I'm not sure that's how this works.

In an isolated console project, when I do something like

try
{
   throw new InvalidOperationException();
}
catch (InvalidOperationException)
{
   throw;
}
catch
{
   Console.WriteLine("caught");
}

then the resulting program is not printing to the console, it's throwing the error:

Unhandled exception. System.InvalidOperationException: Operation is not valid due to the current state of the object.
   at Program.<Main>$(String[] args) in /home/dachb/Documents/Projekty/ConsoleApp1/ConsoleApp1/Program.cs:line 5

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, that's unfortunate. i thought that's how it worked but i guess not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made one more attempt.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems better now yeah.


tryPurgeCache();
prepareLocalCache();
onlineMetadata = null;
return false;
}

throw;
}
catch (Exception ex)
{
logForModel(beatmapInfo.BeatmapSet, $@"Cached local retrieval for {beatmapInfo} failed with {ex}.");
Expand All @@ -125,6 +145,22 @@ public bool TryLookup(BeatmapInfo beatmapInfo, [NotNullWhen(true)] out OnlineBea
return false;
}

private void tryPurgeCache()
{
log(@"Local metadata cache is corrupted; attempting purge.");

try
{
File.Delete(storage.GetFullPath(cache_database_name));
}
catch (Exception ex)
{
log($@"Failed to purge local metadata cache: {ex}");
}

log(@"Local metadata cache purged due to corruption.");
}

private SqliteConnection getConnection() =>
new SqliteConnection(string.Concat(@"Data Source=", storage.GetFullPath(@"online.db", true)));

Expand Down
19 changes: 1 addition & 18 deletions osu.Game/Database/RealmAccess.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
using System.Reflection;
using System.Threading;
using System.Threading.Tasks;
using osu.Framework;
using osu.Framework.Allocation;
using osu.Framework.Development;
using osu.Framework.Extensions;
Expand Down Expand Up @@ -413,18 +412,7 @@ private void cleanupPendingDeletions(Realm realm)
/// Compact this realm.
/// </summary>
/// <returns></returns>
public bool Compact()
{
try
{
return Realm.Compact(getConfiguration());
}
// Catch can be removed along with entity framework. Is specifically to allow a failure message to arrive to the user (see similar catches in EFToRealmMigrator).
catch (AggregateException ae) when (RuntimeInfo.OS == RuntimeInfo.Platform.macOS && ae.Flatten().InnerException is TypeInitializationException)
{
return true;
}
}
public bool Compact() => Realm.Compact(getConfiguration());

/// <summary>
/// Run work on realm with a return value.
Expand Down Expand Up @@ -720,11 +708,6 @@ private Realm getRealmInstance()

return Realm.GetInstance(getConfiguration());
}
// Catch can be removed along with entity framework. Is specifically to allow a failure message to arrive to the user (see similar catches in EFToRealmMigrator).
catch (AggregateException ae) when (RuntimeInfo.OS == RuntimeInfo.Platform.macOS && ae.Flatten().InnerException is TypeInitializationException)
{
return Realm.GetInstance();
}
finally
{
if (tookSemaphoreLock)
Expand Down
Loading