diff --git a/pkgbuilds/quickshell-git/0002-ipchandler-deregister-through-registry.patch b/pkgbuilds/quickshell-git/0002-ipchandler-deregister-through-registry.patch new file mode 100644 index 0000000..54875d0 --- /dev/null +++ b/pkgbuilds/quickshell-git/0002-ipchandler-deregister-through-registry.patch @@ -0,0 +1,220 @@ +From 8a011617f1c899c48693135103031e97b493cd16 Mon Sep 17 00:00:00 2001 +From: David Heinemeier Hansson +Date: Fri, 24 Jul 2026 14:00:37 -0700 +Subject: [PATCH] io/ipchandler: deregister handlers through the registry they + registered with + +An IpcHandler destroyed as part of a larger teardown - a Loader subtree +being replaced, a plugin swapped out, the root object itself - can leave its +registration behind. The registry then holds a pointer to a freed handler, +and the next `qs ipc call` on that target takes the process down. + +~IpcHandler asks updateRegistration() to remove the registration, and +updateRegistration() re-resolves the registry by way of +EngineGeneration::findObjectGeneration(). That walk needs a QML context, and +a context is invalidated before the objects created in it are destroyed, so +for a handler going down with its context the lookup returns null and the +function returns before removing anything. The code already anticipates the +lookup failing during destruction - that is why the "Unable to identify +engine generation" warning is suppressed when `destroying` is set - but the +early return also skips the removal the destructor asked for. + +Nothing complains at that point. The dead handler keeps the map entry, so +the next handler registering for the same target is refused with "Handler +was registered but will not be used because another handler is registered +for target ...", and the crash lands later, on whichever caller reaches that +target first: + + #4 IpcHandler::findFunction (this=0x7f605cab7140) at src/io/ipchandler.cpp:441 + #5 StringCallCommand::exec (conn=...) at src/io/ipccomm.cpp:146 + #12 IpcServerConnection::onReadyRead () at src/ipc/ipc.cpp:76 + +`this` there reads back as unrelated UTF-16 string data - the allocation had +been reused - and quickshell.ipchandler logs show that handler registered +once, during a reload three hours earlier, and never deregistered. Seen on +0.3.0 (10b439f) with Qt 6.11.1, on a config whose bar rebuilds its widgets +when plugins reload. + +Remember the registry a handler registered with and remove through that, so +removal no longer depends on the generation still being reachable. +Registration continues to resolve the generation, since a handler that has +never registered has no registry to remember. + +The registry can also be destroyed before its handlers: destroy() deletes a +generation's extensions before it destroys the root object, so +~IpcHandlerRegistry clears the back-pointer of every handler it still knows +about, and a handler with no registry simply drops its registration state. + +destroy() also left the deleted extension pointers in `extensions`, where +findExtension() would hand one back to any object reaching for an extension +on its way down. Clear the hash, and refuse registration once the generation +is being destroyed. + +Verified on a shell with 20 IPC targets across ~24 handlers: repeated plugin +reloads now produce one deregistration per destroyed handler (105 +registrations, 81 deregistrations, the difference being exactly the live +handlers), where before some reloads left handlers registered forever. +Shutdown, config reload, and subtree teardown all leave the registry +consistent, and a handler that disables itself from Component.onDestruction +- which crashed on the freed registry before - no longer does. + +Co-Authored-By: Claude Opus 5 (1M context) +--- + src/core/generation.cpp | 6 ++++ + src/core/generation.hpp | 4 +++ + src/io/ipchandler.cpp | 61 +++++++++++++++++++++++++++++++---------- + src/io/ipchandler.hpp | 9 ++++++ + 4 files changed, 66 insertions(+), 14 deletions(-) + +diff --git a/src/core/generation.cpp b/src/core/generation.cpp +index 21febc3..9a3fc13 100644 +--- a/src/core/generation.cpp ++++ b/src/core/generation.cpp +@@ -82,6 +82,12 @@ void EngineGeneration::destroy() { + delete extension; + } + ++ // The root object below is destroyed after this point, and objects can still ++ // reach for an extension while going down - a binding re-evaluating during ++ // destruction, for instance. Drop the deleted pointers so findExtension() ++ // cannot hand one back out. ++ this->extensions.clear(); ++ + if (this->root != nullptr) { + QObject::connect(this->root, &QObject::destroyed, this, [this]() { + // prevent further js execution between garbage collection and engine destruction. +diff --git a/src/core/generation.hpp b/src/core/generation.hpp +index 4543408..2723c24 100644 +--- a/src/core/generation.hpp ++++ b/src/core/generation.hpp +@@ -72,6 +72,10 @@ public: + void destroy(); + void shutdown(); + ++ // True once destroy() has begun. Extensions are deleted at that point while ++ // the object tree is still alive, so objects going down must not ask for one. ++ [[nodiscard]] bool isDestroying() const { return this->destroying; } ++ + signals: + void filesChanged(); + void reloadFinished(); +diff --git a/src/io/ipchandler.cpp b/src/io/ipchandler.cpp +index d4429fd..68fa368 100644 +--- a/src/io/ipchandler.cpp ++++ b/src/io/ipchandler.cpp +@@ -304,24 +304,43 @@ void IpcHandler::onSignalTriggered(const QString& signal, const QString& value) + void IpcHandler::updateRegistration(bool destroying) { + if (!this->complete) return; + +- auto* generation = EngineGeneration::findObjectGeneration(this); +- +- if (!generation) { +- if (!destroying) { +- qmlWarning(this) << "Unable to identify engine generation, cannot register."; +- } +- +- return; +- } +- +- auto* registry = IpcHandlerRegistry::forGeneration(generation); +- + if (this->registeredState.enabled) { +- registry->deregisterHandler(this); +- qCDebug(logIpcHandler) << "Deregistered" << this << "from registry" << registry; ++ // Removal must not depend on findObjectGeneration(): a QML context is ++ // invalidated before the objects created in it are destroyed, so a handler ++ // that goes down as part of a larger teardown can no longer reach its ++ // generation, and returning early there would leave the registry holding a ++ // pointer to this handler after it is freed. The registry it registered ++ // with is still reachable, so remove through that. ++ if (this->registry) { ++ auto* registry = this->registry; ++ registry->deregisterHandler(this); ++ qCDebug(logIpcHandler) << "Deregistered" << this << "from registry" << registry; ++ } else { ++ // The registry was destroyed first and dropped this handler on its way ++ // out, so there is nothing left to remove the registration from. ++ this->registeredState = RegistrationState(false); ++ } + } + + if (this->targetState.enabled && !this->targetState.target.isEmpty()) { ++ // Registration still resolves the generation: a handler that has never ++ // registered has no registry to remember. ++ auto* generation = EngineGeneration::findObjectGeneration(this); ++ ++ if (!generation) { ++ if (!destroying) { ++ qmlWarning(this) << "Unable to identify engine generation, cannot register."; ++ } ++ ++ return; ++ } ++ ++ // Extensions, the registry among them, are deleted at the start of ++ // EngineGeneration::destroy(), while the object tree is still alive. ++ // Nothing may register after that point. ++ if (generation->isDestroying()) return; ++ ++ auto* registry = IpcHandlerRegistry::forGeneration(generation); + registry->registerHandler(this); + qCDebug(logIpcHandler) << "Registered" << this << "to registry" << registry; + } +@@ -362,6 +381,7 @@ void IpcHandlerRegistry::registerHandler(IpcHandler* handler) { + + handler->registeredState = handler->targetState; + handler->registeredState.enabled = true; ++ handler->registry = this; + } + + void IpcHandlerRegistry::deregisterHandler(IpcHandler* handler) { +@@ -377,6 +397,19 @@ void IpcHandlerRegistry::deregisterHandler(IpcHandler* handler) { + } + + handler->registeredState = IpcHandler::RegistrationState(false); ++ handler->registry = nullptr; ++} ++ ++IpcHandlerRegistry::~IpcHandlerRegistry() { ++ // Handlers outlive the registry when a generation is torn down: ++ // EngineGeneration::destroy() deletes its extensions before it destroys the ++ // root object. Drop the back-pointers so the handlers destroyed afterwards ++ // don't reach into this registry. ++ for (const auto& targetVec: this->knownHandlers) { ++ for (auto* handler: targetVec) { ++ handler->registry = nullptr; ++ } ++ } + } + + QString IpcHandler::listMembers(qsizetype indent) { +diff --git a/src/io/ipchandler.hpp b/src/io/ipchandler.hpp +index ab41dc4..449228d 100644 +--- a/src/io/ipchandler.hpp ++++ b/src/io/ipchandler.hpp +@@ -266,6 +266,11 @@ private: + RegistrationState registeredState; + RegistrationState targetState {true}; + bool complete = false; ++ // The registry this handler is registered with, or null if it is not ++ // registered or the registry has been destroyed. Deregistration goes through ++ // this rather than re-resolving the engine generation, which a handler ++ // cannot always reach while it is being destroyed. ++ IpcHandlerRegistry* registry = nullptr; + + QHash functionMap; + QHash propertyMap; +@@ -276,6 +281,10 @@ private: + + class IpcHandlerRegistry: public EngineGenerationExt { + public: ++ IpcHandlerRegistry() = default; ++ ~IpcHandlerRegistry() override; ++ Q_DISABLE_COPY_MOVE(IpcHandlerRegistry); ++ + static IpcHandlerRegistry* forGeneration(EngineGeneration* generation); + + void registerHandler(IpcHandler* handler); +-- +2.55.0 + diff --git a/pkgbuilds/quickshell-git/PKGBUILD b/pkgbuilds/quickshell-git/PKGBUILD index 9d373b1..f00ab65 100644 --- a/pkgbuilds/quickshell-git/PKGBUILD +++ b/pkgbuilds/quickshell-git/PKGBUILD @@ -3,7 +3,7 @@ _pkgname=quickshell pkgname="$_pkgname-git" pkgver=0.3.0.r18.g10b439f -pkgrel=2 +pkgrel=3 pkgdesc='Flexible toolkit for making desktop shells with QtQuick' arch=(x86_64 aarch64) url='https://git.outfoxxed.me/quickshell/quickshell' @@ -39,10 +39,12 @@ conflicts=("$_pkgname") _pkgsrc="$_pkgname" source=("$_pkgsrc"::"git+$url.git#commit=10b439fc6e3fd65c15fe1c486271b31da05ed023" quickshell-check.hook - 0001-launch-clear-crash-relaunch-env.patch) + 0001-launch-clear-crash-relaunch-env.patch + 0002-ipchandler-deregister-through-registry.patch) sha256sums=('SKIP' '8543e21aeaaa5441b73a679160e7601a957f16c433e8d6bd9257e80bd0e94083' - '867f154dd3ea09ec751664e84fe9da47f6e0321b77abaa0cf3eb391a978de305') + '867f154dd3ea09ec751664e84fe9da47f6e0321b77abaa0cf3eb391a978de305' + 'ba22289eba7ccd64672df003d0f8e3130a1e6799a5b75686b489fe39794e9725') prepare() { @@ -52,6 +54,13 @@ prepare() { # boots a duplicate shell instead of running its command. # Carried on the fix-crash-env-leak branch of https://github.com/omacom-io/quickshell patch -Np1 -i "$srcdir/0001-launch-clear-crash-relaunch-env.patch" + + # An IpcHandler deregistered itself by re-resolving its registry from its + # engine generation, which is unreachable once its QML context is torn down, + # so a handler destroyed with a reloading bar or plugin left the registry + # pointing at freed memory and the next `qs ipc call` on that target crashed. + # Carried on the fix-ipc-handler-lifetime branch of https://github.com/omacom-io/quickshell + patch -Np1 -i "$srcdir/0002-ipchandler-deregister-through-registry.patch" } pkgver() {