From 321bcf5c4437e8ccc599be9004ad1590a6639c46 Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Sun, 10 Dec 2023 19:09:51 +0100 Subject: [PATCH] GUIFormspecMenu: Fix race condition between quit event and cleanup in Game (#14010) To not instantly free GUIFormSpec upon close/quit, Game periodically cleans up the remaining instance on the next frame. When a new formspec is received and processed after closing the previous formspec but before the cleanup in Game, the formspec would be closed regardless. This now re-creates the formspec when the old one is already pending for removal. --- src/client/game.cpp | 1 + src/gui/guiFormSpecMenu.cpp | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/client/game.cpp b/src/client/game.cpp index cc8891547..ea57f5085 100644 --- a/src/client/game.cpp +++ b/src/client/game.cpp @@ -4124,6 +4124,7 @@ void Game::updateFrame(ProfilerGraph *graph, RunStats *stats, f32 dtime, break; if (formspec->getReferenceCount() == 1) { + // See GUIFormSpecMenu::create what refcnt = 1 means m_game_ui->deleteFormspec(); break; } diff --git a/src/gui/guiFormSpecMenu.cpp b/src/gui/guiFormSpecMenu.cpp index ad1f30005..445c5b902 100644 --- a/src/gui/guiFormSpecMenu.cpp +++ b/src/gui/guiFormSpecMenu.cpp @@ -138,11 +138,23 @@ void GUIFormSpecMenu::create(GUIFormSpecMenu *&cur_formspec, Client *client, gui::IGUIEnvironment *guienv, JoystickController *joystick, IFormSource *fs_src, TextDest *txt_dest, const std::string &formspecPrepend, ISoundManager *sound_manager) { + if (cur_formspec && cur_formspec->getReferenceCount() == 1) { + /* + Why reference count == 1? Reason: + 1 on creation (see "drop()" remark below) + +1 for being a guiroot child + +1 when focused (CGUIEnvironment::setFocus) + + Hence re-create the formspec when it's existing without any parent. + */ + cur_formspec->drop(); + cur_formspec = nullptr; + } + if (cur_formspec == nullptr) { cur_formspec = new GUIFormSpecMenu(joystick, guiroot, -1, &g_menumgr, client, guienv, client->getTextureSource(), sound_manager, fs_src, txt_dest, formspecPrepend); - cur_formspec->doPause = false; /* Caution: do not call (*cur_formspec)->drop() here -- @@ -151,12 +163,13 @@ void GUIFormSpecMenu::create(GUIFormSpecMenu *&cur_formspec, Client *client, remaining reference (i.e. the menu was removed) and delete it in that case. */ - } else { cur_formspec->setFormspecPrepend(formspecPrepend); cur_formspec->setFormSource(fs_src); cur_formspec->setTextDest(txt_dest); } + + cur_formspec->doPause = false; } void GUIFormSpecMenu::removeTooltip()