From 24d8955531a3f3aaccdb0a63cab95f7be4419ed5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 12 Oct 2026 14:32:59 +0200 Subject: [PATCH] UI: Handle a caller finishing itself from dialogFinished() That set dialogFinished_ to the caller, which processFinishDialog() then deleted while it was still in the stack, leaking the popup. Take the pointer out first and loop. Nothing does this now (MemStickScreen and DevMenuScreen work around it), but it's easy to do by accident. Co-Authored-By: Claude Opus 5.5 --- Common/UI/ScreenManager.cpp | 53 ++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/Common/UI/ScreenManager.cpp b/Common/UI/ScreenManager.cpp index 3987a7594d..ff7aa55c43 100644 --- a/Common/UI/ScreenManager.cpp +++ b/Common/UI/ScreenManager.cpp @@ -414,34 +414,37 @@ Screen *ScreenManager::dialogParent(const Screen *dialog) const { } void ScreenManager::processFinishDialog() { - if (dialogFinished_) { - { - // Another dialog may have been pushed before the render, so search for it. - Screen *caller = dialogParent(dialogFinished_); - bool erased = false; - for (size_t i = 0; i < stack_.size(); ++i) { - if (stack_[i].screen == dialogFinished_) { - stack_[i].screen->focusChanged(ScreenFocusChange::FOCUS_LOST_TOP); - stack_.erase(stack_.begin() + i); - erased = true; - } - } + // The caller's dialogFinished() can finish the caller too, which sets dialogFinished_ again. So take it + // out first (or we'd delete the caller while it's still in the stack) and loop to process the new one. + while (dialogFinished_) { + const Screen *finished = dialogFinished_; + const DialogResult result = dialogResult_; + dialogFinished_ = nullptr; - if (erased && !stack_.empty()) { - stack_.back().screen->focusChanged(ScreenFocusChange::FOCUS_BECAME_TOP); - } - - if (!caller) { - ERROR_LOG(Log::UI, "ERROR: no top screen when finishing dialog"); - } else if (caller != topScreen()) { - // The caller may get confused if we call dialogFinished() now. - WARN_LOG(Log::UI, "Skipping non-top dialog when finishing dialog."); - } else { - caller->dialogFinished(dialogFinished_, dialogResult_); + // Another dialog may have been pushed before the render, so search for it. + Screen *caller = dialogParent(finished); + bool erased = false; + for (size_t i = 0; i < stack_.size(); ++i) { + if (stack_[i].screen == finished) { + stack_[i].screen->focusChanged(ScreenFocusChange::FOCUS_LOST_TOP); + stack_.erase(stack_.begin() + i); + erased = true; } } - delete dialogFinished_; - dialogFinished_ = nullptr; + + if (erased && !stack_.empty()) { + stack_.back().screen->focusChanged(ScreenFocusChange::FOCUS_BECAME_TOP); + } + + if (!caller) { + ERROR_LOG(Log::UI, "ERROR: no top screen when finishing dialog"); + } else if (caller != topScreen()) { + // The caller may get confused if we call dialogFinished() now. + WARN_LOG(Log::UI, "Skipping non-top dialog when finishing dialog."); + } else { + caller->dialogFinished(finished, result); + } + delete finished; } }