From 0bcfb8e199004cb123598d9d5972d97ebf383cd2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 18:52:29 +0900 Subject: [PATCH 1/2] Fix regression in implementation of fix --- osu.Game/Database/RealmContextFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 6948918fe7..a20139e830 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -160,6 +160,7 @@ namespace osu.Game.Database if (!currentThreadCanCreateContexts.Value) { contextCreationLock.Wait(); + currentThreadCanCreateContexts.Value = true; tookSemaphoreLock = true; } else @@ -167,7 +168,6 @@ namespace osu.Game.Database // the semaphore is used to handle blocking of all context creation during certain periods. // once the semaphore has been taken by this code section, it is safe to create further contexts on the same thread. // this can happen if a realm subscription is active and triggers a callback which has user code that calls `CreateContext`. - currentThreadCanCreateContexts.Value = true; } contexts_created.Value++; From 566e10f8ccff29b3c0780832ec1811c13f1e4276 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 18:54:01 +0900 Subject: [PATCH 2/2] Refactor test to be easier to follow --- osu.Game.Tests/Database/GeneralUsageTests.cs | 36 +++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/osu.Game.Tests/Database/GeneralUsageTests.cs b/osu.Game.Tests/Database/GeneralUsageTests.cs index fec1a0dedd..d15e34f2da 100644 --- a/osu.Game.Tests/Database/GeneralUsageTests.cs +++ b/osu.Game.Tests/Database/GeneralUsageTests.cs @@ -36,34 +36,36 @@ namespace osu.Game.Tests.Database }); } + /// + /// Test to ensure that a `CreateContext` call nested inside a subscription doesn't cause any deadlocks + /// due to context fetching semaphores. + /// [Test] - public void TestNestedContextCreation() + public void TestNestedContextCreationWithSubscription() { RunTestWithRealm((realmFactory, _) => { - var mainContext = realmFactory.Context; bool callbackRan = false; - var subscription = mainContext.All().SubscribeForNotifications((sender, changes, error) => + using (var context = realmFactory.CreateContext()) { - realmFactory.CreateContext(); - callbackRan = true; - }); - - Task.Factory.StartNew(() => - { - using (var threadContext = realmFactory.CreateContext()) + var subscription = context.All().SubscribeForNotifications((sender, changes, error) => { - threadContext.Write(r => r.Add(new RealmBeatmap(CreateRuleset(), new RealmBeatmapDifficulty(), new RealmBeatmapMetadata()))); - } - }, TaskCreationOptions.LongRunning | TaskCreationOptions.HideScheduler).Wait(); + using (realmFactory.CreateContext()) + { + callbackRan = true; + } + }); - // will create a context but also run the callback above (Refresh is implicitly run when getting a new context). - realmFactory.CreateContext(); + // Force the callback above to run. + using (realmFactory.CreateContext()) + { + } + + subscription.Dispose(); + } Assert.IsTrue(callbackRan); - - subscription.Dispose(); }); }