osu-framework icon indicating copy to clipboard operation
osu-framework copied to clipboard

`ConfigManager` can save after disposal

Open Susko3 opened this issue 3 years ago • 2 comments

Noticed this when looking into https://github.com/ppy/osu-framework/issues/4950.

~~Since saves are debounced, this can easily happen if a save is queued right before disposal.~~ (Doesn't happen because the current == lastSave check will probably fail.)

Nothing bad really happens, it's just weird to me that this can happen.

QueueBackgroundSave() and Save() should probably throw if disposed. But the task in QueueBackgroundSave() should silently fail. If this is alright with you, let me know so I can PR it.

Susko3 avatar Feb 07 '22 15:02 Susko3

I think what would make the most sense here is to throw ObjectDisposedException in QueueBackgroundSave and Save (and probably in all other public methods too?), and unbind all QueueBackgroundSave events from the config bindables on disposal (should be safe to just perform UnbindAll).

frenzibyte avatar Feb 07 '22 19:02 frenzibyte

Throwing an exception sounds amicable.

smoogipoo avatar Feb 08 '22 04:02 smoogipoo