From b97c4e8b44202aa8b3049ed33661555ee1e5593b Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 9 Apr 2018 12:45:44 +0900 Subject: [PATCH 1/2] Fix all possible cases of crossthread import data races --- osu.Game/Database/ArchiveModelManager.cs | 2 ++ osu.Game/Overlays/BeatmapSet/Header.cs | 4 ++-- osu.Game/Overlays/DirectOverlay.cs | 12 ++++++++-- osu.Game/Overlays/Music/PlaylistOverlay.cs | 18 ++++++++++++-- .../Overlays/Settings/Sections/SkinSection.cs | 24 +++++++++++++++---- 5 files changed, 50 insertions(+), 10 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 7050e34712..3afb22f0ad 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -35,11 +35,13 @@ namespace osu.Game.Database /// /// Fired when a new becomes available in the database. + /// This is not guaranteed to run on the update thread. /// public event Action ItemAdded; /// /// Fired when a is removed from the database. + /// This is not guaranteed to run on the update thread. /// public event Action ItemRemoved; diff --git a/osu.Game/Overlays/BeatmapSet/Header.cs b/osu.Game/Overlays/BeatmapSet/Header.cs index b9a35ec1f0..8056dbff3c 100644 --- a/osu.Game/Overlays/BeatmapSet/Header.cs +++ b/osu.Game/Overlays/BeatmapSet/Header.cs @@ -246,11 +246,11 @@ namespace osu.Game.Overlays.BeatmapSet if (beatmaps != null) beatmaps.ItemAdded -= handleBeatmapAdd; } - private void handleBeatmapAdd(BeatmapSetInfo beatmap) + private void handleBeatmapAdd(BeatmapSetInfo beatmap) => Schedule(() => { if (beatmap.OnlineBeatmapSetID == BeatmapSet?.OnlineBeatmapSetID) downloadButtonsContainer.FadeOut(transition_duration); - } + }); private void download(bool noVideo) { diff --git a/osu.Game/Overlays/DirectOverlay.cs b/osu.Game/Overlays/DirectOverlay.cs index 8d8a4aebaa..3f1aa04c36 100644 --- a/osu.Game/Overlays/DirectOverlay.cs +++ b/osu.Game/Overlays/DirectOverlay.cs @@ -188,12 +188,12 @@ namespace osu.Game.Overlays beatmaps.ItemAdded += setAdded; } - private void setAdded(BeatmapSetInfo set) + private void setAdded(BeatmapSetInfo set) => Schedule(() => { // if a new map was imported, we should remove it from search results (download completed etc.) panels?.FirstOrDefault(p => p.SetInfo.OnlineBeatmapSetID == set.OnlineBeatmapSetID)?.FadeOut(400).Expire(); BeatmapSets = BeatmapSets?.Where(b => b.OnlineBeatmapSetID != set.OnlineBeatmapSetID); - } + }); private void updateResultCounts() { @@ -323,6 +323,14 @@ namespace osu.Game.Overlays private int distinctCount(List list) => list.Distinct().ToArray().Length; + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (beatmaps != null) + beatmaps.ItemAdded -= setAdded; + } + public class ResultCounts { public readonly int Artists; diff --git a/osu.Game/Overlays/Music/PlaylistOverlay.cs b/osu.Game/Overlays/Music/PlaylistOverlay.cs index ac7ec6257b..c981e5f493 100644 --- a/osu.Game/Overlays/Music/PlaylistOverlay.cs +++ b/osu.Game/Overlays/Music/PlaylistOverlay.cs @@ -74,8 +74,8 @@ namespace osu.Game.Overlays.Music }, }; - beatmaps.ItemAdded += list.AddBeatmapSet; - beatmaps.ItemRemoved += list.RemoveBeatmapSet; + beatmaps.ItemAdded += handleBeatmapAdded; + beatmaps.ItemRemoved += handleBeatmapRemoved; list.BeatmapSets = beatmaps.GetAllUsableBeatmapSets(); @@ -95,6 +95,9 @@ namespace osu.Game.Overlays.Music beatmapBacking.TriggerChange(); } + private void handleBeatmapAdded(BeatmapSetInfo setInfo) => Schedule(() => list.AddBeatmapSet(setInfo)); + private void handleBeatmapRemoved(BeatmapSetInfo setInfo) => Schedule(() => list.RemoveBeatmapSet(setInfo)); + protected override void PopIn() { filter.Search.HoldFocus = true; @@ -153,6 +156,17 @@ namespace osu.Game.Overlays.Music track.Restart(); } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (beatmaps != null) + { + beatmaps.ItemAdded -= handleBeatmapAdded; + beatmaps.ItemRemoved -= handleBeatmapRemoved; + } + } } //todo: placeholder diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 5df5304751..f16ddb2811 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -21,9 +21,13 @@ namespace osu.Game.Overlays.Settings.Sections public override FontAwesome Icon => FontAwesome.fa_paint_brush; + private SkinManager skins; + [BackgroundDependencyLoader] private void load(OsuConfigManager config, SkinManager skins) { + this.skins = skins; + FlowContent.Spacing = new Vector2(0, 5); Children = new Drawable[] { @@ -47,15 +51,27 @@ namespace osu.Game.Overlays.Settings.Sections }, }; - void reloadSkins() => skinDropdown.Items = skins.GetAllUsableSkins().Select(s => new KeyValuePair(s.ToString(), s.ID)); - skins.ItemAdded += _ => reloadSkins(); - skins.ItemRemoved += _ => reloadSkins(); + skins.ItemAdded += reloadSkins; + skins.ItemRemoved += reloadSkins; - reloadSkins(); + reloadSkins(null); skinDropdown.Bindable = config.GetBindable(OsuSetting.Skin); } + private void reloadSkins(SkinInfo changed) => Schedule(() => skinDropdown.Items = skins.GetAllUsableSkins().Select(s => new KeyValuePair(s.ToString(), s.ID))); + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (skins != null) + { + skins.ItemAdded -= reloadSkins; + skins.ItemRemoved -= reloadSkins; + } + } + private class SizeSlider : OsuSliderBar { public override string TooltipText => Current.Value.ToString(@"0.##x"); From 903dd7a01515c0ed9ace9d192587af5a188a241f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 11 Apr 2018 19:24:19 +0900 Subject: [PATCH 2/2] Fix regression causing hard crash Regressed in #2373. My bad. --- osu.Game/Overlays/Settings/Sections/SkinSection.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index f16ddb2811..a2215035dd 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -51,15 +51,17 @@ namespace osu.Game.Overlays.Settings.Sections }, }; - skins.ItemAdded += reloadSkins; - skins.ItemRemoved += reloadSkins; + skins.ItemAdded += onItemsChanged; + skins.ItemRemoved += onItemsChanged; - reloadSkins(null); + reloadSkins(); skinDropdown.Bindable = config.GetBindable(OsuSetting.Skin); } - private void reloadSkins(SkinInfo changed) => Schedule(() => skinDropdown.Items = skins.GetAllUsableSkins().Select(s => new KeyValuePair(s.ToString(), s.ID))); + private void reloadSkins() => skinDropdown.Items = skins.GetAllUsableSkins().Select(s => new KeyValuePair(s.ToString(), s.ID)); + + private void onItemsChanged(SkinInfo _) => Schedule(reloadSkins); protected override void Dispose(bool isDisposing) { @@ -67,8 +69,8 @@ namespace osu.Game.Overlays.Settings.Sections if (skins != null) { - skins.ItemAdded -= reloadSkins; - skins.ItemRemoved -= reloadSkins; + skins.ItemAdded -= onItemsChanged; + skins.ItemRemoved -= onItemsChanged; } }