mirror of
https://github.com/Mudlet/Mudlet
synced 2026-08-13 18:26:27 -04:00
#### Brief overview of PR changes/additions - Profile loading now only considers real `*.xml` saves: an empty QSaveFile temporary left behind by a crash during a save can no longer be loaded as "the profile", which made a profile open with its connection settings intact but every trigger/script seemingly gone. Affected profiles heal themselves on next load by falling back to the newest real save. - Packages that uninstall themselves from their own timer script or event-handler script (a common auto-updater pattern) no longer free the very objects still executing: `TimerUnit`/`ScriptUnit` uninstall now defers deletion while `TTimer::execute()` / `Host::raiseEvent()` are on the call stack, completing the #9337/#9383 fix that already covered triggers/aliases/keys. Deferred timer deletes are flushed before the queued post-uninstall save runs, so removed items cannot be serialized back into the profile. - `Host::saveProfile()`'s background module task no longer reads `writers`/`saveFutures` concurrently with the main thread (data race in the profile save path). #### Motivation for adding to Mudlet Fixes a real-world heap-corruption crash cluster (Sentry MUDLET-32 / MUDLET-2S / MUDLET-48: `STATUS_HEAP_CORRUPTION` on 4.21.0/4.21.1, frames touching lua51/Qt6Core/libpugixml, breadcrumbs showing package uninstall activity around saves) and the profile data loss it caused. #### Other info (issues closed, discussion etc) Root cause of the crashes: #9111 (in the 4.20.1 → 4.21.0 window) changed the `*Unit::uninstall()` methods from unregister-only to immediate `delete`. A package script calling `uninstallPackage()` on its own package then freed objects still on the call stack - use-after-free that poisons the heap, typically detected slightly later during the background save serialization (hence the pugixml/lua frames, aborts mid-save, and zero-byte `....xml.XXXXXX` QSaveFile leftovers in `current/`). #9383 fixed the trigger/alias/key cases; this completes timers and scripts, which reproduce under ASan on current development (heap-use-after-free in `Tree<TScript>::isActive()` / `TTimer::execute()`). Data-loss mechanism (generic): a crash mid-save leaves a 0-byte QSaveFile temporary as the newest file in `current/`; `mudlet::loadProfile()` picked the newest file of any name, tried to load the empty temp, and the profile opened "gutted" (connection details live in separate files and survived). Verified end-to-end with affected profile data and covered by a synthetic regression test. Both new functional tests fail on pre-fix code (`PackageSelfUninstallTest` trips ASan heap-use-after-free; `ProfileLoadTempFileTest` reproduces the data loss) and pass with the fix; full functional suite green (24/24). Known remaining (pre-existing) issue documented in-code at `Host::pendingXmlSaveFutures()`: module writing still touches `writers` from the background task for profiles that use modules; fixing that properly means moving module serialization back to the main thread and deserves its own PR. **Test case:** 1. Create a package containing a timer or event-handler script that calls `uninstallPackage()` on its own package, and let it fire - no crash, package cleanly removed, next save does not resurrect it. 2. Simulate an interrupted save: place an empty file named like `2026-01-01#12-00-00.xml.AbCdEf` in a profile's `current/` folder with the newest timestamp - the profile still loads the newest real save with all triggers intact, and the temporary no longer appears in Connect → Options → Profile history. 3. `ctest -R "ProfileLoadTempFileTest|PackageSelfUninstallTest"` in an ASan (default Debug) build. Assisted-by: Claude:claude-fable-5 Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Vadim Peretokin <vadim.peretokin@mudlet.org>
193 lines
7.9 KiB
C++
193 lines
7.9 KiB
C++
/***************************************************************************
|
|
* Copyright (C) 2026 by Mudlet Developers *
|
|
* *
|
|
* 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 profile data-loss after a crash during save.
|
|
*
|
|
* Profile saves go through QSaveFile: the data is written to a randomly named
|
|
* temporary next to the target ("<name>.xml.AbCdEf") which is renamed over the
|
|
* real file on commit. If Mudlet dies mid-save, that temporary is left behind,
|
|
* empty, as the NEWEST file in the profile's current/ directory.
|
|
*
|
|
* mudlet::loadProfile() used to load the newest file of ANY name from
|
|
* current/, so after such a crash it would "load" the empty leftover instead
|
|
* of the newest real save: the profile opened with its connection settings
|
|
* (stored in separate files) intact but every trigger/alias/script seemingly
|
|
* wiped out. This test crashes a save in effigy - by planting an empty
|
|
* QSaveFile-style leftover newer than a real save - and verifies the loader
|
|
* skips it and restores the real data.
|
|
*
|
|
* Run with: ctest -R ProfileLoadTempFileTest -V
|
|
*/
|
|
|
|
#include <QtTest/QtTest>
|
|
|
|
#include <QTemporaryDir>
|
|
|
|
#include "Host.h"
|
|
#include "HostManager.h"
|
|
#include "MudletInstanceCoordinator.h"
|
|
#include "TTrigger.h"
|
|
#include "TriggerUnit.h"
|
|
#include "mudlet.h"
|
|
|
|
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 initializeQRCResourcesForProfileLoadTempFileTest();
|
|
|
|
namespace {
|
|
// A minimal but complete profile save holding one trigger - the "guts" whose
|
|
// survival the test asserts:
|
|
const QString scmTriggerName = qsl("synthetic data-loss canary");
|
|
const QString scmProfileXml = qsl(R"(<?xml version="1.0" encoding="UTF-8"?>
|
|
<!DOCTYPE MudletPackage>
|
|
<MudletPackage version="1.001">
|
|
<TriggerPackage>
|
|
<Trigger isActive="yes" isFolder="no" isTempTrigger="no" isMultiline="no" isPerlSlashGOption="no" isColorizerTrigger="no" isFilterTrigger="no" isSoundTrigger="no" isColorTrigger="no" isColorTriggerFg="no" isColorTriggerBg="no">
|
|
<name>synthetic data-loss canary</name>
|
|
<script>-- intentionally empty</script>
|
|
<triggerType>0</triggerType>
|
|
<conditonLineDelta>0</conditonLineDelta>
|
|
<mStayOpen>0</mStayOpen>
|
|
<mCommand></mCommand>
|
|
<packageName></packageName>
|
|
<mFgColor>#ff0000</mFgColor>
|
|
<mBgColor>#ffff00</mBgColor>
|
|
<mSoundFile></mSoundFile>
|
|
<colorTriggerFgColor>#000000</colorTriggerFgColor>
|
|
<colorTriggerBgColor>#000000</colorTriggerBgColor>
|
|
<regexCodeList>
|
|
<string>^synthetic pattern$</string>
|
|
</regexCodeList>
|
|
<regexCodePropertyList>
|
|
<integer>1</integer>
|
|
</regexCodePropertyList>
|
|
</Trigger>
|
|
</TriggerPackage>
|
|
</MudletPackage>
|
|
)");
|
|
} // namespace
|
|
|
|
class ProfileLoadTempFileTest : public QObject
|
|
{
|
|
Q_OBJECT
|
|
|
|
private:
|
|
const QString mProfileName = qsl("ProfileLoadTempFile-Test");
|
|
QTemporaryDir mConfigDir;
|
|
QByteArray mSavedXdg;
|
|
|
|
static bool setModificationTime(const QString& path, const QDateTime& when)
|
|
{
|
|
QFile file(path);
|
|
if (!file.open(QIODevice::ReadWrite)) {
|
|
return false;
|
|
}
|
|
return file.setFileTime(when, QFileDevice::FileModificationTime);
|
|
}
|
|
|
|
// setupConfig() prefers a portable.txt marker over the XDG override; skip
|
|
// rather than report a baffling failure if one is present:
|
|
bool portableMarkerPresent() const
|
|
{
|
|
return QFileInfo::exists(qsl("%1/portable.txt").arg(QCoreApplication::applicationDirPath())) || QFileInfo::exists(qsl("%1/.config/mudlet/portable.txt").arg(QDir::homePath()));
|
|
}
|
|
|
|
private slots:
|
|
void initTestCase()
|
|
{
|
|
initializeQRCResourcesForProfileLoadTempFileTest();
|
|
|
|
// Keep the test hermetic: point the config dir resolution at a
|
|
// temporary directory instead of the user's real profiles.
|
|
QVERIFY(mConfigDir.isValid());
|
|
mSavedXdg = qgetenv("XDG_CONFIG_HOME");
|
|
QVERIFY(QDir().mkpath(qsl("%1/mudlet/profiles").arg(mConfigDir.path())));
|
|
qputenv("XDG_CONFIG_HOME", mConfigDir.path().toUtf8());
|
|
|
|
mudlet::start();
|
|
mudlet::self()->setupConfig();
|
|
mudlet::self()->takeOwnershipOfInstanceCoordinator(std::make_unique<MudletInstanceCoordinator>("MudletInstanceCoordinator"));
|
|
mudlet::self()->init();
|
|
mudlet::self()->setStorePasswordsSecurely(false);
|
|
|
|
if (portableMarkerPresent()) {
|
|
QSKIP("portable.txt marker present - config dir cannot be redirected for this test");
|
|
}
|
|
QVERIFY2(mudlet::getMudletPath(enums::profilesPath).startsWith(mConfigDir.path()), "test config dir redirection did not take effect");
|
|
}
|
|
|
|
void cleanupTestCase()
|
|
{
|
|
delete mudlet::self();
|
|
mSavedXdg.isNull() ? qunsetenv("XDG_CONFIG_HOME") : qputenv("XDG_CONFIG_HOME", mSavedXdg);
|
|
}
|
|
|
|
void test_loaderSkipsLeftoverSaveTemporary()
|
|
{
|
|
// 1. A real save, holding one trigger:
|
|
const QString folder = mudlet::getMudletPath(enums::profileXmlFilesPath, mProfileName);
|
|
QVERIFY(QDir().mkpath(folder));
|
|
const QString xmlPath = qsl("%1/2020-01-01#00-00-00.xml").arg(folder);
|
|
{
|
|
QFile xmlFile(xmlPath);
|
|
QVERIFY(xmlFile.open(QIODevice::WriteOnly | QIODevice::Text));
|
|
QVERIFY(xmlFile.write(scmProfileXml.toUtf8()) > 0);
|
|
}
|
|
|
|
// 2. What a crash mid-save leaves behind: an empty QSaveFile temporary
|
|
// that is the newest file in current/:
|
|
const QString leftoverPath = qsl("%1/2020-01-02#00-00-00.xml.AbCdEf").arg(folder);
|
|
{
|
|
QFile leftover(leftoverPath);
|
|
QVERIFY(leftover.open(QIODevice::WriteOnly));
|
|
}
|
|
const QDateTime now = QDateTime::currentDateTime();
|
|
QVERIFY(setModificationTime(xmlPath, now.addSecs(-3600)));
|
|
QVERIFY(setModificationTime(leftoverPath, now));
|
|
|
|
// 3. Load the profile through the production loader; it must pick the
|
|
// real save, not the newer empty leftover:
|
|
Host* pHost = mudlet::self()->loadProfile(mProfileName, false);
|
|
QVERIFY(pHost);
|
|
QVERIFY2(pHost->mLoadedOk, "loader tried to load a leftover QSaveFile temporary instead of the newest real save");
|
|
QVERIFY2(pHost->getTriggerUnit()->findTrigger(scmTriggerName), "trigger from the real save is missing - the profile lost its data");
|
|
}
|
|
};
|
|
|
|
void initializeQRCResourcesForProfileLoadTempFileTest()
|
|
{
|
|
#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 "ProfileLoadTempFileTest.moc"
|
|
QTEST_MAIN(ProfileLoadTempFileTest)
|