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.
This commit is contained in:
SmallJoker 2023-12-10 19:09:51 +01:00 committed by GitHub
parent 689aaf50b3
commit 321bcf5c44
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 16 additions and 2 deletions

@ -4124,6 +4124,7 @@ void Game::updateFrame(ProfilerGraph *graph, RunStats *stats, f32 dtime,
break; break;
if (formspec->getReferenceCount() == 1) { if (formspec->getReferenceCount() == 1) {
// See GUIFormSpecMenu::create what refcnt = 1 means
m_game_ui->deleteFormspec(); m_game_ui->deleteFormspec();
break; break;
} }

@ -138,11 +138,23 @@ void GUIFormSpecMenu::create(GUIFormSpecMenu *&cur_formspec, Client *client,
gui::IGUIEnvironment *guienv, JoystickController *joystick, IFormSource *fs_src, gui::IGUIEnvironment *guienv, JoystickController *joystick, IFormSource *fs_src,
TextDest *txt_dest, const std::string &formspecPrepend, ISoundManager *sound_manager) 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) { if (cur_formspec == nullptr) {
cur_formspec = new GUIFormSpecMenu(joystick, guiroot, -1, &g_menumgr, cur_formspec = new GUIFormSpecMenu(joystick, guiroot, -1, &g_menumgr,
client, guienv, client->getTextureSource(), sound_manager, fs_src, client, guienv, client->getTextureSource(), sound_manager, fs_src,
txt_dest, formspecPrepend); txt_dest, formspecPrepend);
cur_formspec->doPause = false;
/* /*
Caution: do not call (*cur_formspec)->drop() here -- 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) remaining reference (i.e. the menu was removed)
and delete it in that case. and delete it in that case.
*/ */
} else { } else {
cur_formspec->setFormspecPrepend(formspecPrepend); cur_formspec->setFormspecPrepend(formspecPrepend);
cur_formspec->setFormSource(fs_src); cur_formspec->setFormSource(fs_src);
cur_formspec->setTextDest(txt_dest); cur_formspec->setTextDest(txt_dest);
} }
cur_formspec->doPause = false;
} }
void GUIFormSpecMenu::removeTooltip() void GUIFormSpecMenu::removeTooltip()