From 531794b26b6af72f3f25d609694e135fc960cf64 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 16 Aug 2023 14:23:30 +0900 Subject: [PATCH] Fix `ModelManager` not correctly re-retrieving realm objects before performing operations Falls into the age-old trap of attempting to retrieve an item from realm without first ensuring that realm is in an up-to-date state. Consider this scenario: - Editor is entered from main menu, causing it to create a new beatmap from its async `load()` method. - Editor opens correctly, then main thread performs a file operations on the same beatmap. - Main thread is potentially not refreshed yet, and will result in `null` instance when performing the re-fetch in `performFileOperation`. I've fixed this by using the safe implementation inside `RealmLive`. Feels like we want this is one place which is always used as the correct method. On a quick search, there are 10-20 other usages of `Realm.Find` which could also have similar issues, but it'll be a bit of a pain to go through and fix each of these. In 99.9% of cases, the accesses are on instances which couldn't have just been created (or the usage of recently-imported/created is blocked by realm subscription flows, ie. baetmap import) so I'm not touching them for now. Something to keep in mind when working with realm going forward though. --- osu.Game/Database/ModelManager.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/osu.Game/Database/ModelManager.cs b/osu.Game/Database/ModelManager.cs index 47feb8a8f9..6b538e62ea 100644 --- a/osu.Game/Database/ModelManager.cs +++ b/osu.Game/Database/ModelManager.cs @@ -48,16 +48,13 @@ namespace osu.Game.Database // This method should be removed as soon as all the surrounding pieces support non-detached operations. if (!item.IsManaged) { - // Importantly, begin the realm write *before* re-fetching, else the update realm may not be in a consistent state - // (ie. if an async import finished very recently). - Realm.Realm.Write(realm => + // We use RealmLive here as it handled re-retrieval and refreshing of realm if required. + new RealmLive(item.ID, Realm).PerformWrite(i => { - var managed = realm.Find(item.ID); - Debug.Assert(managed != null); - operation(managed); + operation(i); item.Files.Clear(); - item.Files.AddRange(managed.Files.Detach()); + item.Files.AddRange(i.Files.Detach()); }); } else