From 40d1b97af10fa5d5a6f8a70f4d65886da78181cc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 26 Nov 2021 14:39:35 +0900 Subject: [PATCH] Avoid attempting to fetch a non-managed `RealmLive` instance from the realm backing For compatibility reasons, we quite often convert completely unmanaged instances to `ILive`s so they fit the required parameters of a property or method call. This ensures such cases will not cause any issues when trying to interact with the underlying data. Originally I had this allowing write operations, but that seems a bit unsafe (when performing a write one would assume that the underlying data is being persisted, whereas in this case it is not). We can change this if the requirements change in the future, but I think throwing is the safest bet for now. --- osu.Game.Tests/Database/RealmLiveTests.cs | 17 +++++++++++++++++ osu.Game/Database/EntityFrameworkLive.cs | 3 +++ osu.Game/Database/ILive.cs | 5 +++++ osu.Game/Database/RealmLive.cs | 20 ++++++++++++++++---- 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Database/RealmLiveTests.cs b/osu.Game.Tests/Database/RealmLiveTests.cs index 33aa1afb89..4f7681e3ae 100644 --- a/osu.Game.Tests/Database/RealmLiveTests.cs +++ b/osu.Game.Tests/Database/RealmLiveTests.cs @@ -30,6 +30,23 @@ namespace osu.Game.Tests.Database }); } + [Test] + public void TestAccessNonManaged() + { + var beatmap = new RealmBeatmap(CreateRuleset(), new RealmBeatmapDifficulty(), new RealmBeatmapMetadata()); + var liveBeatmap = beatmap.ToLive(); + + Assert.IsFalse(beatmap.Hidden); + Assert.IsFalse(liveBeatmap.Value.Hidden); + Assert.IsFalse(liveBeatmap.PerformRead(l => l.Hidden)); + + Assert.Throws(() => liveBeatmap.PerformWrite(l => l.Hidden = true)); + + Assert.IsFalse(beatmap.Hidden); + Assert.IsFalse(liveBeatmap.Value.Hidden); + Assert.IsFalse(liveBeatmap.PerformRead(l => l.Hidden)); + } + [Test] public void TestValueAccessWithOpenContext() { diff --git a/osu.Game/Database/EntityFrameworkLive.cs b/osu.Game/Database/EntityFrameworkLive.cs index 1d7b53911a..e4507b8d3b 100644 --- a/osu.Game/Database/EntityFrameworkLive.cs +++ b/osu.Game/Database/EntityFrameworkLive.cs @@ -9,6 +9,7 @@ namespace osu.Game.Database { public EntityFrameworkLive(T item) { + IsManaged = true; // no way to really know. Value = item; } @@ -29,6 +30,8 @@ namespace osu.Game.Database perform(Value); } + public bool IsManaged { get; } + public T Value { get; } } } diff --git a/osu.Game/Database/ILive.cs b/osu.Game/Database/ILive.cs index 9359b09eaf..ae287f1f6f 100644 --- a/osu.Game/Database/ILive.cs +++ b/osu.Game/Database/ILive.cs @@ -31,6 +31,11 @@ namespace osu.Game.Database /// The action to perform. void PerformWrite(Action perform); + /// + /// Whether this instance is tracking data which is managed by the database backing. + /// + bool IsManaged { get; } + /// /// Resolve the value of this instance on the current thread's context. /// diff --git a/osu.Game/Database/RealmLive.cs b/osu.Game/Database/RealmLive.cs index abb69644d6..fe631a9964 100644 --- a/osu.Game/Database/RealmLive.cs +++ b/osu.Game/Database/RealmLive.cs @@ -17,6 +17,8 @@ namespace osu.Game.Database { public Guid ID { get; } + public bool IsManaged { get; } + private readonly SynchronizationContext? fetchedContext; private readonly int fetchedThreadId; @@ -33,8 +35,13 @@ namespace osu.Game.Database { this.data = data; - fetchedContext = SynchronizationContext.Current; - fetchedThreadId = Thread.CurrentThread.ManagedThreadId; + if (data.IsManaged) + { + IsManaged = true; + + fetchedContext = SynchronizationContext.Current; + fetchedThreadId = Thread.CurrentThread.ManagedThreadId; + } ID = data.ID; } @@ -75,13 +82,18 @@ namespace osu.Game.Database /// Perform a write operation on this live object. /// /// The action to perform. - public void PerformWrite(Action perform) => + public void PerformWrite(Action perform) + { + if (!IsManaged) + throw new InvalidOperationException("Can't perform writes on a non-managed underlying value"); + PerformRead(t => { var transaction = t.Realm.BeginWrite(); perform(t); transaction.Commit(); }); + } public T Value { @@ -102,7 +114,7 @@ namespace osu.Game.Database } } - private bool originalDataValid => isCorrectThread && data.IsValid; + private bool originalDataValid => !IsManaged || (isCorrectThread && data.IsValid); // this matches realm's internal thread validation (see https://github.com/realm/realm-dotnet/blob/903b4d0b304f887e37e2d905384fb572a6496e70/Realm/Realm/Native/SynchronizationContextScheduler.cs#L72) private bool isCorrectThread