From 567e9f33a9b74e577761b1eb62564d35be78e8d2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 4 Jul 2021 16:24:39 +0900 Subject: [PATCH] Fix thread safety of realm `Refresh` operation Due to the lack of locking, there was a chance the the update thread `context` was retrieved just before the `flushContexts` call, followed by `.Refresh()` being run while the blocking behaviour was invoked. This can be seen in test failures such as https://ci.appveyor.com/project/peppy/osu/builds/39859786/tests. As an aside, I tried multiple different methods to avoid `lock()` on the update thread but they felt flaky. The overhead of lock when there's no contention is reportedly around 30-50ns, so likely not of concern. We can address it at a later point if it becomes one. --- osu.Game/Database/RealmContextFactory.cs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index fb5e2faff8..3354b97849 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -38,6 +38,8 @@ namespace osu.Game.Database private static readonly GlobalStatistic pending_writes = GlobalStatistics.Get("Realm", "Pending writes"); private static readonly GlobalStatistic active_usages = GlobalStatistics.Get("Realm", "Active usages"); + private readonly object updateContextLock = new object(); + private Realm context; public Realm Context @@ -107,8 +109,11 @@ namespace osu.Game.Database { base.Update(); - if (context?.Refresh() == true) - refreshes.Value++; + lock (updateContextLock) + { + if (context?.Refresh() == true) + refreshes.Value++; + } } private Realm createContext() @@ -156,7 +161,9 @@ namespace osu.Game.Database Logger.Log(@"Flushing realm contexts...", LoggingTarget.Database); var previousContext = context; - context = null; + + lock (updateContextLock) + context = null; // wait for all threaded usages to finish while (active_usages.Value > 0)