quickshell-git: drop patches, bump to 0.3.0.r20.g28771c7
Both fixes we carried are upstream now, in the only two commits made since our pin: 43d4fa9 strips the crash relaunch environment and closes the info fd, and 28771c7 makes IpcHandler deregister through the registry it registered with rather than re-resolving its engine generation. Upstream reaches the IPC fix differently - IpcHandlerRegistry became a QObject held in a QPointer, where we kept a raw back-pointer cleared from ~IpcHandlerRegistry - but it covers the same two cases: a handler outliving its QML context, and a registry destroyed before its handlers. Built clean without the patches at 0.3.0.r20.g28771c7-1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
David Heinemeier Hansson
co-authored by
Claude Opus 5
Claude Opus 5
parent
ae07234a01
commit
0433b2ac30
@@ -1,81 +0,0 @@
|
|||||||
From c4b9162e4f288d8d17c6d0a2cb5f05d9c4a5c4e5 Mon Sep 17 00:00:00 2001
|
|
||||||
From: David Heinemeier Hansson <david@hey.com>
|
|
||||||
Date: Fri, 24 Jul 2026 07:36:46 -0700
|
|
||||||
Subject: [PATCH] launch: clear crash relaunch env after consuming it
|
|
||||||
|
|
||||||
The crash handler re-execs the crashed process with __QUICKSHELL_CRASH_INFO_FD,
|
|
||||||
__QUICKSHELL_CRASH_DUMP_PID and __QUICKSHELL_CRASH_SIGNAL in its environment,
|
|
||||||
with CLOEXEC stripped from the info fd so it survives the exec.
|
|
||||||
|
|
||||||
The relaunched shell kept all of that for its lifetime: the variables stayed
|
|
||||||
in its environment and the info fd stayed open until shutdown. Every process
|
|
||||||
it spawned inherited them, so any quickshell invocation from such a process
|
|
||||||
(e.g. `qs ipc` from a script launched by the shell) mistook itself for a
|
|
||||||
crash relaunch and booted the crashed instance's config instead of running
|
|
||||||
its own command, registering a duplicate instance of the shell.
|
|
||||||
|
|
||||||
Consume the relaunch info, then close the fd and unset the variables before
|
|
||||||
launching.
|
|
||||||
|
|
||||||
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
||||||
---
|
|
||||||
src/launch/main.cpp | 33 +++++++++++++++++++++++----------
|
|
||||||
1 file changed, 23 insertions(+), 10 deletions(-)
|
|
||||||
|
|
||||||
diff --git a/src/launch/main.cpp b/src/launch/main.cpp
|
|
||||||
index efd6628..07d5e71 100644
|
|
||||||
--- a/src/launch/main.cpp
|
|
||||||
+++ b/src/launch/main.cpp
|
|
||||||
@@ -30,17 +30,31 @@ void checkCrashRelaunch(char** argv, QCoreApplication* coreApplication) {
|
|
||||||
|
|
||||||
if (!lastInfoFdStr.isEmpty()) {
|
|
||||||
auto lastInfoFd = lastInfoFdStr.toInt();
|
|
||||||
+ auto crashPid = qEnvironmentVariable("__QUICKSHELL_CRASH_DUMP_PID").toInt();
|
|
||||||
|
|
||||||
- QFile file;
|
|
||||||
- if (!file.open(lastInfoFd, QFile::ReadOnly, QFile::AutoCloseHandle)) {
|
|
||||||
- qFatal() << "Failed to open crash info fd. Cannot restart.";
|
|
||||||
+ RelaunchInfo info;
|
|
||||||
+
|
|
||||||
+ {
|
|
||||||
+ QFile file;
|
|
||||||
+ if (!file.open(lastInfoFd, QFile::ReadOnly, QFile::AutoCloseHandle)) {
|
|
||||||
+ qFatal() << "Failed to open crash info fd. Cannot restart.";
|
|
||||||
+ }
|
|
||||||
+
|
|
||||||
+ file.seek(0);
|
|
||||||
+
|
|
||||||
+ auto ds = QDataStream(&file);
|
|
||||||
+ ds >> info;
|
|
||||||
}
|
|
||||||
|
|
||||||
- file.seek(0);
|
|
||||||
-
|
|
||||||
- auto ds = QDataStream(&file);
|
|
||||||
- RelaunchInfo info;
|
|
||||||
- ds >> info;
|
|
||||||
+ // The crash handler stripped CLOEXEC from the info fd and injected the crash
|
|
||||||
+ // variables into the environment so they survive the re-exec. Both are consumed
|
|
||||||
+ // at this point and must not leak further: children of the relaunched shell
|
|
||||||
+ // inherit its environment, so another quickshell invocation from one of them
|
|
||||||
+ // (e.g. `qs ipc` from a spawned script) would relaunch the crashed config
|
|
||||||
+ // instead of running its own command.
|
|
||||||
+ qunsetenv("__QUICKSHELL_CRASH_INFO_FD");
|
|
||||||
+ qunsetenv("__QUICKSHELL_CRASH_DUMP_PID");
|
|
||||||
+ qunsetenv("__QUICKSHELL_CRASH_SIGNAL");
|
|
||||||
|
|
||||||
LogManager::init(
|
|
||||||
!info.noColor,
|
|
||||||
@@ -50,8 +64,7 @@ void checkCrashRelaunch(char** argv, QCoreApplication* coreApplication) {
|
|
||||||
info.logRules
|
|
||||||
);
|
|
||||||
|
|
||||||
- qCritical().nospace() << "Quickshell has crashed under pid "
|
|
||||||
- << qEnvironmentVariable("__QUICKSHELL_CRASH_DUMP_PID").toInt()
|
|
||||||
+ qCritical().nospace() << "Quickshell has crashed under pid " << crashPid
|
|
||||||
<< " (Coredumps will be available under that pid.)";
|
|
||||||
|
|
||||||
qCritical() << "Further crash information is stored under"
|
|
||||||
--
|
|
||||||
2.55.0
|
|
||||||
|
|
||||||
@@ -1,220 +0,0 @@
|
|||||||
From 8a011617f1c899c48693135103031e97b493cd16 Mon Sep 17 00:00:00 2001
|
|
||||||
From: David Heinemeier Hansson <david@hey.com>
|
|
||||||
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) <noreply@anthropic.com>
|
|
||||||
---
|
|
||||||
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<QString, IpcFunction> functionMap;
|
|
||||||
QHash<QString, IpcProperty> 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
|
|
||||||
|
|
||||||
@@ -2,8 +2,8 @@
|
|||||||
|
|
||||||
_pkgname=quickshell
|
_pkgname=quickshell
|
||||||
pkgname="$_pkgname-git"
|
pkgname="$_pkgname-git"
|
||||||
pkgver=0.3.0.r18.g10b439f
|
pkgver=0.3.0.r20.g28771c7
|
||||||
pkgrel=3
|
pkgrel=1
|
||||||
pkgdesc='Flexible toolkit for making desktop shells with QtQuick'
|
pkgdesc='Flexible toolkit for making desktop shells with QtQuick'
|
||||||
arch=(x86_64 aarch64)
|
arch=(x86_64 aarch64)
|
||||||
url='https://git.outfoxxed.me/quickshell/quickshell'
|
url='https://git.outfoxxed.me/quickshell/quickshell'
|
||||||
@@ -37,31 +37,10 @@ provides=("$_pkgname")
|
|||||||
conflicts=("$_pkgname")
|
conflicts=("$_pkgname")
|
||||||
|
|
||||||
_pkgsrc="$_pkgname"
|
_pkgsrc="$_pkgname"
|
||||||
source=("$_pkgsrc"::"git+$url.git#commit=10b439fc6e3fd65c15fe1c486271b31da05ed023"
|
source=("$_pkgsrc"::"git+$url.git#commit=28771c7c74b42e20afca0b1b63980cb46515537c"
|
||||||
quickshell-check.hook
|
quickshell-check.hook)
|
||||||
0001-launch-clear-crash-relaunch-env.patch
|
|
||||||
0002-ipchandler-deregister-through-registry.patch)
|
|
||||||
sha256sums=('SKIP'
|
sha256sums=('SKIP'
|
||||||
'8543e21aeaaa5441b73a679160e7601a957f16c433e8d6bd9257e80bd0e94083'
|
'8543e21aeaaa5441b73a679160e7601a957f16c433e8d6bd9257e80bd0e94083')
|
||||||
'867f154dd3ea09ec751664e84fe9da47f6e0321b77abaa0cf3eb391a978de305'
|
|
||||||
'ba22289eba7ccd64672df003d0f8e3130a1e6799a5b75686b489fe39794e9725')
|
|
||||||
|
|
||||||
|
|
||||||
prepare() {
|
|
||||||
cd "$_pkgsrc"
|
|
||||||
# After a crash relaunch, quickshell leaks __QUICKSHELL_CRASH_* into the
|
|
||||||
# environment of every child it spawns, so any `qs` call from those children
|
|
||||||
# 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() {
|
pkgver() {
|
||||||
cd "$_pkgsrc"
|
cd "$_pkgsrc"
|
||||||
|
|||||||
Reference in New Issue
Block a user