From 3cf1a386004e003d556266d7d54339ac46d42072 Mon Sep 17 00:00:00 2001 From: Christian Semmler Date: Fri, 3 Apr 2026 20:37:04 -0700 Subject: [PATCH] Fix use-after-free crash in ScenePlayer when remote player disconnects mid-animation When a remote player's ROI is destroyed (disconnect, timeout, or respawn), notify all active ScenePlayer instances to null out dangling references before the ROI is freed. The animation engine already handles null ROI map entries gracefully, so playback continues for remaining participants. --- .../multiplayer/animation/phonemeplayer.h | 1 + .../multiplayer/animation/sceneplayer.h | 1 + .../extensions/multiplayer/networkmanager.h | 1 + .../multiplayer/animation/phonemeplayer.cpp | 9 ++++ .../src/multiplayer/animation/sceneplayer.cpp | 46 ++++++++++++++++++- extensions/src/multiplayer/networkmanager.cpp | 18 ++++++++ 6 files changed, 74 insertions(+), 2 deletions(-) diff --git a/extensions/include/extensions/multiplayer/animation/phonemeplayer.h b/extensions/include/extensions/multiplayer/animation/phonemeplayer.h index 86260589..de60bcdf 100644 --- a/extensions/include/extensions/multiplayer/animation/phonemeplayer.h +++ b/extensions/include/extensions/multiplayer/animation/phonemeplayer.h @@ -30,6 +30,7 @@ class PhonemePlayer { ); void Tick(float p_elapsedMs, const std::vector& p_tracks); void Cleanup(); + void NotifyROIDestroyed(LegoROI* p_roi); private: std::vector m_states; diff --git a/extensions/include/extensions/multiplayer/animation/sceneplayer.h b/extensions/include/extensions/multiplayer/animation/sceneplayer.h index aaa1bbde..8aeb4689 100644 --- a/extensions/include/extensions/multiplayer/animation/sceneplayer.h +++ b/extensions/include/extensions/multiplayer/animation/sceneplayer.h @@ -45,6 +45,7 @@ class ScenePlayer { ); void Tick(); void Stop(); + void NotifyROIDestroyed(LegoROI* p_roi); bool IsPlaying() const { return m_playing; } bool IsObserverMode() const { return m_observerMode; } diff --git a/extensions/include/extensions/multiplayer/networkmanager.h b/extensions/include/extensions/multiplayer/networkmanager.h index 391ffb81..340b79bb 100644 --- a/extensions/include/extensions/multiplayer/networkmanager.h +++ b/extensions/include/extensions/multiplayer/networkmanager.h @@ -222,6 +222,7 @@ class NetworkManager : public MxCore { void TickAnimation(); void StopScenePlayback(uint16_t p_animIndex, bool p_unlockRemotes); void StopAllPlayback(); + void NotifyAnimationsROIDestroyed(RemotePlayer* p_player); void UnlockRemotesForAnim(uint16_t p_animIndex); // Horn sound synchronization diff --git a/extensions/src/multiplayer/animation/phonemeplayer.cpp b/extensions/src/multiplayer/animation/phonemeplayer.cpp index edf1a1bc..af7b6273 100644 --- a/extensions/src/multiplayer/animation/phonemeplayer.cpp +++ b/extensions/src/multiplayer/animation/phonemeplayer.cpp @@ -188,6 +188,15 @@ void PhonemePlayer::Tick(float p_elapsedMs, const std::vectorWrappedSetLocal2WorldWithWorldDataUpdate(p.savedTransform); - p.roi->SetVisibility(TRUE); + if (p.roi) { + p.roi->WrappedSetLocal2WorldWithWorldDataUpdate(p.savedTransform); + p.roi->SetVisibility(TRUE); + } } m_participants.clear(); @@ -586,6 +588,46 @@ void ScenePlayer::Stop() m_hideOnStop = false; } +void ScenePlayer::NotifyROIDestroyed(LegoROI* p_roi) +{ + if (!m_playing || !p_roi) { + return; + } + + for (auto& p : m_participants) { + if (p.roi == p_roi) { + p.roi = nullptr; + } + if (p.vehicleROI == p_roi) { + p.vehicleROI = nullptr; + } + } + + if (m_roiMap) { + for (MxU32 i = 0; i < m_roiMapSize; i++) { + if (m_roiMap[i] == p_roi) { + m_roiMap[i] = nullptr; + } + } + } + + for (auto& roi : m_ptAtCamROIs) { + if (roi == p_roi) { + roi = nullptr; + } + } + + if (m_animRootROI == p_roi) { + m_animRootROI = nullptr; + } + + if (m_vehicleROI == p_roi) { + m_vehicleROI = nullptr; + } + + m_phonemePlayer.NotifyROIDestroyed(p_roi); +} + void ScenePlayer::CleanupProps() { for (auto* propROI : m_propROIs) { diff --git a/extensions/src/multiplayer/networkmanager.cpp b/extensions/src/multiplayer/networkmanager.cpp index c47f63f3..1e894bdc 100644 --- a/extensions/src/multiplayer/networkmanager.cpp +++ b/extensions/src/multiplayer/networkmanager.cpp @@ -1029,6 +1029,7 @@ void NetworkManager::HandleState(const PlayerStateMsg& p_msg) // Respawn only if display actor changed (not on actorId change) if (it->second->GetDisplayActorIndex() != p_msg.displayActorIndex) { + NotifyAnimationsROIDestroyed(it->second.get()); if (it->second->GetROI()) { m_roiToPlayer.erase(it->second->GetROI()); } @@ -1261,6 +1262,7 @@ void NetworkManager::RemoveRemotePlayer(uint32_t p_peerId) break; } } + NotifyAnimationsROIDestroyed(it->second.get()); if (it->second->GetROI()) { m_roiToPlayer.erase(it->second->GetROI()); } @@ -1280,6 +1282,7 @@ void NetworkManager::RemoveRemotePlayer(uint32_t p_peerId) void NetworkManager::RemoveAllRemotePlayers() { for (auto& [peerId, player] : m_remotePlayers) { + NotifyAnimationsROIDestroyed(player.get()); player->Despawn(); } m_remotePlayers.clear(); @@ -1428,6 +1431,21 @@ void NetworkManager::StopAllPlayback() } } +void NetworkManager::NotifyAnimationsROIDestroyed(RemotePlayer* p_player) +{ + LegoROI* roi = p_player->GetROI(); + LegoROI* vehicleROI = p_player->GetRideVehicleROI(); + + for (auto& [animIndex, scenePlayer] : m_playingAnims) { + if (roi) { + scenePlayer->NotifyROIDestroyed(roi); + } + if (vehicleROI) { + scenePlayer->NotifyROIDestroyed(vehicleROI); + } + } +} + void NetworkManager::UnlockRemotesForAnim(uint16_t p_animIndex) { for (auto& [peerId, player] : m_remotePlayers) {