mirror of
https://github.com/ppy/osu.git
synced 2026-06-05 11:23:40 +08:00
Fix dodgy threading
The fact that the stuff "just worked" previously due to one load-bearing detach in a random location is really scary because a lot of this was just not written the way it is supposed to be.
This commit is contained in:
@@ -543,6 +543,44 @@ namespace osu.Game.Database
|
||||
return writeTask;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Write changes to realm asynchronously, guaranteeing order of execution.
|
||||
/// </summary>
|
||||
/// <param name="action">The work to run.</param>
|
||||
public Task<T> WriteAsync<T>(Func<Realm, T> action)
|
||||
{
|
||||
ObjectDisposedException.ThrowIf(isDisposed, this);
|
||||
|
||||
// Required to ensure the write is tracked and accounted for before disposal.
|
||||
// Can potentially be avoided if we have a need to do so in the future.
|
||||
if (!ThreadSafety.IsUpdateThread)
|
||||
throw new InvalidOperationException(@$"{nameof(WriteAsync)} must be called from the update thread.");
|
||||
|
||||
// CountdownEvent will fail if already at zero.
|
||||
if (!pendingAsyncWrites.TryAddCount())
|
||||
pendingAsyncWrites.Reset(1);
|
||||
|
||||
// Regardless of calling Realm.GetInstance or Realm.GetInstanceAsync, there is a blocking overhead on retrieval.
|
||||
// Adding a forced Task.Run resolves this.
|
||||
var writeTask = Task.Run(async () =>
|
||||
{
|
||||
T result;
|
||||
total_writes_async.Value++;
|
||||
|
||||
// Not attempting to use Realm.GetInstanceAsync as there's seemingly no benefit to us (for now) and it adds complexity due to locking
|
||||
// concerns in getRealmInstance(). On a quick check, it looks to be more suited to cases where realm is connecting to an online sync
|
||||
// server, which we don't use. May want to report upstream or revisit in the future.
|
||||
using (var realm = getRealmInstance())
|
||||
// ReSharper disable once AccessToDisposedClosure (WriteAsync should be marked as [InstantHandle]).
|
||||
result = await realm.WriteAsync(() => action(realm)).ConfigureAwait(false);
|
||||
|
||||
pendingAsyncWrites.Signal();
|
||||
return result;
|
||||
});
|
||||
|
||||
return writeTask;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Subscribe to a realm collection and begin watching for asynchronous changes.
|
||||
/// </summary>
|
||||
|
||||
@@ -205,9 +205,7 @@ namespace osu.Game.Database
|
||||
|
||||
Directory.CreateDirectory(mountedPath);
|
||||
|
||||
// Detach files from the model to avoid realm contention when copying to the external location.
|
||||
// This is safe as we are not modifying the model in any way.
|
||||
foreach (var realmFile in model.Files.Detach())
|
||||
foreach (var realmFile in model.Files)
|
||||
{
|
||||
string sourcePath = Files.Storage.GetFullPath(realmFile.File.GetStoragePath());
|
||||
string destinationPath = Path.Join(mountedPath, realmFile.Filename);
|
||||
|
||||
@@ -14,6 +14,7 @@ using osu.Game.Input.Bindings;
|
||||
using osu.Game.Models;
|
||||
using osu.Game.Rulesets;
|
||||
using osu.Game.Scoring;
|
||||
using osu.Game.Skinning;
|
||||
using Realms;
|
||||
|
||||
namespace osu.Game.Database
|
||||
@@ -177,6 +178,7 @@ namespace osu.Game.Database
|
||||
c.CreateMap<RealmUser, RealmUser>();
|
||||
c.CreateMap<RealmFile, RealmFile>();
|
||||
c.CreateMap<RealmNamedFileUsage, RealmNamedFileUsage>();
|
||||
c.CreateMap<SkinInfo, SkinInfo>();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -53,12 +53,11 @@ namespace osu.Game.Skinning
|
||||
/// <param name="task">The <see cref="ImportTask"/> to update the <paramref name="original"/> with</param>
|
||||
/// <param name="original">The <see cref="SkinInfo"/> to update</param>
|
||||
/// <returns></returns>
|
||||
public override Task<Live<SkinInfo>?> ImportAsUpdate(ProgressNotification notification, ImportTask task, SkinInfo original)
|
||||
public override async Task<Live<SkinInfo>?> ImportAsUpdate(ProgressNotification notification, ImportTask task, SkinInfo original)
|
||||
{
|
||||
var skinInfoLive = original.ToLive(Realm);
|
||||
|
||||
skinInfoLive.PerformWrite(skinInfo =>
|
||||
return await Realm.WriteAsync<Live<SkinInfo>?>(r =>
|
||||
{
|
||||
var skinInfo = r.Find<SkinInfo>(original.ID)!;
|
||||
skinInfo.Files.Clear();
|
||||
|
||||
string[] filesInMountedDirectory = Directory.EnumerateFiles(task.Path, "*.*", SearchOption.AllDirectories).Select(f => Path.GetRelativePath(task.Path, f)).ToArray();
|
||||
@@ -67,28 +66,28 @@ namespace osu.Game.Skinning
|
||||
{
|
||||
using var stream = File.OpenRead(Path.Combine(task.Path, file));
|
||||
|
||||
modelManager.AddFile(original, stream, file);
|
||||
modelManager.AddFile(skinInfo, stream, file, r);
|
||||
}
|
||||
|
||||
string skinIniPath = Path.Combine(task.Path, "skin.ini");
|
||||
|
||||
if (!File.Exists(skinIniPath))
|
||||
return;
|
||||
|
||||
using (var stream = File.OpenRead(skinIniPath))
|
||||
using (var lineReader = new LineBufferedReader(stream))
|
||||
if (File.Exists(skinIniPath))
|
||||
{
|
||||
var decodedSkinIni = new LegacySkinDecoder().Decode(lineReader);
|
||||
using (var stream = File.OpenRead(skinIniPath))
|
||||
using (var lineReader = new LineBufferedReader(stream))
|
||||
{
|
||||
var decodedSkinIni = new LegacySkinDecoder().Decode(lineReader);
|
||||
|
||||
if (!string.IsNullOrEmpty(decodedSkinIni.SkinInfo.Name))
|
||||
skinInfo.Name = decodedSkinIni.SkinInfo.Name;
|
||||
if (!string.IsNullOrEmpty(decodedSkinIni.SkinInfo.Name))
|
||||
skinInfo.Name = decodedSkinIni.SkinInfo.Name;
|
||||
|
||||
if (!string.IsNullOrEmpty(decodedSkinIni.SkinInfo.Creator))
|
||||
skinInfo.Creator = decodedSkinIni.SkinInfo.Creator;
|
||||
if (!string.IsNullOrEmpty(decodedSkinIni.SkinInfo.Creator))
|
||||
skinInfo.Creator = decodedSkinIni.SkinInfo.Creator;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
return Task.FromResult(skinInfoLive)!;
|
||||
return skinInfo.ToLive(Realm);
|
||||
}).ConfigureAwait(false);
|
||||
}
|
||||
|
||||
protected override void Populate(SkinInfo model, ArchiveReader? archive, Realm realm, CancellationToken cancellationToken = default)
|
||||
|
||||
Reference in New Issue
Block a user