mudlet/test/functional_tests/ScriptEventHandlerLifetimeTest.cpp
Vadim Peretokin a8b0b06c32
fix: Mudlet no longer crashes when adding an event handler after switching scripts (#9839)
#### Brief overview of PR changes/additions
- `slot_scriptsSelected()` now drops the noted "Add User Event" item
after tearing the Registered Events list down, instead of relying on
`saveScript()` doing it beforehand. `QListWidget::clear()` drops the
selection before it deletes the items, and that selection change runs
`slot_scriptMainAreaEditHandler()`, which re-notes the item that is
about to be freed.
- Also releases the row the "-" button takes out of that list -
`takeItem()` hands ownership over, and the returned item was being
dropped.
- New `ScriptEventHandlerLifetimeTest` covers the four ways the list
gets torn down while an entry is noted (switch script, re-click the same
script, add a script, jump from the search results), plus renaming and
deleting so the fix cannot overreach.

#### Motivation for adding to Mudlet
Pressing "+" after switching scripts dereferenced freed memory and
killed the client, losing whatever was unsaved.

#### Other info (issues closed, discussion etc)
Fixes #9835. Reported on Windows 10 / Mudlet 4.22.0, and reproduced on
Windows 11 against both 4.22.0 and the current PTB.

Confirmed under ASan as a `heap-use-after-free` in
`QListWidgetItem::text()` from `slot_scriptMainAreaAddHandler()`, freed
by `QListModel::clear()` from `slot_scriptsSelected()`. Where the
replacement item happens to land on the freed block there is no crash
and "+" silently renames the wrong entry instead.

Note this also stops a name typed into "Add User Event" but never added
from following you to the next script.

**Test case:** Script editor > new script, type an event name into "Add
User Event" and press "+", save. Make a second script, save. Select the
first script, click its entry under "Registered Events", then click the
second script and press "+" - 4.22.0 crashes, this branch adds the
handler to the second script.

Assisted-by: Claude:claude-opus-5
Assisted-by: Claude:claude-fable-5
Signed-off-by: Vadim Peretokin <vadim.peretokin@mudlet.org>

---------

Signed-off-by: Vadim Peretokin <vadim.peretokin@mudlet.org>
2026-08-13 13:37:57 +02:00

454 lines
18 KiB
C++

/***************************************************************************
* Copyright (C) 2026 by Vadim Peretokin - vadim.peretokin@mudlet.org *
* *
* This program is free software; you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation; either version 2 of the License, or *
* (at your option) any later version. *
* *
* This program is distributed in the hope that it will be useful, *
* but WITHOUT ANY WARRANTY; without even the implied warranty of *
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the *
* GNU General Public License for more details. *
* *
* You should have received a copy of the GNU General Public License *
* along with this program; if not, write to the *
* Free Software Foundation, Inc., *
* 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *
***************************************************************************/
#include <QtTest/QtTest>
#include <chrono>
#include "EditorUndoStack.h"
#include "Host.h"
#include "MudletInstanceCoordinator.h"
#include "ScriptUnit.h"
#include "TScript.h"
#include "TTreeWidget.h"
#include "TelnetServerStub.h"
#include "ctelnet.h"
#include "dlgConnectionProfiles.h"
#include "dlgScriptsMainArea.h"
#include "dlgTriggerEditor.h"
#include "mudlet.h"
using namespace std::chrono_literals;
extern void qInitResources_mudlet();
extern void qInitResources_qm();
extern void qInitResources_additional_splash_screens();
extern void qInitResources_mudlet_fonts_common();
extern void qInitResources_mudlet_fonts_posix();
static void initializeQRCResources()
{
#ifdef INCLUDE_VARIABLE_SPLASH_SCREEN
qInitResources_additional_splash_screens();
#endif
#ifdef INCLUDE_FONTS
qInitResources_mudlet_fonts_common();
#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
qInitResources_mudlet_fonts_posix();
#endif
#endif
qInitResources_mudlet();
qInitResources_qm();
}
// Run with: ctest -R ScriptEventHandlerLifetimeTest -V
//
// These pin down the noted "Add User Event" item being dropped whenever the "Registered
// Events" list is torn down, so "+" cannot reach a freed item (#9835). For why the note
// outlives the items at all, see the comment on the
// slot_scriptMainAreaClearHandlerSelection() call in dlgTriggerEditor::slot_scriptsSelected().
class ScriptEventHandlerLifetimeTest : public QObject
{
Q_OBJECT
private:
TelnetServerStub* mpServer = nullptr;
dlgTriggerEditor* mpEditor = nullptr;
Host* mpHost = nullptr;
const QString mProfileName = qsl("ScriptEventHandlerLifetime-Test-Profile");
QString mPort;
const QString mLocalhost = qsl("localhost");
void deleteProfileDirectory(const QString& profileName)
{
const QString path = mudlet::getMudletPath(enums::profileHomePath, profileName);
QDir dir(path);
if (dir.exists()) {
dir.removeRecursively();
}
}
void startProfile(const QString& profileName, const QString& address, const QString& port)
{
QTimer::singleShot(0ms, qApp, [profileName, address, port]() {
mudlet::self()->startAutoLogin({});
QTest::qWait(100ms);
Q_ASSERT_X(mudlet::self()->mpConnectionDialog, "startProfile", "Connection dialog not initialized");
Q_ASSERT_X(mudlet::self()->mpConnectionDialog->new_profile_button, "startProfile", "New profile button not found");
QTest::mouseClick(mudlet::self()->mpConnectionDialog->new_profile_button, Qt::LeftButton);
QTest::qWait(100ms);
Q_ASSERT_X(QApplication::focusWidget(), "startProfile", "No widget has focus after clicking new profile button");
QTest::keyClicks(QApplication::focusWidget(), profileName);
QTest::qWait(100ms);
QTest::keyClick(QApplication::focusWidget(), Qt::Key_Tab);
QTest::qWait(100ms);
QTest::keyClicks(QApplication::focusWidget(), address);
QTest::qWait(100ms);
QTest::keyClick(QApplication::focusWidget(), Qt::Key_Tab);
QTest::qWait(100ms);
QTest::keyClicks(QApplication::focusWidget(), port);
QTest::qWait(100ms);
QTest::keyClick(QApplication::focusWidget(), Qt::Key_Return);
});
QSignalSpy spy(mudlet::self(), &mudlet::signal_profileLoaded);
if (!spy.wait(5000)) {
QFAIL("Profile took too long to load.");
}
mpHost = mudlet::self()->getActiveHost();
if (!mpHost) {
QFAIL("No active host available for the test.");
}
QSignalSpy spy2(&(mpHost->mTelnet), &cTelnet::signal_connected);
if (!spy2.wait(2000)) {
QFAIL("Could not connect with the host.");
}
}
QListWidget* handlerList() const { return mpEditor->mpScriptsMainArea->listWidget_script_registered_event_handlers; }
QLineEdit* handlerEntry() const { return mpEditor->mpScriptsMainArea->lineEdit_script_event_handler_entry; }
QStringList savedHandlersOf(QTreeWidgetItem* pTreeItem) const
{
if (!pTreeItem) {
return {};
}
TScript* pScript = mpHost->getScriptUnit()->getScript(pTreeItem->data(0, Qt::UserRole).toInt());
return pScript ? pScript->getEventHandlerList() : QStringList{};
}
QTreeWidgetItem* addSavedScript(const QString& name, const QStringList& handlers)
{
mpEditor->treeWidget_scripts->setCurrentItem(mpEditor->mpScriptsBaseItem);
mpEditor->addScript(false);
QTest::qWait(50ms);
QTreeWidgetItem* pTreeItem = mpEditor->mpCurrentScriptItem;
if (!pTreeItem) {
QTest::qFail("addScript() left no current script item", __FILE__, __LINE__);
return nullptr;
}
mpEditor->mpScriptsMainArea->lineEdit_script_name->setText(name);
for (const QString& handler : handlers) {
handlerEntry()->setText(handler);
mpEditor->slot_scriptMainAreaAddHandler();
}
if (handlerList()->count() != handlers.count()) {
QTest::qFail("the handlers did not all reach the Registered Events list", __FILE__, __LINE__);
return nullptr;
}
mpEditor->slot_saveSelectedItem();
QTest::qWait(50ms);
return pTreeItem;
}
QTreeWidgetItem* findEventHandlerSearchResult() const
{
QList<QTreeWidgetItem*> pending;
for (int i = 0; i < mpEditor->treeWidget_searchResults->topLevelItemCount(); ++i) {
pending.append(mpEditor->treeWidget_searchResults->topLevelItem(i));
}
while (!pending.isEmpty()) {
QTreeWidgetItem* pResult = pending.takeFirst();
if (pResult->data(0, dlgTriggerEditor::TypeRole).toInt() == dlgTriggerEditor::SearchResultIsEventHandler) {
return pResult;
}
for (int i = 0; i < pResult->childCount(); ++i) {
pending.append(pResult->child(i));
}
}
return nullptr;
}
void removeScripts(const QList<QTreeWidgetItem*>& treeItems)
{
for (QTreeWidgetItem* pTreeItem : treeItems) {
mpEditor->treeWidget_scripts->setCurrentItem(pTreeItem);
mpEditor->slot_deleteItemOrGroup();
QTest::qWait(20ms);
}
mpEditor->mpUndoStack->clear();
}
private slots:
void initTestCase()
{
initializeQRCResources();
mpServer = new TelnetServerStub(qApp);
mpServer->start(mLocalhost, 0); // ephemeral OS-assigned port avoids collisions across concurrent test runs
QVERIFY2(mpServer->isListening(), qPrintable(qsl("TelnetServerStub failed to start: %1").arg(mpServer->errorString())));
mPort = QString::number(mpServer->serverPort());
mudlet::start();
mudlet::self()->setupConfig();
mudlet::self()->takeOwnershipOfInstanceCoordinator(std::make_unique<MudletInstanceCoordinator>("MudletInstanceCoordinator"));
mudlet::self()->init();
mudlet::self()->setStorePasswordsSecurely(false);
deleteProfileDirectory(mProfileName);
startProfile(mProfileName, mLocalhost, mPort);
mudlet::self()->slot_showScriptDialog();
QTest::qWait(200ms);
mpEditor = mpHost->mpEditorDialog;
QVERIFY2(mpEditor != nullptr, "Editor dialog should be created");
mpEditor->slot_showScripts();
QTest::qWait(100ms);
}
void init()
{
if (!mpEditor) {
QFAIL("the editor was never created, the profile setup must have failed");
}
}
void cleanupTestCase()
{
mpEditor = nullptr;
mpHost = nullptr;
delete mpServer;
mpServer = nullptr;
deleteProfileDirectory(mProfileName);
delete mudlet::self();
}
void testSwitchingScriptsDropsTheNotedHandler()
{
QTreeWidgetItem* pScriptA = addSavedScript(qsl("ScriptA"), {qsl("myTestEvent")});
QTreeWidgetItem* pScriptB = addSavedScript(qsl("ScriptB"), {});
mpEditor->treeWidget_scripts->setCurrentItem(pScriptA);
QTest::qWait(50ms);
QCOMPARE(handlerList()->count(), 1);
// same signal path as a real click on the entry
handlerList()->setCurrentRow(0);
QTest::qWait(50ms);
QCOMPARE(mpEditor->mpScriptsMainAreaEditHandlerItem, handlerList()->item(0));
QCOMPARE(mpEditor->mIsScriptsMainAreaEditHandler, true);
QCOMPARE(handlerEntry()->text(), qsl("myTestEvent"));
mpEditor->treeWidget_scripts->setCurrentItem(pScriptB);
QTest::qWait(50ms);
QCOMPARE(mpEditor->mpScriptsMainAreaEditHandlerItem, nullptr);
QCOMPARE(mpEditor->mIsScriptsMainAreaEditHandler, false);
QCOMPARE(handlerEntry()->text(), QString());
handlerEntry()->setText(qsl("otherEvent"));
mpEditor->slot_scriptMainAreaAddHandler();
mpEditor->slot_saveSelectedItem();
QTest::qWait(50ms);
QCOMPARE(savedHandlersOf(pScriptB), QStringList{qsl("otherEvent")});
QCOMPARE(savedHandlersOf(pScriptA), QStringList{qsl("myTestEvent")});
removeScripts({pScriptA, pScriptB});
}
// One click on a tree entry emits itemSelectionChanged and then itemClicked, so
// slot_scriptsSelected() runs twice and the second run, on the already-current item,
// is an easy-to-miss second teardown. The replacement item can land on the freed one,
// in which case "+" silently renames it instead of crashing.
void testReselectingTheSameScriptDropsTheNotedHandler()
{
QTreeWidgetItem* pScript = addSavedScript(qsl("SoloScript"), {qsl("myTestEvent")});
mpEditor->treeWidget_scripts->setCurrentItem(pScript);
QTest::qWait(50ms);
QCOMPARE(handlerList()->count(), 1);
handlerList()->setCurrentRow(0);
QTest::qWait(50ms);
QCOMPARE(mpEditor->mpScriptsMainAreaEditHandlerItem, handlerList()->item(0));
emit mpEditor->treeWidget_scripts->itemClicked(pScript, 0);
QTest::qWait(50ms);
QCOMPARE(mpEditor->mpScriptsMainAreaEditHandlerItem, nullptr);
QCOMPARE(mpEditor->mIsScriptsMainAreaEditHandler, false);
handlerEntry()->setText(qsl("secondEvent"));
mpEditor->slot_scriptMainAreaAddHandler();
mpEditor->slot_saveSelectedItem();
QTest::qWait(50ms);
QCOMPARE(savedHandlersOf(pScript), (QStringList{qsl("myTestEvent"), qsl("secondEvent")}));
removeScripts({pScript});
}
// Dropping the note takes the "Add User Event" text with it, so a half-typed name
// cannot follow the user to the next script and land on that one instead.
void testTypedButUnaddedTextDoesNotFollowToTheNextScript()
{
QTreeWidgetItem* pScriptA = addSavedScript(qsl("TypedTextA"), {});
QTreeWidgetItem* pScriptB = addSavedScript(qsl("TypedTextB"), {});
mpEditor->treeWidget_scripts->setCurrentItem(pScriptA);
QTest::qWait(50ms);
handlerEntry()->setText(qsl("neverAddedEvent"));
mpEditor->treeWidget_scripts->setCurrentItem(pScriptB);
QTest::qWait(50ms);
QCOMPARE(handlerEntry()->text(), QString());
mpEditor->slot_scriptMainAreaAddHandler();
mpEditor->slot_saveSelectedItem();
QTest::qWait(50ms);
QCOMPARE(savedHandlersOf(pScriptB), QStringList{});
QCOMPARE(savedHandlersOf(pScriptA), QStringList{});
removeScripts({pScriptA, pScriptB});
}
// addScript() points mpCurrentScriptItem at the new script before selecting it, so
// that selection skips the save as well while still tearing the list down.
void testAddingAScriptDropsTheNotedHandler()
{
QTreeWidgetItem* pScript = addSavedScript(qsl("BeforeNewScript"), {qsl("myTestEvent")});
mpEditor->treeWidget_scripts->setCurrentItem(pScript);
QTest::qWait(50ms);
handlerList()->setCurrentRow(0);
QTest::qWait(50ms);
QCOMPARE(mpEditor->mpScriptsMainAreaEditHandlerItem, handlerList()->item(0));
mpEditor->addScript(false);
QTest::qWait(50ms);
QTreeWidgetItem* pNewScript = mpEditor->mpCurrentScriptItem;
QVERIFY(pNewScript != pScript);
QCOMPARE(mpEditor->mpScriptsMainAreaEditHandlerItem, nullptr);
QCOMPARE(mpEditor->mIsScriptsMainAreaEditHandler, false);
mpEditor->mpScriptsMainArea->lineEdit_script_name->setText(qsl("AfterNewScript"));
handlerEntry()->setText(qsl("brandNewEvent"));
mpEditor->slot_scriptMainAreaAddHandler();
mpEditor->slot_saveSelectedItem();
QTest::qWait(50ms);
QCOMPARE(savedHandlersOf(pNewScript), QStringList{qsl("brandNewEvent")});
QCOMPARE(savedHandlersOf(pScript), QStringList{qsl("myTestEvent")});
removeScripts({pScript, pNewScript});
}
// Nothing tears the list down between selecting the entry and pressing "+", so the
// note has to survive here - guards against dropping it too eagerly.
void testRenamingASelectedHandlerStillWorks()
{
QTreeWidgetItem* pScript = addSavedScript(qsl("RenameScript"), {qsl("firstEvent")});
mpEditor->treeWidget_scripts->setCurrentItem(pScript);
QTest::qWait(50ms);
handlerList()->setCurrentRow(0);
QTest::qWait(50ms);
handlerEntry()->setText(qsl("renamedEvent"));
mpEditor->slot_scriptMainAreaAddHandler();
mpEditor->slot_saveSelectedItem();
QTest::qWait(50ms);
QCOMPARE(savedHandlersOf(pScript), QStringList{qsl("renamedEvent")});
removeScripts({pScript});
}
// The "-" button takes the row out of the list widget, which hands its
// ownership over, so this also holds the leak checker over that path.
void testDeletingAHandlerReleasesIt()
{
QTreeWidgetItem* pScript = addSavedScript(qsl("DeleteScript"), {qsl("firstEvent"), qsl("secondEvent")});
mpEditor->treeWidget_scripts->setCurrentItem(pScript);
QTest::qWait(50ms);
QCOMPARE(handlerList()->count(), 2);
handlerList()->setCurrentRow(0);
QTest::qWait(50ms);
mpEditor->slot_scriptMainAreaDeleteHandler();
QTest::qWait(50ms);
QCOMPARE(handlerList()->count(), 1);
QCOMPARE(handlerList()->item(0)->text(), qsl("secondEvent"));
QCOMPARE(mpEditor->mpScriptsMainAreaEditHandlerItem, nullptr);
QCOMPARE(mpEditor->mIsScriptsMainAreaEditHandler, false);
mpEditor->slot_saveSelectedItem();
QTest::qWait(50ms);
QCOMPARE(savedHandlersOf(pScript), QStringList{qsl("secondEvent")});
removeScripts({pScript});
}
// slot_itemSelectedInSearchResults() notes the item by hand rather than through the
// list widget's selection, and only after every teardown of the list - so that note
// has to survive, and the next script selection has to drop it.
void testSearchResultNotesALiveHandler()
{
QTreeWidgetItem* pScriptA = addSavedScript(qsl("SearchScript"), {qsl("searchableEvent")});
QTreeWidgetItem* pScriptB = addSavedScript(qsl("OtherScript"), {});
mpEditor->treeWidget_scripts->setCurrentItem(pScriptB);
QTest::qWait(50ms);
mpEditor->comboBox_searchTerms->insertItem(0, qsl("searchableEvent"));
mpEditor->slot_searchMudletItems(0);
QTest::qWait(50ms);
QTreeWidgetItem* pResult = findEventHandlerSearchResult();
QVERIFY2(pResult != nullptr, "the search found no event handler result to jump to");
mpEditor->slot_itemSelectedInSearchResults(pResult);
QTest::qWait(50ms);
QCOMPARE(mpEditor->mpCurrentScriptItem, pScriptA);
QCOMPARE(handlerList()->count(), 1);
QCOMPARE(mpEditor->mpScriptsMainAreaEditHandlerItem, handlerList()->item(0));
QCOMPARE(mpEditor->mIsScriptsMainAreaEditHandler, true);
QCOMPARE(handlerEntry()->text(), qsl("searchableEvent"));
mpEditor->treeWidget_scripts->setCurrentItem(pScriptB);
QTest::qWait(50ms);
QCOMPARE(mpEditor->mpScriptsMainAreaEditHandlerItem, nullptr);
QCOMPARE(mpEditor->mIsScriptsMainAreaEditHandler, false);
handlerEntry()->setText(qsl("afterSearchEvent"));
mpEditor->slot_scriptMainAreaAddHandler();
mpEditor->slot_saveSelectedItem();
QTest::qWait(50ms);
QCOMPARE(savedHandlersOf(pScriptB), QStringList{qsl("afterSearchEvent")});
QCOMPARE(savedHandlersOf(pScriptA), QStringList{qsl("searchableEvent")});
removeScripts({pScriptA, pScriptB});
}
};
#include "ScriptEventHandlerLifetimeTest.moc"
QTEST_MAIN(ScriptEventHandlerLifetimeTest)