Fix: crash when a package uninstalls itself from its own alias/key/tr… (#9383)

---
Fix: crash when a package uninstalls itself from its own
alias/key/trigger (#9337)

<!-- Keep the title short & concise so anyone non-technical can
understand it,
     the title appears in PTB changelogs -->
Brief overview of PR changes/additions

If a package has an alias, key or trigger whose script uninstalls its
own package (for example calling uninstallPackage() on itself), Mudlet
crashes. This fixes that.

The problem is that uninstall() deletes the matched items right away,
but at that point we're still inside match()/processDataStream() for
that same item, so we end up using memory that was just freed (a
use-after-free).

The fix is to not delete straight away if we're still in the middle of
processing. Instead we deactivate the items so they stop matching for
the rest of the pass, and then actually delete them later in doCleanup()
once processing has finished unwinding. Same change applied to aliases,
keys and triggers since they all share this pattern.

Motivation for adding to Mudlet

It's a hard crash that a user can hit with a completely normal package
setup — a "reload package" style alias/trigger is a pretty common thing
to have.

Other info (issues closed, discussion etc)

Closes #9337.

---

src/AliasUnit.cpp

@@ -75,6 +75,19 @@ void AliasUnit::uninstall(const QString& packageName)
             uninstallList.append(rootAlias);
         }
     }
+    // Re-entrant uninstall (#9337): an alias's own script (e.g. uninstallPackage())
+    // is removing its package while match()/processDataStream() are still on the
+    // stack for that alias. Deleting now would be a use-after-free, so defer to
+    // doCleanup() at depth 0. Deactivating is enough to stop them firing for the
+    // rest of this pass: processDataStream()'s loop skips deactivated items and
+    // match() returns early on !isActive() for those reached via a parent.
+    if (mProcessingDepth > 0) {
+        for (auto alias : uninstallList) {
+            alias->setIsActive(false);
+            mCleanupSet.remove(alias); // keep the two deferred-delete paths disjoint
+        }
+        return;
+    }
     for (auto& alias : uninstallList) {
         delete alias;
     }
@@ -273,6 +286,9 @@ bool AliasUnit::processDataStream(const QString& data)
     mProcessingDepth++;
 
     for (auto alias : copyOfNodeList) {
+        if (!alias->isActive() && !alias->shouldBeActive()) {
+            continue;
+        }
         // = data.replace( "\n", "" );
         if (alias->match(data)) {
             state = true;
@@ -427,6 +443,18 @@ void AliasUnit::doCleanup()
         itAlias.remove();
         delete pAlias;
     }
+    // Flush the deletes uninstall() deferred (#9337). uninstallList is ordered
+    // children-before-parents and each ~Tree unlinks from its parent, so deleting
+    // children first empties the parent's child list (no double free); the seen
+    // set guards a node queued twice by re-entrant uninstalls.
+    QSet<TAlias*> deletedAliases;
+    for (auto alias : uninstallList) {
+        if (!deletedAliases.contains(alias)) {
+            deletedAliases.insert(alias);
+            delete alias;
+        }
+    }
+    uninstallList.clear();
 }
 
 void AliasUnit::markCleanup(TAlias* pT)

src/KeyUnit.cpp

@@ -84,6 +84,19 @@ void KeyUnit::uninstall(const QString& packageName)
             uninstallList.append(rootKey);
         }
     }
+    // Re-entrant uninstall (#9337): a key's own script (e.g. uninstallPackage())
+    // is removing its package while match()/processDataStream() are still on the
+    // stack for that key. Deleting now would be a use-after-free, so defer to
+    // doCleanup() at depth 0. Deactivating is enough to stop them firing for the
+    // rest of this pass: match() returns early on !isActive(), so the processing
+    // loop skips them without needing a loop-level guard.
+    if (mProcessingDepth > 0) {
+        for (auto key : uninstallList) {
+            key->setIsActive(false);
+            mCleanupSet.remove(key); // keep the two deferred-delete paths disjoint
+        }
+        return;
+    }
     for (auto& key : uninstallList) {
         delete key;
     }
@@ -466,6 +479,18 @@ void KeyUnit::doCleanup()
         itKey.remove();
         delete pKey;
     }
+    // Flush the deletes uninstall() deferred (#9337). uninstallList is ordered
+    // children-before-parents and each ~Tree unlinks from its parent, so deleting
+    // children first empties the parent's child list (no double free); the seen
+    // set guards a node queued twice by re-entrant uninstalls.
+    QSet<TKey*> deletedKeys;
+    for (auto key : uninstallList) {
+        if (!deletedKeys.contains(key)) {
+            deletedKeys.insert(key);
+            delete key;
+        }
+    }
+    uninstallList.clear();
 }
 
 void KeyUnit::setupKeyNames()

src/TriggerUnit.cpp

@@ -88,6 +88,20 @@ void TriggerUnit::uninstall(const QString& packageName)
             uninstallList.append(rootTrigger);
         }
     }
+    // Re-entrant uninstall (#9337): a trigger's own script (e.g. uninstallPackage())
+    // is removing its package while match()/processDataStream() are still on the
+    // stack for that trigger. Deleting now would be a use-after-free, so defer to
+    // doCleanup() at depth 0. Deactivating is enough to stop them firing for the
+    // rest of this pass: processDataStream()'s loop skips deactivated triggers
+    // and match() runs its whole body inside if (isActive()) for those reached
+    // via a parent chain or filter.
+    if (mProcessingDepth > 0) {
+        for (auto trigger : uninstallList) {
+            trigger->setIsActive(false);
+            mCleanupSet.remove(trigger); // keep the two deferred-delete paths disjoint
+        }
+        return;
+    }
     for (auto& trigger : uninstallList) {
         delete trigger;
     }
@@ -307,6 +321,9 @@ void TriggerUnit::processDataStream(const QString& data, int line)
     // same hazard for the same reason — see Mudlet issue #4297.
     auto copyOfNodeList = mTriggerRootNodeList;
     for (auto trigger : copyOfNodeList) {
+        if (!trigger->isActive()) {
+            continue;
+        }
         trigger->match(subject, data, line);
     }
     free(subject);
@@ -474,6 +491,18 @@ void TriggerUnit::doCleanup()
         itTrigger.remove();
         delete pTrigger;
     }
+    // Flush the deletes uninstall() deferred (#9337). uninstallList is ordered
+    // children-before-parents and each ~Tree unlinks from its parent, so deleting
+    // children first empties the parent's child list (no double free); the seen
+    // set guards a node queued twice by re-entrant uninstalls.
+    QSet<TTrigger*> deletedTriggers;
+    for (auto trigger : uninstallList) {
+        if (!deletedTriggers.contains(trigger)) {
+            deletedTriggers.insert(trigger);
+            delete trigger;
+        }
+    }
+    uninstallList.clear();
 }
 
 void TriggerUnit::markCleanup(TTrigger* pT)