[ue] Crash accessing deleted UTrackEntry as renderer object

[sgarces]

UE 5.3.2 - Spine runtime 4.2

I'm getting a fairly reproducible crash with USpineWidget as an animation end callback is attempted to be sent. I believe this is a similar crash as in https://github.com/EsotericSoftware/spine-runtimes/issues/1190

As explained in that post, spine::TrackEntry holds a raw pointer reference to the UTrackEntry that is created when an animation is set. In the function USpineWidget::SynchronizeProperties() the array trackEntries is emptied, making all the UTrackEntry's garbage collectible. This can happen even when the AnimationState (and any outstanding TrackEntry's) are still alive and well.

My use case was to have a screen that is removed from view, but with a reference kept, so it's not released. There is a SpineWidget playing a looping animation. When the screen is added to view again, SynchronizedProperties is called, clearing the trackEntries array and potentially releasing the UTrackEntry but with an active AnimationState frozen in time (as Tick hasn't been called). Once the animation end event is trying to fire, the function callbackWidget gets a stale Renderer Object and the game crashes.

I've attached a text file with the (surprisingly large) callstack. spine_track_entry_crash_callstack.txt

At first I attempted to fix it by going through the UTrackEntry's and calling SetTrackEntry(nullptr), but that wouldn't work, as that function doesn't actually update the TrackEntry that is being disconnected. I believe this to be an error and, in fact, there are two other flows that can cause accessing dangling pointers if the dispose event is received (these are the only cases where SetTrackEntry(nullptr) is called).

As such, the easiest fix is to replace the line trackEntries.Empty(); in USpineWidget::SynchronizeProperties() with:

for (UTrackEntry* entry : trackEntries) 
{
    if (entry && entry->GetTrackEntry())
    {
        entry->GetTrackEntry()->setRendererObject(nullptr);
    }
}
trackEntries.Empty();

I also think that UTrackEntry::SetTrackEntry() should be updated to clear the reference from the TrackEntry, by adding this code at the beginning of the function:

if (entry) 
    entry->setRendererObject(nullptr);

Besides this, I also believe the crash reported in bug #1190 is still active, and the solution outlined is correct (replacing trackEntries.Empty() in USpineSkeletonAnimationComponent::BeginPlay() with DisposeState().

I will be implementing this changes in my version of the runtime, but I thought others might benefit as well.

[sgarces]

One correction to add. I don't believe anymore that using DisposeState() in USpineSkeletonAnimationComponent::BeginPlay() is the right fix, as that changes the behavior in that the state could have been created beforehand (before BeginPlay) and in that case the whole thing will fail as it doesn't get recreated even after calling CheckState(). So the correct fix would be to do the same thing as for USpineWidget::SynchronizeProperties() and unlink any track entries that exist. In most cases hopefully this would be none.

[badlogic]

Thank you for the excellent bug report. I'm having trouble reproducing the issue. Do you have a test case for me?

[sgarces]

I spent some time today putting together a minimal test: http://sgarces.com/SpineBug_2557.zip It doesn't crash because, due to how little it's going on, even after deleting the track the memory is not reused, so it "works". I've added some UE_LOG to my test to show the flow of how a UTrackEntry is deleted and then subsequently referenced.

Hope this helps.