RepoMirrors/osu
1
0
Fork 0
mirror of https://github.com/ppy/osu synced 2024-12-16 11:56:31 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #11632 from bdach/ongoing-tracker-fix-more

Browse Source 
Fix ongoing operation tracker double-returning internal lease after screen exit
This commit is contained in:
Dan Balasescu 2021-02-03 23:17:53 +09:00 committed by GitHub
commit 75801097ea
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 52 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
osu.Game.Tests/NonVisual/OngoingOperationTrackerTest.cs
View File

@ -3,8 +3,12 @@
using System;
using NUnit.Framework;
using osu.Framework.Allocation;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Framework.Screens;
using osu.Framework.Testing;
using osu.Game.Screens;
using osu.Game.Screens.OnlinePlay;
using osu.Game.Tests.Visual;
@ -58,5 +62,45 @@ namespace osu.Game.Tests.NonVisual
AddStep("end operation", () => operation.Dispose());
AddAssert("operation is ended", () => !operationInProgress.Value);
}
[Test]
public void TestOperationDisposalAfterScreenExit()
{
TestScreenWithTracker screen = null;
OsuScreenStack stack;
IDisposable operation = null;
AddStep("create screen with tracker", () =>
{
Child = stack = new OsuScreenStack
{
RelativeSizeAxes = Axes.Both
};
stack.Push(screen = new TestScreenWithTracker());
});
AddUntilStep("wait for loaded", () => screen.IsLoaded);
AddStep("begin operation", () => operation = screen.OngoingOperationTracker.BeginOperation());
AddAssert("operation in progress", () => screen.OngoingOperationTracker.InProgress.Value);
AddStep("dispose after screen exit", () =>
{
screen.Exit();
operation.Dispose();
});
AddAssert("operation ended", () => !screen.OngoingOperationTracker.InProgress.Value);
}
private class TestScreenWithTracker : OsuScreen
{
public OngoingOperationTracker OngoingOperationTracker { get; private set; }
[BackgroundDependencyLoader]
private void load()
{
InternalChild = OngoingOperationTracker = new OngoingOperationTracker();
}
}
}
}

21
osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs
View File

@ -47,26 +47,21 @@ namespace osu.Game.Screens.OnlinePlay
private void endOperationWithKnownLease(LeasedBindable<bool> lease)
{
if (lease != leasedInProgress)
return;
// for extra safety, marshal the end of operation back to the update thread if necessary.
Scheduler.Add(() =>
{
leasedInProgress?.Return();
if (lease != leasedInProgress)
return;
// UnbindAll() is purposefully used instead of Return() - the two do roughly the same thing, with one difference:
// the former won't throw if the lease has already been returned before.
// this matters because framework can unbind the lease via the internal UnbindAllBindables(), which is not always detectable
// (it is in the case of disposal, but not in the case of screen exit - at least not cleanly).
leasedInProgress?.UnbindAll();
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;