1
0
mirror of https://github.com/ppy/osu.git synced 2025-03-20 03:47:28 +08:00

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<T>`.
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<T>` 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.
This commit is contained in:
Dean Herbert 2023-08-16 14:23:30 +09:00
parent 5bd7370439
commit 531794b26b

View File

@ -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<TModel>(item.ID, Realm).PerformWrite(i =>
{
var managed = realm.Find<TModel>(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