diff --git a/osu.Game.Tests/NonVisual/OngoingOperationTrackerTest.cs b/osu.Game.Tests/NonVisual/OngoingOperationTrackerTest.cs new file mode 100644 index 0000000000..eef9582af9 --- /dev/null +++ b/osu.Game.Tests/NonVisual/OngoingOperationTrackerTest.cs @@ -0,0 +1,62 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using NUnit.Framework; +using osu.Framework.Bindables; +using osu.Framework.Testing; +using osu.Game.Screens.OnlinePlay; +using osu.Game.Tests.Visual; + +namespace osu.Game.Tests.NonVisual +{ + [HeadlessTest] + public class OngoingOperationTrackerTest : OsuTestScene + { + private OngoingOperationTracker tracker; + private IBindable operationInProgress; + + [SetUpSteps] + public void SetUp() + { + AddStep("create tracker", () => Child = tracker = new OngoingOperationTracker()); + AddStep("bind to operation status", () => operationInProgress = tracker.InProgress.GetBoundCopy()); + } + + [Test] + public void TestOperationTracking() + { + IDisposable firstOperation = null; + IDisposable secondOperation = null; + + AddStep("begin first operation", () => firstOperation = tracker.BeginOperation()); + AddAssert("first operation in progress", () => operationInProgress.Value); + + AddStep("cannot start another operation", + () => Assert.Throws(() => tracker.BeginOperation())); + + AddStep("end first operation", () => firstOperation.Dispose()); + AddAssert("first operation is ended", () => !operationInProgress.Value); + + AddStep("start second operation", () => secondOperation = tracker.BeginOperation()); + AddAssert("second operation in progress", () => operationInProgress.Value); + + AddStep("dispose first operation again", () => firstOperation.Dispose()); + AddAssert("second operation still in progress", () => operationInProgress.Value); + + AddStep("dispose second operation", () => secondOperation.Dispose()); + AddAssert("second operation is ended", () => !operationInProgress.Value); + } + + [Test] + public void TestOperationDisposalAfterTracker() + { + IDisposable operation = null; + + AddStep("begin operation", () => operation = tracker.BeginOperation()); + AddStep("dispose tracker", () => tracker.Expire()); + AddStep("end operation", () => operation.Dispose()); + AddAssert("operation is ended", () => !operationInProgress.Value); + } + } +} diff --git a/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs b/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs index 5c9e9ce90b..b7ee84eb9e 100644 --- a/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs +++ b/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System; -using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -43,17 +42,46 @@ namespace osu.Game.Screens.OnlinePlay leasedInProgress = inProgress.BeginLease(true); leasedInProgress.Value = true; - // for extra safety, marshal the end of operation back to the update thread if necessary. - return new InvokeOnDisposal(() => Scheduler.Add(endOperation, false)); + return new OngoingOperation(this, leasedInProgress); } - private void endOperation() + private void endOperationWithKnownLease(LeasedBindable lease) { - if (leasedInProgress == null) - throw new InvalidOperationException("Cannot end operation multiple times."); + if (lease != leasedInProgress) + return; - leasedInProgress.Return(); + // for extra safety, marshal the end of operation back to the update thread if necessary. + Scheduler.Add(() => + { + leasedInProgress?.Return(); + leasedInProgress = null; + }, false); + } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + // base call does an UnbindAllBindables(). + // clean up the leased reference here so that it doesn't get returned twice. leasedInProgress = null; } + + private class OngoingOperation : IDisposable + { + private readonly OngoingOperationTracker tracker; + private readonly LeasedBindable lease; + + public OngoingOperation(OngoingOperationTracker tracker, LeasedBindable lease) + { + this.tracker = tracker; + this.lease = lease; + } + + public void Dispose() + { + tracker.endOperationWithKnownLease(lease); + } + } } }