fix: two ways a profile save could lose or resurrect your data (#9704)

#### Brief overview of PR changes/additions
- Variables: the export skipped its refresh whenever the editor's
Variables view was on screen, so anything a script wrote into a saved
variable while it sat open was dropped from the save - including the
session's last save, which is taken with whatever view the editor was
left on. The variables are now read into a throwaway tree, which also
stops a save stranding the editor's variable search results.
- Packages: a save taken while a unit was still executing an item of a
package that had just been uninstalled wrote that package's items back
into the profile, where they returned as orphans the Package Manager
could not remove. The XML writers now skip what the units have queued
for a deferred delete, the module writer included - reloading a module
from a script used to write both the pre- and post-reload copies of its
items into the module file.
- `LuaInterface::getVars()` is now `setjmp`-guarded like every other
Lua-touching method there, so a panic cannot jump past the export's
scope with its variable tree and registry references still held.

#### Motivation for adding to Mudlet
Both are silent data loss in everyday use: quitting with the editor on
the Variables tab, and the `mpkg`/auto-updater shape of uninstalling a
package from a script.

#### Other info (issues closed, discussion etc)
From the 5.0 QA sweep, findings C13 and C14. The variables half re-opens
the loss that `20009c5ec` "fix: variables added while playing are no
longer lost when saving (#9492)" fixed, via the guard it added; the
packages half is the missing counterpart to the self-uninstall deferral
in `276e8bbfd` (#9383) and its follow-ups. #9492's own cases still pass
unchanged.

**Test case:** create a table from the command line, tick it to be saved
in the editor's Variables view, leave the editor there, run `lua
myTable.later = "x"`, quit and reopen - `later` is still there. `ctest
-R 'XMLexportVariablesTest|PackageSelfUninstallTest'` covers both
halves; all 12 new cases were verified to fail against the unfixed
source.

Assisted-by: Claude:claude-opus-5
This commit is contained in:
Vadim Peretokin 2026-08-07 10:14:45 +02:00 committed by GitHub
parent 159d4bbe02
commit 6c67a13826
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 617 additions and 42 deletions

View file

@ -49,6 +49,8 @@ LuaInterface::LuaInterface(lua_State* L)
lua_atpanic(L, &onPanic);
}
// Does not release lrefs: a profile reset closes the lua_State before this
// object is replaced, so unref'ing here would write into a freed state.
LuaInterface::~LuaInterface() = default;
int LuaInterface::onPanic(lua_State* L)
@ -68,6 +70,19 @@ VarUnit* LuaInterface::getVarUnit()
return varUnit.data();
}
lua_State* LuaInterface::getState() const
{
return mL;
}
void LuaInterface::releaseVariableReferences()
{
for (const int ref : std::as_const(lrefs)) {
luaL_unref(mL, LUA_REGISTRYINDEX, ref);
}
lrefs.clear();
}
QStringList LuaInterface::varName(TVar* var)
{
QStringList names;
@ -825,17 +840,19 @@ void LuaInterface::getVars(bool hide)
//returns the base item
// QElapsedTimer t;
// t.start();
// onPanic() longjmp()s to the shared buf, so without a setjmp of our own
// that jump lands in whichever frame set it last - usually one that has
// already returned, taking the caller's scope down with it.
if (setjmp(buf) != 0) {
qWarning() << "LuaInterface::getVars() WARNING - Lua panicked while reading the variables in; the variable tree is incomplete.";
return;
}
lua_pushnil(mL);
depth = 0;
auto global = new TVar();
global->setName("_G", LUA_TSTRING);
global->setValue("{}", LUA_TTABLE);
QListIterator<int> it(lrefs);
while (it.hasNext()) {
const int ref = it.next();
luaL_unref(mL, LUA_REGISTRYINDEX, ref);
}
lrefs.clear();
releaseVariableReferences();
varUnit->clear();
varUnit->setBase(global);
varUnit->addVariable(global);

View file

@ -65,12 +65,15 @@ public:
void renameVar(TVar*);
void createVar(TVar*);
VarUnit* getVarUnit();
// Anything that builds a variable tree and throws it away owes this call:
// ~LuaInterface cannot make it, see there.
void releaseVariableReferences();
bool loadVar(TVar* var);
bool reparentCVariable(TVar* from, TVar* to, TVar* curVar);
bool reparentVariable(QTreeWidgetItem*, QTreeWidgetItem*, QTreeWidgetItem*);
std::pair<bool, QString> validMove(QTreeWidgetItem*);
void getAllChildren(TVar* var, QList<TVar*>* list);
lua_State* getState();
lua_State* getState() const;
static int onPanic(lua_State*);
private:

View file

@ -95,7 +95,7 @@ void XMLexport::writeModuleXML(const QString& moduleName)
auto triggerPackage = mudletPackage.append_child("TriggerPackage");
//we go a level down for all these functions so as to not infinitely nest the module
for (auto& it : pHost->mTriggerUnit.mTriggerRootNodeList) {
if (!it || it->mPackageName != moduleName) {
if (!it || pHost->mTriggerUnit.uninstallList.contains(it) || it->mPackageName != moduleName) {
continue;
}
if (!it->isTemporary() && it->mModuleMember) {
@ -105,7 +105,7 @@ void XMLexport::writeModuleXML(const QString& moduleName)
auto timerPackage = mudletPackage.append_child("TimerPackage");
for (auto& it : pHost->mTimerUnit.mTimerRootNodeList) {
if (!it || it->mPackageName != moduleName) {
if (!it || pHost->mTimerUnit.uninstallList.contains(it) || it->mPackageName != moduleName) {
continue;
}
if (!it->isTemporary() && it->mModuleMember) {
@ -115,7 +115,7 @@ void XMLexport::writeModuleXML(const QString& moduleName)
auto aliasPackage = mudletPackage.append_child("AliasPackage");
for (auto& it : pHost->mAliasUnit.mAliasRootNodeList) {
if (!it || it->mPackageName != moduleName) {
if (!it || pHost->mAliasUnit.uninstallList.contains(it) || it->mPackageName != moduleName) {
continue;
}
if (!it->isTemporary() && it->mModuleMember) {
@ -125,7 +125,7 @@ void XMLexport::writeModuleXML(const QString& moduleName)
auto actionPackage = mudletPackage.append_child("ActionPackage");
for (auto& it : pHost->mActionUnit.mActionRootNodeList) {
if (!it || it->mPackageName != moduleName) {
if (!it || pHost->mActionUnit.uninstallList.contains(it) || it->mPackageName != moduleName) {
continue;
}
if (it->mModuleMember) {
@ -135,7 +135,7 @@ void XMLexport::writeModuleXML(const QString& moduleName)
auto scriptPackage = mudletPackage.append_child("ScriptPackage");
for (auto& it : pHost->mScriptUnit.mScriptRootNodeList) {
if (!it || it->mPackageName != moduleName) {
if (!it || pHost->mScriptUnit.uninstallList.contains(it) || it->mPackageName != moduleName) {
continue;
}
if (it->mModuleMember) {
@ -145,7 +145,7 @@ void XMLexport::writeModuleXML(const QString& moduleName)
auto keyPackage = mudletPackage.append_child("KeyPackage");
for (auto& it : pHost->mKeyUnit.mKeyRootNodeList) {
if (!it || it->mPackageName != moduleName) {
if (!it || pHost->mKeyUnit.uninstallList.contains(it) || it->mPackageName != moduleName) {
continue;
}
if (!it->isTemporary() && it->mModuleMember) {
@ -772,33 +772,37 @@ void XMLexport::writeVariablePackage(Host* pHost, pugi::xml_node& mudletPackage)
}
}
// Refresh the variable tree so it reflects the current Lua state. The tree
// is otherwise only rebuilt at profile load and when the Variables editor
// populates it, so a variable (or saved table member) a script created
// afterwards would be missing here and silently dropped from the saved
// profile. Skip the refresh only while that editor view is on screen: it
// owns the tree and rebuilding it here would invalidate the widget the user
// is interacting with (its variables would stop responding until refreshed).
const bool variablesEditorOnScreen = pHost->mpEditorDialog && pHost->mpEditorDialog->variablesViewActive();
TVar* base = vu->getBase();
if (!variablesEditorOnScreen || !base) {
lI->getVars(false);
base = vu->getBase();
}
// Into a throwaway tree rather than the live one: the Variables editor's
// QTreeWidgetItems point into the live tree, so rebuilding it here would
// strand every one of them. Reusing it as it stands is no good either - only
// the editor rebuilds it, so anything a script did since is missing from it.
LuaInterface saveTimeInterface(lI->getState());
VarUnit* saveTimeUnit = saveTimeInterface.getVarUnit();
// A fresh tree carries no per-variable saved/hidden flags, so isSaved() and
// isHidden() have to answer from these name-keyed sets.
saveTimeUnit->savedVars = vu->savedVars;
saveTimeUnit->hidden = vu->hidden;
saveTimeUnit->hiddenByUser = vu->hiddenByUser;
saveTimeInterface.getVars(false);
if (base) {
if (TVar* base = saveTimeUnit->getBase()) {
QListIterator<TVar*> itVariable(base->getChildren(false));
while (itVariable.hasNext()) {
writeVariable(itVariable.next(), lI, vu, variablePackage);
writeVariable(itVariable.next(), &saveTimeInterface, saveTimeUnit, variablePackage);
}
}
saveTimeInterface.releaseVariableReferences();
}
// A unit busy executing an item of a package being uninstalled can only
// deactivate it; it stays registered in uninstallList until doCleanup() flushes
// it. Such an item is gone as far as the profile is concerned, so no writer that
// walks a root node list may serialize it. The list is empty at any other time.
void XMLexport::writeKeyPackage(const Host* pHost, pugi::xml_node& mudletPackage, bool skipModuleMembers)
{
auto keyPackage = mudletPackage.append_child("KeyPackage");
for (auto it : pHost->mKeyUnit.mKeyRootNodeList) {
if (!it || it->isTemporary() || (skipModuleMembers && it->mModuleMember)) {
if (!it || pHost->mKeyUnit.uninstallList.contains(it) || it->isTemporary() || (skipModuleMembers && it->mModuleMember)) {
continue;
}
writeKey(it, keyPackage);
@ -809,7 +813,7 @@ void XMLexport::writeScriptPackage(const Host* pHost, pugi::xml_node& mudletPack
{
auto scriptPackage = mudletPackage.append_child("ScriptPackage");
for (auto it : pHost->mScriptUnit.mScriptRootNodeList) {
if (!it || (skipModuleMembers && it->mModuleMember)) {
if (!it || pHost->mScriptUnit.uninstallList.contains(it) || (skipModuleMembers && it->mModuleMember)) {
continue;
}
writeScript(it, scriptPackage);
@ -820,7 +824,7 @@ void XMLexport::writeActionPackage(const Host* pHost, pugi::xml_node& mudletPack
{
auto actionPackage = mudletPackage.append_child("ActionPackage");
for (auto it : pHost->mActionUnit.mActionRootNodeList) {
if (!it || (skipModuleMembers && it->mModuleMember)) {
if (!it || pHost->mActionUnit.uninstallList.contains(it) || (skipModuleMembers && it->mModuleMember)) {
continue;
}
writeAction(it, actionPackage);
@ -831,7 +835,7 @@ void XMLexport::writeAliasPackage(const Host* pHost, pugi::xml_node& mudletPacka
{
auto aliasPackage = mudletPackage.append_child("AliasPackage");
for (auto it : pHost->mAliasUnit.mAliasRootNodeList) {
if (!it || (skipModuleMembers && it->mModuleMember)) {
if (!it || pHost->mAliasUnit.uninstallList.contains(it) || (skipModuleMembers && it->mModuleMember)) {
continue;
}
if (!it->isTemporary()) {
@ -844,7 +848,7 @@ void XMLexport::writeTimerPackage(const Host* pHost, pugi::xml_node& mudletPacka
{
auto timerPackage = mudletPackage.append_child("TimerPackage");
for (auto it : pHost->mTimerUnit.mTimerRootNodeList) {
if (!it || (skipModuleMembers && it->mModuleMember)) {
if (!it || pHost->mTimerUnit.uninstallList.contains(it) || (skipModuleMembers && it->mModuleMember)) {
continue;
}
if (!it->isTemporary()) {
@ -857,7 +861,7 @@ void XMLexport::writeTriggerPackage(const Host* pHost, pugi::xml_node& mudletPac
{
auto triggerPackage = mudletPackage.append_child("TriggerPackage");
for (auto it : pHost->mTriggerUnit.mTriggerRootNodeList) {
if (!it || (ignoreModuleMembers && it->mModuleMember)) {
if (!it || pHost->mTriggerUnit.uninstallList.contains(it) || (ignoreModuleMembers && it->mModuleMember)) {
continue;
}
if (!it->isTemporary()) {

View file

@ -9706,11 +9706,6 @@ EditorViewType dlgTriggerEditor::determineViewFromVisibleTree()
return EditorViewType::cmUnknownView;
}
bool dlgTriggerEditor::variablesViewActive() const
{
return isVisible() && mCurrentView == EditorViewType::cmVarsView;
}
EditorViewType dlgTriggerEditor::resolveCurrentView()
{
if (mCurrentView != EditorViewType::cmUnknownView) {

View file

@ -204,9 +204,6 @@ public:
int canRecast(QTreeWidgetItem*, int newNameType, int newValueType);
void saveVar();
void repopulateVars();
// true while the Variables view is the one shown on screen, so a profile
// save can avoid rebuilding the tree out from under the live widget
bool variablesViewActive() const;
void changeView(EditorViewType);
void recurseVariablesUp(QTreeWidgetItem* const, QList<QTreeWidgetItem*>&);
void recurseVariablesDown(QTreeWidgetItem* const, QList<QTreeWidgetItem*>&);

View file

@ -38,23 +38,48 @@
* inside TTimer::execute() / Host::raiseEvent() / TScript::compileScript(); with
* the deferral in place all scenarios complete cleanly.
*
* The second half covers the other side of that deferral: an item whose delete
* is outstanding is still registered, and must not be written back into the
* profile by a save taken before the unit goes idle.
*
* Run with: ctest -R PackageSelfUninstallTest -V
*/
#include <QtTest/QtTest>
#include <QScopeGuard>
#include <QTemporaryDir>
#include "ActionUnit.h"
#include "AliasUnit.h"
#include "Host.h"
#include "HostManager.h"
#include "LuaInterface.h"
#include "MudletInstanceCoordinator.h"
#include "ScriptUnit.h"
#include "TAction.h"
#include "TAlias.h"
#include "TEvent.h"
#include "TScript.h"
#include "TTimer.h"
#include "TTrigger.h"
#include "TimerUnit.h"
#include "TriggerUnit.h"
#include "VarUnit.h"
#include "XMLexport.h"
#include "XMLimport.h"
#include "mudlet.h"
extern "C" {
#if defined(INCLUDE_VERSIONED_LUA_HEADERS)
#include <lua5.1/lauxlib.h>
#include <lua5.1/lua.h>
#else
#include <lauxlib.h>
#include <lua.h>
#endif
}
extern void qInitResources_mudlet();
extern void qInitResources_qm();
extern void qInitResources_additional_splash_screens();
@ -62,6 +87,38 @@ extern void qInitResources_mudlet_fonts_common();
extern void qInitResources_mudlet_fonts_posix();
void initializeQRCResourcesForPackageSelfUninstallTest();
// TriggerUnit only holds its depth inside processDataStream(), so a save at
// depth has to come from a trigger's own script. Stands in for the Lua
// saveProfile() one would call - a bare test Host has no console for that.
static Host* gpMidPassExportHost = nullptr;
static QString gMidPassExportPath;
static QString gMidPassExportedXml;
static int exportProfileMidPass(lua_State* L)
{
Q_UNUSED(L)
gMidPassExportedXml.clear();
if (!gpMidPassExportHost) {
return 0;
}
auto writer = std::make_shared<XMLexport>(gpMidPassExportHost);
// variables included: the only export here that builds the variable tree,
// and it does so with a Lua call frame live
if (!writer->exportPackage(gMidPassExportPath, true, false)) {
qWarning() << "exportProfileMidPass() - the export itself failed";
return 0;
}
QFile file(gMidPassExportPath);
if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
qWarning() << "exportProfileMidPass() - could not read back" << gMidPassExportPath;
return 0;
}
gMidPassExportedXml = QString::fromUtf8(file.readAll());
file.close();
QFile::remove(gMidPassExportPath);
return 0;
}
class PackageSelfUninstallTest : public QObject
{
Q_OBJECT
@ -99,6 +156,7 @@ private slots:
// NB: mLoadedOk is left false on purpose - the deferred saveProfile()
// that uninstallPackage() schedules then declines to run, which this
// console-less test Host could not service anyway.
createKeeperItems();
}
void cleanupTestCase()
@ -257,6 +315,344 @@ private slots:
// save would serialize it back in:
QVERIFY2(!mpHost->getTimerUnit()->findFirstTimer(qsl("selfUninstallTimer")), "uninstalled package timer is still registered");
}
// The trigger route, driven the whole way: the package's own trigger fires,
// uninstalls its package and saves, all inside the pass. Its export is the
// only one here that includes the variables, so it doubles as the check that
// the variable tree can be built from inside a live Lua call frame.
void test_saveFromTriggerScriptDoesNotResurrectItsPackage()
{
const QString packageName = qsl("resurrect-trigger");
mpHost->mInstalledPackages << packageName;
gpMidPassExportHost = mpHost;
gMidPassExportPath = qsl("%1/mid-pass-export.xml").arg(mConfigDir.path());
lua_State* L = mpHost->mLuaInterpreter.getLuaGlobalState();
lua_register(L, "qaExportProfileMidPass", exportProfileMidPass);
QCOMPARE(luaL_dostring(L, "midPassSavedVar = 'saved from inside the pass'"), 0);
mpHost->getLuaInterface()->getVarUnit()->savedVars.insert(qsl("midPassSavedVar"));
auto pGroup = new TTrigger(nullptr, mpHost);
pGroup->setIsFolder(true);
pGroup->registerTrigger();
pGroup->setName(qsl("resurrectTriggerGroup"));
pGroup->mPackageName = packageName;
pGroup->setIsActive(true);
auto pKicker = new TTrigger(pGroup, mpHost);
pKicker->setRegexCodeList({qsl("^resurrect me$")}, {REGEX_PERL});
pKicker->registerTrigger();
QVERIFY2(pKicker->setScript(qsl("uninstallPackage(\"%1\")\nqaExportProfileMidPass()").arg(packageName)), "trigger script failed to compile");
pKicker->setName(qsl("resurrectTriggerKicker"));
pKicker->setIsActive(true);
// a sibling that never runs: the whole group must go, not just the one
// that fired
auto pBystander = new TTrigger(pGroup, mpHost);
pBystander->setRegexCodeList({qsl("^never matched$")}, {REGEX_PERL});
pBystander->registerTrigger();
pBystander->setName(qsl("resurrectTriggerBystander"));
pBystander->setIsActive(true);
QVERIFY2(exportedProfileXml().contains(qsl("resurrectTriggerGroup")), "the trigger group should be in the profile before its package is uninstalled");
mpHost->getTriggerUnit()->processDataStream(qsl("resurrect me"), -1);
QVERIFY2(!mpHost->mInstalledPackages.contains(packageName), "package was not uninstalled");
QVERIFY2(!gMidPassExportedXml.isEmpty(), "the mid-pass export produced nothing to check");
QVERIFY2(!gMidPassExportedXml.contains(qsl("resurrectTriggerGroup")), "a save taken mid-pass wrote the uninstalled package's trigger group back into the profile");
QVERIFY2(!gMidPassExportedXml.contains(qsl("resurrectTriggerKicker")), "a save taken mid-pass wrote the uninstalled package's trigger back into the profile");
QVERIFY2(!gMidPassExportedXml.contains(qsl("resurrectTriggerBystander")), "a save taken mid-pass wrote the uninstalled package's trigger back into the profile");
const QString keeperError = keepersMissingFrom(gMidPassExportedXml);
QVERIFY2(keeperError.isEmpty(), qPrintable(keeperError));
QVERIFY2(gMidPassExportedXml.contains(qsl("saved from inside the pass")), "the variables were not read out of Lua by a save taken from inside a script");
QVERIFY2(mpHost->getTriggerUnit()->findItems(qsl("resurrectTriggerKicker")).empty(), "uninstalled trigger is still registered");
mpHost->getLuaInterface()->getVarUnit()->savedVars.remove(qsl("midPassSavedVar"));
gpMidPassExportHost = nullptr;
}
// The alias route: a package shipping its own "uninstall" alias.
void test_saveFromAliasScriptDoesNotResurrectItsPackage()
{
const QString packageName = qsl("resurrect-alias");
mpHost->mInstalledPackages << packageName;
gpMidPassExportHost = mpHost;
gMidPassExportPath = qsl("%1/mid-pass-alias-export.xml").arg(mConfigDir.path());
lua_register(mpHost->mLuaInterpreter.getLuaGlobalState(), "qaExportProfileMidPass", exportProfileMidPass);
auto pAlias = new TAlias(qsl("resurrectAlias"), mpHost);
pAlias->setRegexCode(qsl("^resurrect me$"));
mpHost->getAliasUnit()->registerAlias(pAlias);
pAlias->mPackageName = packageName;
QVERIFY2(pAlias->setScript(qsl("uninstallPackage(\"%1\")\nqaExportProfileMidPass()").arg(packageName)), "alias script failed to compile");
pAlias->setIsActive(true);
QVERIFY2(exportedProfileXml().contains(qsl("resurrectAlias")), "the alias should be in the profile before its package is uninstalled");
mpHost->getAliasUnit()->processDataStream(qsl("resurrect me"));
QVERIFY2(!mpHost->mInstalledPackages.contains(packageName), "package was not uninstalled");
QVERIFY2(!gMidPassExportedXml.isEmpty(), "the mid-pass export produced nothing to check");
QVERIFY2(!gMidPassExportedXml.contains(qsl("resurrectAlias")), "a save taken mid-pass wrote the uninstalled package's alias back into the profile");
const QString keeperError = keepersMissingFrom(gMidPassExportedXml);
QVERIFY2(keeperError.isEmpty(), qPrintable(keeperError));
QVERIFY2(!mpHost->getAliasUnit()->findFirstAlias(qsl("resurrectAlias")), "uninstalled alias is still registered");
gpMidPassExportHost = nullptr;
}
// The timer route. beginProcessing()/endProcessing() below are the calls
// TTimer::execute() wraps its whole callback in.
void test_saveDuringTimerCallbackDoesNotResurrectItsPackage()
{
const QString packageName = qsl("resurrect-timer");
mpHost->mInstalledPackages << packageName;
auto pTimer = new TTimer(qsl("resurrectTimer"), QTime(0, 0, 30), mpHost);
mpHost->getTimerUnit()->registerTimer(pTimer);
pTimer->mPackageName = packageName;
QVERIFY2(pTimer->setScript(qsl("local noop = true\n")), "timer script failed to compile");
QVERIFY2(exportedProfileXml().contains(qsl("resurrectTimer")), "the timer should be in the profile before its package is uninstalled");
QString xml;
{
mpHost->getTimerUnit()->beginProcessing();
// a failed QVERIFY returns from the slot; a level left on would
// wedge every later test's doCleanup()
const auto depthGuard = qScopeGuard([this]() {
mpHost->getTimerUnit()->endProcessing();
mpHost->getTimerUnit()->doCleanup();
});
QVERIFY(mpHost->uninstallPackage(packageName, enums::PackageModuleType::Package));
xml = exportedProfileXml();
}
QVERIFY2(!xml.isEmpty(), "the export produced nothing to check");
QVERIFY2(!xml.contains(qsl("resurrectTimer")), "a save taken during a timer callback wrote the uninstalled package's timer back into the profile");
const QString keeperError = keepersMissingFrom(xml);
QVERIFY2(keeperError.isEmpty(), qPrintable(keeperError));
QVERIFY2(!mpHost->getTimerUnit()->findFirstTimer(qsl("resurrectTimer")), "uninstalled timer is still registered");
}
// The button route: TAction::execute() holds ActionUnit's depth the same way.
void test_saveDuringButtonScriptDoesNotResurrectItsPackage()
{
const QString packageName = qsl("resurrect-action");
mpHost->mInstalledPackages << packageName;
auto pAction = new TAction(qsl("resurrectAction"), mpHost);
mpHost->getActionUnit()->registerAction(pAction);
pAction->mPackageName = packageName;
QVERIFY2(pAction->setScript(qsl("local noop = true\n")), "button script failed to compile");
QVERIFY2(exportedProfileXml().contains(qsl("resurrectAction")), "the button should be in the profile before its package is uninstalled");
QString xml;
{
mpHost->getActionUnit()->beginProcessing();
const auto depthGuard = qScopeGuard([this]() {
mpHost->getActionUnit()->endProcessing();
mpHost->getActionUnit()->doCleanup();
});
QVERIFY(mpHost->uninstallPackage(packageName, enums::PackageModuleType::Package));
xml = exportedProfileXml();
}
QVERIFY2(!xml.isEmpty(), "the export produced nothing to check");
QVERIFY2(!xml.contains(qsl("resurrectAction")), "a save taken during a button script wrote the uninstalled package's button back into the profile");
const QString keeperError = keepersMissingFrom(xml);
QVERIFY2(keeperError.isEmpty(), qPrintable(keeperError));
QVERIFY2(!mpHost->getActionUnit()->findAction(qsl("resurrectAction")), "uninstalled button is still registered");
}
// The event-handler route: Host::raiseEvent() holds ScriptUnit's depth.
void test_saveDuringEventDispatchDoesNotResurrectItsPackage()
{
const QString packageName = qsl("resurrect-script");
mpHost->mInstalledPackages << packageName;
auto pScript = new TScript(nullptr, mpHost);
mpHost->getScriptUnit()->registerScript(pScript);
pScript->mPackageName = packageName;
pScript->setName(qsl("resurrectScript"));
QVERIFY2(pScript->setScript(qsl("local noop = true\n")), "script failed to compile");
QVERIFY2(exportedProfileXml().contains(qsl("resurrectScript")), "the script should be in the profile before its package is uninstalled");
QString xml;
{
mpHost->getScriptUnit()->beginProcessing();
const auto depthGuard = qScopeGuard([this]() {
mpHost->getScriptUnit()->endProcessing();
mpHost->getScriptUnit()->doCleanup();
});
QVERIFY(mpHost->uninstallPackage(packageName, enums::PackageModuleType::Package));
xml = exportedProfileXml();
}
QVERIFY2(!xml.isEmpty(), "the export produced nothing to check");
QVERIFY2(!xml.contains(qsl("resurrectScript")), "a save taken during an event dispatch wrote the uninstalled package's script back into the profile");
const QString keeperError = keepersMissingFrom(xml);
QVERIFY2(keeperError.isEmpty(), qPrintable(keeperError));
QVERIFY2(mpHost->getScriptUnit()->findItems(qsl("resurrectScript")).empty(), "uninstalled script is still registered");
}
// Host::reloadModule() - reachable from Lua - uninstalls and reinstalls a
// module back to back, so from a script the old items are still registered
// when the new ones arrive.
void test_moduleSaveDuringReloadDoesNotDuplicateItsItems()
{
const QString moduleName = qsl("resurrect-module");
registerModuleAs(moduleName);
QVERIFY2(importModuleTimerNamed(moduleName, qsl("moduleTimerBeforeReload")), "could not import the module's timer");
QVERIFY2(exportedModuleXml(moduleName).contains(qsl("moduleTimerBeforeReload")), "the module's timer should be in its file before the reload");
QString xml;
{
mpHost->getTimerUnit()->beginProcessing();
const auto depthGuard = qScopeGuard([this]() {
mpHost->getTimerUnit()->endProcessing();
mpHost->getTimerUnit()->doCleanup();
});
// the uninstall half: at depth the old timer only gets deactivated
QVERIFY(mpHost->uninstallPackage(moduleName, enums::PackageModuleType::ModuleSync));
// ... and the reinstall half brings the module back with fresh items
registerModuleAs(moduleName);
QVERIFY2(importModuleTimerNamed(moduleName, qsl("moduleTimerAfterReload")), "could not re-import the module's timer");
xml = exportedModuleXml(moduleName);
}
QVERIFY2(!xml.isEmpty(), "the module export produced nothing to check");
QVERIFY2(xml.contains(qsl("moduleTimerAfterReload")), "the reloaded module's timer must be written to its file");
QVERIFY2(!xml.contains(qsl("moduleTimerBeforeReload")), "a module save taken mid-reload wrote the pre-reload copy of the timer back into the module file");
}
private:
// Items of a package that is never uninstalled. Every other assertion here
// is an absence, so without these an over-broad filter passes the whole file
// while emptying the user's profile.
void createKeeperItems()
{
const QString keeperPackage = qsl("keeper-package");
mpHost->mInstalledPackages << keeperPackage;
auto pTrigger = new TTrigger(nullptr, mpHost);
pTrigger->setRegexCodeList({qsl("^never matched$")}, {REGEX_PERL});
pTrigger->registerTrigger();
pTrigger->setName(qsl("keeperTrigger"));
pTrigger->mPackageName = keeperPackage;
auto pAlias = new TAlias(qsl("keeperAlias"), mpHost);
pAlias->setRegexCode(qsl("^never matched$"));
mpHost->getAliasUnit()->registerAlias(pAlias);
pAlias->mPackageName = keeperPackage;
auto pTimer = new TTimer(qsl("keeperTimer"), QTime(0, 0, 30), mpHost);
mpHost->getTimerUnit()->registerTimer(pTimer);
pTimer->mPackageName = keeperPackage;
auto pAction = new TAction(qsl("keeperAction"), mpHost);
mpHost->getActionUnit()->registerAction(pAction);
pAction->mPackageName = keeperPackage;
auto pScript = new TScript(nullptr, mpHost);
mpHost->getScriptUnit()->registerScript(pScript);
pScript->setName(qsl("keeperScript"));
pScript->mPackageName = keeperPackage;
}
QString keepersMissingFrom(const QString& xml) const
{
for (const auto& name : {qsl("keeperTrigger"), qsl("keeperAlias"), qsl("keeperTimer"), qsl("keeperAction"), qsl("keeperScript")}) {
if (!xml.contains(name)) {
return qsl("a save taken while a delete was outstanding dropped \"%1\", which belongs to a package that is still installed").arg(name);
}
}
return {};
}
void registerModuleAs(const QString& moduleName)
{
mpHost->mInstalledModules[moduleName] = QStringList{qsl("%1/%2.xml").arg(mConfigDir.path(), moduleName), qsl("0")};
mpHost->mModulesLoadedOk << moduleName;
}
// The module-member flag is private to XMLimport, so a genuine module item
// can only be made by importing one: that creates the module's master folder
// per unit, and renaming the timer's tells the two copies apart.
bool importModuleTimerNamed(const QString& moduleName, const QString& itemName)
{
const QString path = qsl("%1/%2-import.xml").arg(mConfigDir.path(), itemName);
auto* pSeed = new TTimer(itemName, QTime(0, 0, 30), mpHost);
mpHost->getTimerUnit()->registerTimer(pSeed);
pSeed->setScript(qsl("local noop = true\n"));
const bool exported = XMLexport(pSeed).exportTimer(path);
mpHost->getTimerUnit()->unregisterTimer(pSeed);
delete pSeed;
if (!exported) {
return false;
}
QFile file(path);
if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
return false;
}
XMLimport importer(mpHost);
const bool imported = importer.importPackage(&file, moduleName, 1).first;
file.close();
QFile::remove(path);
if (!imported) {
return false;
}
TTimer* pTimer = mpHost->getTimerUnit()->findFirstTimer(moduleName);
if (!pTimer) {
return false;
}
pTimer->setName(itemName);
return true;
}
// Builds the document writeModuleXML() produces for a save and reads it back.
QString exportedModuleXml(const QString& moduleName)
{
const QString path = qsl("%1/module-export.xml").arg(mConfigDir.path());
XMLexport writer(mpHost);
writer.writeModuleXML(moduleName);
if (!XMLexport::saveXmlDocToFile(path, *writer.cloneExportDocument())) {
return {};
}
return readBack(path);
}
// The writers a profile save uses, without the console Host::saveProfile()
// would need.
QString exportedProfileXml()
{
const QString path = qsl("%1/profile-export.xml").arg(mConfigDir.path());
auto writer = std::make_shared<XMLexport>(mpHost);
if (!writer->exportPackage(path, true, true)) {
return {};
}
return readBack(path);
}
static QString readBack(const QString& path)
{
QFile file(path);
if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
return {};
}
const QString xml = QString::fromUtf8(file.readAll());
file.close();
QFile::remove(path);
return xml;
}
};
void initializeQRCResourcesForPackageSelfUninstallTest()

View file

@ -40,8 +40,11 @@
#include "XMLexport.h"
#include "ctelnet.h"
#include "dlgConnectionProfiles.h"
#include "dlgTriggerEditor.h"
#include "mudlet.h"
#include <QTreeWidget>
extern "C" {
#if defined(INCLUDE_VERSIONED_LUA_HEADERS)
#include <lua5.1/lauxlib.h>
@ -68,6 +71,7 @@ class XMLexportVariablesTest : public QObject
private:
TelnetServerStub* mpServer = nullptr;
Host* mpHost = nullptr;
dlgTriggerEditor* mpEditor = nullptr;
const QString mHostname = "XMLexportVars-Test";
const QString mLocalhost = "localhost";
@ -95,6 +99,7 @@ private slots:
void cleanupTestCase()
{
mpEditor = nullptr;
mpHost = nullptr;
delete mpServer;
mpServer = nullptr;
@ -107,6 +112,10 @@ private slots:
// must still be written out - the save path has to refresh the tree.
void test_lateCreatedSavedVariableIsExported()
{
// QTest runs slots in declaration order and these stand for a profile
// whose Variables view was never opened. Profile load builds the editor
// dialog itself, so what matters is that no slot has shown it yet.
QVERIFY2(!mpEditor, "a Variables-view test was declared before the ones that must run without it");
LuaInterface* lI = mpHost->getLuaInterface();
VarUnit* vu = lI->getVarUnit();
// build the tree directly, standing in for the initial build that
@ -387,7 +396,161 @@ private slots:
vu->removeHidden(qsl("userHiddenPrefVar"));
}
// VarUnit has two hidden sets: hiddenByUser, and hidden, which
// Host::hideMudletsVariables() fills with Mudlet's own Lua API. Both have to
// reach the export's tree or a saved table drags the internals into the XML.
void test_internallyHiddenMemberOfSavedTableIsNotExported()
{
LuaInterface* lI = mpHost->getLuaInterface();
VarUnit* vu = lI->getVarUnit();
lua_State* L = mpHost->mLuaInterpreter.getLuaGlobalState();
QCOMPARE(luaL_dostring(L, "internalHiddenTable = {plainMember = 'plain member value', internalMember = 'internal member value'}"), 0);
vu->savedVars.insert(qsl("internalHiddenTable"));
// what addHidden(TVar*, 0) records - the non-user half of the pair
vu->hidden.insert(qsl("internalHiddenTable.internalMember"));
const QString xml = exportProfileXml();
QVERIFY(!xml.isEmpty());
QVERIFY2(xml.contains(qsl("plain member value")), "a plain member of a saved table must be exported");
QVERIFY2(!xml.contains(qsl("internal member value")), "a member hidden by Mudlet itself must not ride along with its saved table");
vu->savedVars.remove(qsl("internalHiddenTable"));
vu->hidden.remove(qsl("internalHiddenTable.internalMember"));
QCOMPARE(luaL_dostring(L, "internalHiddenTable = nil"), 0);
}
// A variable tree takes a Lua registry reference per reference-keyed entry.
// The export throws its tree away, so if the references went with it the
// registry would grow by that many slots on every save.
void test_exportDoesNotLeakLuaRegistryReferences()
{
lua_State* L = mpHost->mLuaInterpreter.getLuaGlobalState();
// several reference-keyed members, so a leak grows the registry visibly
QCOMPARE(luaL_dostring(L, "refKeyLeakTable = {} for i = 1, 20 do refKeyLeakTable[{}] = i end"), 0);
// freed slots go on a free list and come straight back out, so the
// number stops climbing once the registry fits one pass's worth.
// Measuring after the first export leaves that one-off growth out.
QVERIFY(!exportProfileXml().isEmpty());
lua_pushboolean(L, 1);
const int refAfterOne = luaL_ref(L, LUA_REGISTRYINDEX);
luaL_unref(L, LUA_REGISTRYINDEX, refAfterOne);
for (int i = 0; i < 5; ++i) {
QVERIFY(!exportProfileXml().isEmpty());
}
lua_pushboolean(L, 1);
const int refAfterSix = luaL_ref(L, LUA_REGISTRYINDEX);
luaL_unref(L, LUA_REGISTRYINDEX, refAfterSix);
// five more exports keeping 20 references each would put this 100 higher
QVERIFY2(refAfterSix < refAfterOne + 20,
qPrintable(qsl("the exports pinned Lua registry slots: a reference taken after one export was %1, one taken after six was %2").arg(refAfterOne).arg(refAfterSix)));
QCOMPARE(luaL_dostring(L, "refKeyLeakTable = nil"), 0);
}
// A script adds to a saved table while the editor sits on the Variables
// view. A session's last save is taken with whatever view was left on
// screen, so quitting from there is enough to reach this.
void test_savedTableMemberIsExportedWithVariablesViewOpen()
{
QVERIFY2(showEditorOnVariablesView(), "the script editor could not be opened on the Variables view");
LuaInterface* lI = mpHost->getLuaInterface();
VarUnit* vu = lI->getVarUnit();
lua_State* L = mpHost->mLuaInterpreter.getLuaGlobalState();
QCOMPARE(luaL_dostring(L, "varsViewTable = {seedMember = 'seed member value'}"), 0);
vu->savedVars.insert(qsl("varsViewTable"));
vu->savedVars.insert(qsl("varsViewTable.seedMember"));
mpEditor->repopulateVars();
// a script running afterwards, with the view still up
QCOMPARE(luaL_dostring(L, "varsViewTable.lateMember = 'late member value'"), 0);
QCOMPARE(luaL_dostring(L, "varsViewTable.seedMember = nil"), 0);
const QString xml = exportProfileXml();
QVERIFY(!xml.isEmpty());
QVERIFY2(xml.contains(qsl("late member value")), "a member added while the Variables view was open must still be saved");
// secondary: writeVariable() re-reads values from Lua, so a stale tree
// writes this one out empty rather than with its old value
QVERIFY2(!xml.contains(qsl("seed member value")), "a member a script removed while the Variables view was open must not be saved back");
auto* pVariablesTree = mpEditor->findChild<QTreeWidget*>(qsl("treeWidget_variables"));
QVERIFY2(pVariablesTree, "the editor has no variables tree widget");
QTreeWidgetItem* pBaseItem = pVariablesTree->topLevelItem(0);
QVERIFY2(pBaseItem && pBaseItem->childCount() > 0, "the Variables view did not populate");
QVERIFY2(vu->getWVar(pBaseItem->child(0)), "a save taken with the Variables view on screen must leave its items resolving to their variables");
vu->savedVars.remove(qsl("varsViewTable"));
vu->savedVars.remove(qsl("varsViewTable.seedMember"));
QCOMPARE(luaL_dostring(L, "varsViewTable = nil"), 0);
}
// ... and the same for a whole variable rather than a table member.
void test_lateSavedVariableIsExportedWithVariablesViewOpen()
{
QVERIFY2(showEditorOnVariablesView(), "the script editor could not be opened on the Variables view");
LuaInterface* lI = mpHost->getLuaInterface();
VarUnit* vu = lI->getVarUnit();
mpEditor->repopulateVars();
lua_State* L = mpHost->mLuaInterpreter.getLuaGlobalState();
QCOMPARE(luaL_dostring(L, "varsViewLateVar = 'late variable value'"), 0);
vu->savedVars.insert(qsl("varsViewLateVar"));
const QString xml = exportProfileXml();
QVERIFY(!xml.isEmpty());
QVERIFY2(xml.contains(qsl("late variable value")), "a saved variable created while the Variables view was open must still be saved");
vu->savedVars.remove(qsl("varsViewLateVar"));
QCOMPARE(luaL_dostring(L, "varsViewLateVar = nil"), 0);
}
// The other side: a save must not pull the tree out from under the editor.
// Its tree widget and search results resolve items through VarUnit's
// item -> TVar map, which rebuilding the shared tree empties.
void test_variablesEditorItemMappingSurvivesExport()
{
QVERIFY2(showEditorOnVariablesView(), "the script editor could not be opened on the Variables view");
mpEditor->repopulateVars();
VarUnit* vu = mpHost->getLuaInterface()->getVarUnit();
auto* pVariablesTree = mpEditor->findChild<QTreeWidget*>(qsl("treeWidget_variables"));
QVERIFY2(pVariablesTree, "the editor has no variables tree widget");
QTreeWidgetItem* pBaseItem = pVariablesTree->topLevelItem(0);
QVERIFY2(pBaseItem && pBaseItem->childCount() > 0, "the Variables view did not populate");
QTreeWidgetItem* pVariableItem = pBaseItem->child(0);
TVar* pMappedBefore = vu->getWVar(pVariableItem);
QVERIFY2(pMappedBefore, "the Variables view's items should resolve to a variable");
// any save does it: the Save Profile button, the autosave, a package change
mpEditor->slot_showTriggers();
QVERIFY(!exportProfileXml().isEmpty());
QVERIFY2(vu->getWVar(pVariableItem) == pMappedBefore, "a profile save must leave the Variables editor's items resolving to their variables");
}
private:
// Returns false rather than asserting: a QVERIFY here would only return from
// this helper, leaving the caller to dereference a null editor.
bool showEditorOnVariablesView()
{
if (!mpEditor) {
mudlet::self()->slot_showScriptDialog();
QTest::qWait(100);
mpEditor = mpHost->mpEditorDialog;
if (!mpEditor) {
return false;
}
}
mpEditor->slot_showVariables();
QTest::qWait(50);
return true;
}
QString exportProfileXml()
{
const QString xmlPath = mudlet::getMudletPath(enums::profileHomePath, mHostname) + qsl("/xmlexport-test.xml");