mirror of
https://github.com/ppy/osu.git
synced 2024-11-11 11:37:28 +08:00
Fix leak of ModSettingChangeTracker
instances
The `SelectedMods.BindValueChanged()` callback in `ModSelectOverlay` can in some instances run recursively. This is most heavily leaned on in scenarios where `SelectedMods` is updated by an external component. In such cases, the mod select overlay needs to replace the mod instances received externally with mod instances which it owns, so that the changes made on the overlay can propagate outwards. This in particular means that prior to this commit, it was possible to encounter the following scenario: modSettingChangeTracker?.Dispose(); updateFromExternalSelection(); // mutates SelectedMods to perform the replacement // therefore causing a recursive call modSettingChangeTracker?.Dispose(); // inner call continues modSettingChangeTracker = new ModSettingChangeTracker(SelectedMods.Value); // outer call continues modSettingChangeTracker = new ModSettingChangeTracker(SelectedMods.Value); This leaks one `modSettingChangeTracker` instance from the inner call, which is never disposed. To avoid this, move the disposal to the same side of the recursion that the creation happens on, changing the call pattern to: updateFromExternalSelection(); // mutates SelectedMods to perform the replacement // therefore causing a recursive call modSettingChangeTracker?.Dispose(); // inner call continues modSettingChangeTracker = new ModSettingChangeTracker(SelectedMods.Value); modSettingChangeTracker?.Dispose(); // outer call continues modSettingChangeTracker = new ModSettingChangeTracker(SelectedMods.Value); which, while slightly wasteful, does not cause any leaks. The solution is definitely suboptimal, but addressing this properly would entail a major rewrite of the mod instance management in the mods overlay, which is probably not the wisest move to make right now.
This commit is contained in:
parent
d35355970f
commit
2e3daf0a54
@ -244,12 +244,12 @@ namespace osu.Game.Overlays.Mods
|
||||
|
||||
SelectedMods.BindValueChanged(val =>
|
||||
{
|
||||
modSettingChangeTracker?.Dispose();
|
||||
|
||||
updateMultiplier();
|
||||
updateFromExternalSelection();
|
||||
updateCustomisation();
|
||||
|
||||
modSettingChangeTracker?.Dispose();
|
||||
|
||||
if (AllowCustomisation)
|
||||
{
|
||||
modSettingChangeTracker = new ModSettingChangeTracker(SelectedMods.Value);
|
||||
|
Loading…
Reference in New Issue
Block a user