Fix DrawableOsuHitObject not properly cleaning up dim application callbacks

[bdach]

Should fix #28629.

First of all, to support the claim that this does fix the issue - reproduction is rather difficult, but I believe I found a way to maximise the chances of it reproducing by performing the following steps:

1. Apply the following diff:

diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs index eacd2b3e75..4c00da031a 100644
--- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs @@ -6,6 +6,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Threading;
 using JetBrains.Annotations;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
@@ -95,6 +96,8 @@ public DrawableSlider([CanBeNull] Slider s = null)
         [BackgroundDependencyLoader]
         private void load()
         {
+            Thread.Sleep(100);
+
             tailContainer = new Container<DrawableSliderTail> { RelativeSizeAxes = Axes.Both };

             AddRangeInternal(new Drawable[]

2. Download https://osu.ppy.sh/beatmapsets/1470790#osu/3023028 and open it in the editor.
3. Select all objects using Ctrl-A. Yes, it'll take a while, especially so with the patch above.
4. Rotate the selection by any amount using the right toolbox.
5. Press undo. The game should crash. If it doesn't spam redo and undo until it does.

Now to explain what the fix is.

In the issue thread I spent a considerable time hemming and hawing about which of the dimmable pieces was null, which was a complete miss and a failure at reading. Let's see the stack trace again:

2024-06-27 02:15:20 [error]: at osu.Game.Rulesets.Osu.Objects.Drawables.DrawableOsuHitObject.<UpdateInitialTransforms>g__applyDim|15_0(Drawable piece) in /home/runner/work/osu-auth-client/osu-auth-client/osu/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs:line 101

Line 101, you say? What could be null here?

osu/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs

Line 101 in bd8addf

using (piece.BeginDelayedSequence(InitialLifetimeOffset - OsuHitWindows.MISS_WINDOW))

Okay, what's InitialLifetimeOffset, then?

osu/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs

Line 108 in bd8addf

protected sealed override double InitialLifetimeOffset => HitObject.TimePreempt;

Yes, that's right. It's HitObject that is null here.

Now why does that happen? First, let's note that all stacks where this died went through UpdateState(), which means that the problematic applyDim() calls had to be ApplyCustomUpdateState event callbacks. Meaning that the pieces where HitObject was null were DHOs themselves.

Recall that parent DHOs and child DHOs are pooled separately. Therefore, there is no guarantee that any parent and child DHOs will remain associated with each other for the entire duration of a gameplay session; it is quite the contrary, and nobody should rely on that. Unfortunately for us, adding a applyDimToDrawableHitObject callback to a child object's ApplyCustomUpdateState implicitly creates such an association, because it ends up allocating a closure that captures this (meaning the parent in this context).

Therefore, this now creates a situation where a child DHO can attempt to read state from a former parent DHO which can be in an indeterminate state, and in fact, when this crashes, the former parent DHO is most likely not even in use - hence the null HitObject.

Thus, the fix is to clear the association by unsubscribing from the event when nested objects are cleared.

My hypothesis why the reproduction scenario is like it is, is that both the sleep and the increased pressure on the pool (by way of selecting all objects and therefore forcing the DHOs to be materialised beyond pool capacity) increases the likelihood of getting a crosslink. When pool pressure is low, it is much more likely that a parent DHO will get the same child DHO again on re-application, even though that is not guaranteed.

Just as an additional detail, note that the sentry issue for this lists the "first seen" version as 2024.312.0, which is the release that included #27401 (notably authored by yours truly) which would be directly responsible for this mess.