mudlet/test/functional_tests/ProfileSaveShutdownRaceTest.cpp
Vadim Peretokin 9b8e53b6bf
fix: a queued quit no longer runs from inside a profile save (#9813)
#### Brief overview of PR changes/additions

- `Host::saveProfile()` no longer pumps the event loop between marking
the save as started and making the `QFutureWatcher` that retires that
mark. A quit is queued (`closeMudlet()` arms it on a zero timer), so it
could be delivered in that gap and run the entire application shutdown
from inside the save - and the rest of `saveProfile()` then carried on
using a `Host` that teardown had already destroyed.
- The nested `Host::waitForProfileSave()` was what let that shutdown
through: it was waiting for a finish notification whose watcher did not
exist yet, and its escape hatch counted a thousand event loop passes,
which on a fast machine are over in well under a millisecond. It is
bounded in wall-clock time now, waits out the background writes on every
pass, and the state it prints if it does give up says something that can
be acted on.
- `saveProfileAs()` had the same pump, and announced
`profileSaveStarted()` before finding out it was going to refuse - which
left the editor's Save Profile action disabled and captioned "Saving…"
with no `profileSaveFinished()` ever coming.

#### Motivation for adding to Mudlet

Uninstall a package and quit straight away and Mudlet can crash on the
way out. It showed up on CI as an intermittent SIGSEGV after a fully
green run, on macOS arm64 and windows64 but not on the slower legs,
preceded by `waitForProfileSave() WARNING - save did not complete after
1000 event loop iterations. State: mWritingHostAndModules=true, writers
pending=0` - which is exactly what a save looks like in the window
between the mark and the watcher.

#### Other info (issues closed, discussion etc)

Closes #9807. Adjacent to #9653/#9684 and #9690, and composes with them:
this is the re-entrancy that opened the window rather than another
dangling pointer.

Reproduced deterministically under AddressSanitizer as a
heap-use-after-free in `Host::pendingXmlSaveFutures()` called from
`Host::saveProfile()`, on a `Host` freed by `~Host()`. Draining a save
that a package change has only queued was considered and left out: the
close path never reaches it (it has always either just started a save or
found one running), and it would turn a multi-select module import from
one coalesced save into one full save per module.

**Test case:** uninstall a package, then quit Mudlet immediately - it
exits cleanly. New `ProfileSaveShutdownRaceTest`; both cases verified to
fail against the unfixed source, the shutdown one by aborting the run
under the sanitizer.

Assisted-by: Claude:claude-opus-5
2026-08-13 15:22:50 +02:00

266 lines
11 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. *
***************************************************************************/
/*
* Regression test for the quit that ran from inside the profile save it was
* meant to wait for (#9807).
*
* Host::saveProfile() marks the save as started and only afterwards makes the
* watcher that will report it finished. It pumped the event loop in between, and
* a quit is queued (closeMudlet() arms it on a zero timer), so the quit could be
* delivered in that gap and run the whole application shutdown from inside the
* save. The close's own Host::waitForProfileSave() then waited for a finish
* notification whose watcher did not exist yet, gave up on it, and let teardown
* destroy the Host that the rest of saveProfile() went on to use. The symptom is
* an intermittent SIGSEGV at the end of an otherwise green run, on fast machines
* only, because the cap it gave up on counted event loop passes rather than
* time.
*
* Uninstalling a package is what makes this likely in practice: it queues its
* save rather than starting it, so a save can begin only moments before a quit.
*
* Run with: ctest -R ProfileSaveShutdownRaceTest -V
*/
#include <QtTest/QtTest>
#include <QPointer>
#include <QTemporaryDir>
#include <chrono>
#include "Host.h"
#include "HostManager.h"
#include "MudletInstanceCoordinator.h"
#include "TelnetServerStub.h"
#include "dlgConnectionProfiles.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();
void initializeQRCResourcesForProfileSaveShutdownRaceTest();
// Counts the warning waitForProfileSave() logs when it stops waiting for a save it has
// not been told the end of. Nothing here may provoke it.
static QtMessageHandler previousMessageHandler = nullptr;
static int gaveUpWaitingWarnings = 0;
static void countGiveUpWarnings(QtMsgType type, const QMessageLogContext& context, const QString& message)
{
if (message.contains(QLatin1String("waitForProfileSave() WARNING"))) {
++gaveUpWaitingWarnings;
}
if (previousMessageHandler) {
previousMessageHandler(type, context, message);
}
}
class ProfileSaveShutdownRaceTest : public QObject
{
Q_OBJECT
private:
TelnetServerStub* mpServer = nullptr;
Host* mpHost = nullptr;
const QString mProfileName = qsl("ProfileSaveShutdownRace-Test");
const QString mLocalhost = qsl("localhost");
QString mPort; // the stub's actual ephemeral port
QTemporaryDir mConfigDir;
QTemporaryDir mExportDir;
QByteArray mSavedXdg;
// A package the profile keeps, so that what a save writes can be told apart from a
// profile XML some earlier save left on disk.
const QString mKeptPackage = qsl("shutdown-race-kept");
bool mCloseAccepted = false;
static void deleteProfileDirectory(const QString& profileName)
{
QDir dir(mudlet::getMudletPath(enums::profileHomePath, profileName));
if (dir.exists()) {
dir.removeRecursively();
}
}
// Whether needle appears in the profile saved last - what actually landed on disk,
// rather than what a save signal says was attempted. Saves are named for the time
// they were taken, so the last in name order is the newest.
static bool lastSavedProfileContains(const QString& profileName, const QString& needle)
{
const QDir directory(mudlet::getMudletPath(enums::profileXmlFilesPath, profileName));
const QStringList saved = directory.entryList(QStringList{qsl("*.xml")}, QDir::Files, QDir::Name);
if (saved.isEmpty()) {
return false;
}
QFile file(directory.absoluteFilePath(saved.last()));
if (!file.open(QFile::ReadOnly | QFile::Text)) {
return false;
}
return QString::fromUtf8(file.readAll()).contains(needle);
}
// Utility function to manually start a profile like a user would do via the GUI
void startProfile(const QString& profileName, const QString& address, const QString& port)
{
QTimer::singleShot(0ms, qApp, [profileName, address, port]() {
mudlet::self()->startAutoLogin({});
QTest::qWait(100ms);
QTest::mouseClick(mudlet::self()->mpConnectionDialog->new_profile_button, Qt::LeftButton);
QTest::qWait(100ms);
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.");
}
}
private slots:
void initTestCase()
{
initializeQRCResourcesForProfileSaveShutdownRaceTest();
// Keep the test hermetic: point the config dir resolution at a temporary
// directory instead of the user's real profiles.
QVERIFY(mConfigDir.isValid());
QVERIFY(mExportDir.isValid());
mSavedXdg = qgetenv("XDG_CONFIG_HOME");
QVERIFY(QDir().mkpath(qsl("%1/mudlet/profiles").arg(mConfigDir.path())));
qputenv("XDG_CONFIG_HOME", mConfigDir.path().toUtf8());
mpServer = new TelnetServerStub(qApp);
mpServer->start(mLocalhost, 0); // ephemeral OS-assigned port avoids collisions across concurrent test runs
mPort = QString::number(mpServer->serverPort());
mudlet::start();
mudlet::self()->setupConfig();
mudlet::self()->takeOwnershipOfInstanceCoordinator(std::make_unique<MudletInstanceCoordinator>(qsl("MudletInstanceCoordinator")));
mudlet::self()->init();
mudlet::self()->setStorePasswordsSecurely(false);
deleteProfileDirectory(mProfileName);
previousMessageHandler = qInstallMessageHandler(countGiveUpWarnings);
startProfile(mProfileName, mLocalhost, mPort);
QVERIFY2(mpHost, "No active host after profile creation");
mpHost->mInstalledPackages << mKeptPackage;
}
void cleanupTestCase()
{
qInstallMessageHandler(previousMessageHandler);
previousMessageHandler = nullptr;
mpHost = nullptr;
delete mpServer;
mpServer = nullptr;
deleteProfileDirectory(mProfileName);
delete mudlet::self();
mSavedXdg.isNull() ? qunsetenv("XDG_CONFIG_HOME") : qputenv("XDG_CONFIG_HOME", mSavedXdg);
}
// A "Save Profile As" that is refused because a save is already running may not
// announce one: the matching profileSaveFinished() only ever comes from a save that
// really runs, so the editor would be left with its Save Profile action disabled
// and captioned "Saving…" for good.
void test_aRefusedSaveAsAnnouncesNoSave()
{
mpHost->waitForProfileSave();
auto [ok, filename, error] = mpHost->saveProfile();
QVERIFY2(ok, qPrintable(error));
QSignalSpy saveSpy(mpHost, &Host::profileSaveStarted);
const QString exportPath = mExportDir.filePath(qsl("refused-save-as.xml"));
auto [exported, exportedTo, exportError] = mpHost->saveProfileAs(exportPath);
QVERIFY2(!exported, "Save As went ahead while a save was already running");
QCOMPARE(saveSpy.count(), 0);
mpHost->waitForProfileSave();
QVERIFY2(!QFile::exists(exportPath), "The refused Save As wrote a file anyway");
QCOMPARE(gaveUpWaitingWarnings, 0);
}
// ...and a quit, which Mudlet always queues, may not be delivered while a save is
// marked as started but has no watcher yet. This one destroys the profile, so it
// stays last: anything after it would run without one.
void test_aQueuedQuitDoesNotRunFromInsideTheSave()
{
mpHost->waitForProfileSave();
Host* host = mpHost;
const QPointer<Host> hostGuard(host);
// Queued on a zero timer, as mudlet::armForceClose() does, and doing to this
// one profile what mudlet::closeEvent() and the mudlet::closeHost() after it
// do: forceClose() keeps TMainConsole::closeEvent() from asking whether to
// save, which would block on a modal dialog, and deleteHost() is the step that
// destroys the Host.
QTimer::singleShot(0ms, qApp, [this, host]() {
host->forceClose();
mCloseAccepted = host->requestClose();
mudlet::self()->getHostManager().deleteHost(mProfileName);
});
auto [ok, filename, error] = mpHost->saveProfile();
QVERIFY2(ok, qPrintable(error));
QVERIFY2(!hostGuard.isNull(), "The event loop ran the quit from inside the save and destroyed the profile it was saving");
QCOMPARE(gaveUpWaitingWarnings, 0);
// Now let the quit run where it belongs, with the save it has to wait for
// already properly under way.
mpHost = nullptr;
QTRY_VERIFY_WITH_TIMEOUT(hostGuard.isNull(), 10000);
QVERIFY2(mCloseAccepted, "Closing the profile was refused");
QCOMPARE(gaveUpWaitingWarnings, 0);
QVERIFY2(lastSavedProfileContains(mProfileName, mKeptPackage), "The save the quit waited for reached no profile on disk");
}
};
void initializeQRCResourcesForProfileSaveShutdownRaceTest()
{
#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();
}
#include "ProfileSaveShutdownRaceTest.moc"
QTEST_MAIN(ProfileSaveShutdownRaceTest)