1
0
mirror of https://github.com/ppy/osu.git synced 2024-12-13 08:32:57 +08:00

Fix difficulty cache lookups sharing underlying mod instances

`DifficultyCacheLookup`s were storing raw `Mod` instances into their
`OrderedMods` field. This could cause the cache lookups to wrongly
succeed in cases of mods with settings. The particular case that
triggered this fix was Difficulty Adjust.

Because the difficulty cache is backed by a dictionary, there are two
stages to the lookup; first `GetHashCode()` is used to find the
appropriate hash bucket to look in, and then items from that hash bucket
are compared against the key being searched for via the implementation
of `Equals()`.

As it turns out, the first hashing step ended up being the saving grace
in most cases, as the hash computation included the values of the mod
settings. But the Difficulty Adjust failure case was triggered by the
quirk that `GetHashCode(0) == GetHashCode(null) == 0`.

In such a case, the `Equals()` fallback was used. But as it turns out,
because the `Mod` instance stored to lookups was not cloned and
therefore potentially externally mutable, it could be polluted after
being stored to the dictionary, and therefore breaking the equality
check. Even though all of the setting values were compared, the hash
bucket didn't match the actual contents of the lookup anymore (because
they were mutated externally, e.g. by the user changing the mod setting
values in the mod settings overlay).

To resolve, clone out the mod structure before creating all difficulty
lookups.
This commit is contained in:
Bartłomiej Dach 2021-08-21 15:39:02 +02:00
parent f642546d6a
commit 995338029c
No known key found for this signature in database
GPG Key ID: BCECCD4FA41F6497

View File

@ -310,7 +310,7 @@ namespace osu.Game.Beatmaps
Beatmap = beatmap;
// In the case that the user hasn't given us a ruleset, use the beatmap's default ruleset.
Ruleset = ruleset ?? Beatmap.Ruleset;
OrderedMods = mods?.OrderBy(m => m.Acronym).ToArray() ?? Array.Empty<Mod>();
OrderedMods = mods?.OrderBy(m => m.Acronym).Select(mod => mod.DeepClone()).ToArray() ?? Array.Empty<Mod>();
}
public bool Equals(DifficultyCacheLookup other)