From e498ced9a3e055eb44964171a2a0b620bb02e6d2 Mon Sep 17 00:00:00 2001 From: ntomsic Date: Wed, 20 May 2026 14:23:38 -0500 Subject: [PATCH] qt: lifetime safety + correctness fixes from PR review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pass over the Qt frontend tightening object lifetime, dropping a few bugs, and consolidating the pieces a senior-engineer review surfaced. UAF risks in the action dispatcher: - MainWindow::onAction queued lambdas captured raw MainWindow* / GhosttySurface* by value. Qt's QueuedConnection cancels a slot if its receiver is gone, but cross-captured pointers (e.g. `src` when posting to `win`, or `win` when posting to `src`) are not protected — a multi-window + tear-off + close race could fire a slot whose body dereferences a deleted object. Every cross-pointer is now wrapped in QPointer; a small `post()` helper queues onto the receiver. Each lambda body re-checks both sides at execution time. - onCloseSurface / onConfirmReadClipboard / onWriteClipboard / onReadClipboard turn a libghostty userdata pointer into a GhosttySurface*. A worker-thread callback can race the surface's destructor; surfaceAlive() now validates against the s_windows registry before any deref. - onWakeup's queued tick still touches s_app inside the lambda; documented why that's the right guard against a last-window-closed race. Object lifetime cleanups: - WindowBlur leaked the org_kde_kwin_blur and dangled its QWindow* hash key when a window was destroyed. Connect to QWindow::destroyed to release the proxy and clear the entry. - InspectorWindow::closeEvent now hides instead of deleting. The owning GhosttySurface holds it as a QPointer and toggles visibility; deleting on WM-close would dangle that pointer and skip libghostty inspector teardown. - GhosttySurface::m_inspectorWindow is now a QPointer; ~Ghostty calls delete on the .data() so an already-destroyed inspector is a no-op. - XkbState (process singleton) gained a destructor and m_query was marked mutable. Documented the single-thread-only constraint — consumedMods mutates m_query. Tab tear-off race: - A namespace-scope `g_tabDropHandled` bool would race two tear-offs in flight in different windows. Replaced with a per-bar m_dropHandled flag, set via a pointer-to-originator carried in the drag's MIME payload. Other correctness: - LayerShellQt::Window::setDesiredSize doesn't exist on the Qt 6 / trixie branch — the layer-shell size comes from QWindow::resize(), which the next line already does. Drop the dead call. - MainWindow had a per-window QTimer firing ghostty_app_tick every 16ms. With N windows that's N redundant ticks for the *same* shared app per frame. One process-wide static timer parented to qApp; frame() is now static and walks every window's surfaces. - onConfirmReadClipboard truncated the warning preview by raw QString::left(200), risking slicing a surrogate pair half. Back off to a non-surrogate boundary first. - onWriteClipboard / OPEN_CONFIG: route via qApp instead of the source surface's owner — the clipboard is process-global; routing via a window that may already be closing was strictly worse. - OverlayScrollbar::setMetrics now repaints while m_opacity > 0 too, so a fading scrollbar tracks live scrollback updates instead of freezing on the last frame. - Unify log prefix to [ghastty] across qt/src/. Was [ghostty] in about half the fprintf calls; the rebrand left the rest stale. - Migrate every callsite to the shared Util.h helpers (translateMods, formatTrigger/triggerKeyName, BellFeature enum, configGet<>). Remove the per-file duplicates. Co-Authored-By: claude-flow --- qt/src/CommandPalette.cpp | 5 +- qt/src/GhosttySurface.cpp | 93 +++---- qt/src/GhosttySurface.h | 6 +- qt/src/GlobalShortcuts.cpp | 4 +- qt/src/InspectorWindow.cpp | 19 +- qt/src/InspectorWindow.h | 9 + qt/src/MainWindow.cpp | 519 ++++++++++++++++++------------------ qt/src/MainWindow.h | 17 +- qt/src/OverlayScrollbar.cpp | 5 +- qt/src/TabWidget.cpp | 54 +++- qt/src/WindowBlur.cpp | 21 +- qt/src/main.cpp | 4 +- 12 files changed, 404 insertions(+), 352 deletions(-) diff --git a/qt/src/CommandPalette.cpp b/qt/src/CommandPalette.cpp index cc8eda612..c8d014df8 100644 --- a/qt/src/CommandPalette.cpp +++ b/qt/src/CommandPalette.cpp @@ -12,6 +12,7 @@ #include "GhosttySurface.h" #include "MainWindow.h" +#include "Util.h" namespace { // Item data roles: the keybind action to run, and the text the filter @@ -86,9 +87,7 @@ void CommandPalette::populate() { // command-palette-entry defaults to a large built-in command set. ghostty_config_command_list_s list = {}; - if (!ghostty_config_get(cfg, &list, "command-palette-entry", - qstrlen("command-palette-entry"))) - return; + if (!configGet(cfg, &list, "command-palette-entry")) return; for (size_t i = 0; i < list.len; ++i) { const ghostty_command_s &c = list.commands[i]; const QString title = QString::fromUtf8(c.title ? c.title : ""); diff --git a/qt/src/GhosttySurface.cpp b/qt/src/GhosttySurface.cpp index 3bcd2f587..6648ac136 100644 --- a/qt/src/GhosttySurface.cpp +++ b/qt/src/GhosttySurface.cpp @@ -5,6 +5,7 @@ #include "OverlayScrollbar.h" #include "SearchBar.h" #include "TabWidget.h" +#include "Util.h" #include #include @@ -73,7 +74,7 @@ GhosttySurface::GhosttySurface(ghostty_app_t app, MainWindow *owner, m_context = new QOpenGLContext(this); m_context->setFormat(QSurfaceFormat::defaultFormat()); if (!m_context->create()) { - std::fprintf(stderr, "[ghostty] GL context creation failed\n"); + std::fprintf(stderr, "[ghastty] GL context creation failed\n"); return; } m_offscreen = new QOffscreenSurface(nullptr, this); @@ -81,7 +82,7 @@ GhosttySurface::GhosttySurface(ghostty_app_t app, MainWindow *owner, m_offscreen->create(); if (!makeCurrent()) { - std::fprintf(stderr, "[ghostty] makeCurrent failed\n"); + std::fprintf(stderr, "[ghastty] makeCurrent failed\n"); return; } @@ -107,7 +108,7 @@ GhosttySurface::GhosttySurface(ghostty_app_t app, MainWindow *owner, m_surface = ghostty_surface_new(m_app, &sc); if (!m_surface) { - std::fprintf(stderr, "[ghostty] ghostty_surface_new failed\n"); + std::fprintf(stderr, "[ghastty] ghostty_surface_new failed\n"); return; } @@ -116,7 +117,8 @@ GhosttySurface::GhosttySurface(ghostty_app_t app, MainWindow *owner, GhosttySurface::~GhosttySurface() { // The inspector window holds m_surface; destroy it before m_surface. - delete m_inspectorWindow; + // QPointer auto-nulls on a destroyed QObject, so .data() is safe. + delete m_inspectorWindow.data(); // Release GL-owning objects with the context current. if (makeCurrent()) { @@ -204,9 +206,7 @@ void GhosttySurface::layoutScrollbar() { bool GhosttySurface::scrollbarAllowed() const { if (!m_owner || !m_owner->config()) return true; const char *value = nullptr; - if (ghostty_config_get(m_owner->config(), &value, "scrollbar", - qstrlen("scrollbar")) && - value) + if (configGet(m_owner->config(), &value, "scrollbar") && value) return qstrcmp(value, "never") != 0; return true; // unknown — default to showing } @@ -233,9 +233,7 @@ void GhosttySurface::flashScrollbar() { if (!m_scrollbar || !scrollbarAllowed()) return; // Handle colour: light on a dark terminal, dark on a light one. ghostty_config_color_s bg{}; - if (m_owner && m_owner->config() && - ghostty_config_get(m_owner->config(), &bg, "background", - qstrlen("background"))) { + if (m_owner && configGet(m_owner->config(), &bg, "background")) { const double luma = 0.299 * bg.r + 0.587 * bg.g + 0.114 * bg.b; m_scrollbar->setHandleColor(luma < 128.0 ? QColor(235, 235, 235) : QColor(45, 45, 45)); @@ -284,14 +282,11 @@ void GhosttySurface::paintEvent(QPaintEvent *) { if (!hasFocus() && qobject_cast(parentWidget())) { ghostty_config_t cfg = m_owner ? m_owner->config() : nullptr; double opacity = 0.7; - if (cfg) - ghostty_config_get(cfg, &opacity, "unfocused-split-opacity", - qstrlen("unfocused-split-opacity")); + configGet(cfg, &opacity, "unfocused-split-opacity"); if (opacity < 1.0) { QColor fill(0, 0, 0); // default: dim toward black ghostty_config_color_s c{}; - if (cfg && ghostty_config_get(cfg, &c, "unfocused-split-fill", - qstrlen("unfocused-split-fill"))) + if (configGet(cfg, &c, "unfocused-split-fill")) fill = QColor(c.r, c.g, c.b); fill.setAlphaF(1.0 - opacity); painter.setCompositionMode(QPainter::CompositionMode_SourceOver); @@ -449,8 +444,7 @@ void GhosttySurface::showResizeOverlay() { m_resizeOverlay->raise(); unsigned long long durNs = 0; - ghostty_config_get(cfg, &durNs, "resize-overlay-duration", - qstrlen("resize-overlay-duration")); + configGet(cfg, &durNs, "resize-overlay-duration"); const int durMs = durNs ? static_cast(durNs / 1000000ULL) : 750; if (!m_resizeHideTimer) { m_resizeHideTimer = new QTimer(this); @@ -569,6 +563,11 @@ void GhosttySurface::premultiplyFramebuffer() { // it Shift+punctuation gets emitted as a kitty CSI the shell can't // decode (Shift+letter happens to work because A-Z survive that // path). +// +// THREAD SAFETY: this is a process singleton accessed only from the Qt +// GUI thread (Qt key events are dispatched there, and so is libghostty's +// inputMethodEvent forwarding). consumedMods mutates m_query, so a +// second thread would race; do not call from worker threads. class XkbState { public: static XkbState &instance() { @@ -588,7 +587,8 @@ public: // Modifiers consumed by the layout to produce `keycode`'s text given // `mods` are depressed. Returns the consumed subset, expressed as - // ghostty mod bits. + // ghostty mod bits. Mutates m_query (mutable) — see thread-safety + // note on the class. ghostty_input_mods_e consumedMods(uint32_t keycode, ghostty_input_mods_e mods) const { if (!m_query) return GHOSTTY_MODS_NONE; @@ -633,32 +633,40 @@ private: m_idxSuper = xkb_keymap_mod_get_index(m_keymap, XKB_MOD_NAME_LOGO); } + ~XkbState() { + // Run on process exit when the static is destroyed. The OS would + // reclaim regardless, but explicit teardown silences leak checkers + // and documents the ownership chain. + if (m_query) xkb_state_unref(m_query); + if (m_unshifted) xkb_state_unref(m_unshifted); + if (m_keymap) xkb_keymap_unref(m_keymap); + if (m_ctx) xkb_context_unref(m_ctx); + } + + XkbState(const XkbState &) = delete; + XkbState &operator=(const XkbState &) = delete; + struct xkb_context *m_ctx = nullptr; struct xkb_keymap *m_keymap = nullptr; struct xkb_state *m_unshifted = nullptr; // permanent no-mods state - struct xkb_state *m_query = nullptr; // reused for consumed-mods queries + // Reused across consumedMods calls (mutated then reset). Mutable so + // consumedMods can stay logically const. + mutable struct xkb_state *m_query = nullptr; xkb_mod_index_t m_idxShift = XKB_MOD_INVALID; xkb_mod_index_t m_idxCtrl = XKB_MOD_INVALID; xkb_mod_index_t m_idxAlt = XKB_MOD_INVALID; xkb_mod_index_t m_idxSuper = XKB_MOD_INVALID; }; -// Translate Qt keyboard modifiers into libghostty's modifier bitfield. -static ghostty_input_mods_e translateMods(Qt::KeyboardModifiers m) { - int r = GHOSTTY_MODS_NONE; - if (m & Qt::ShiftModifier) r |= GHOSTTY_MODS_SHIFT; - if (m & Qt::ControlModifier) r |= GHOSTTY_MODS_CTRL; - if (m & Qt::AltModifier) r |= GHOSTTY_MODS_ALT; - if (m & Qt::MetaModifier) r |= GHOSTTY_MODS_SUPER; - return static_cast(r); -} - void GhosttySurface::sendKey(QKeyEvent *ev, ghostty_input_action_e action) { if (!m_surface) return; // Forward committed text only for printable input; control characters // and special keys (Enter, Tab, arrows, Ctrl+letter, ...) are encoded // by libghostty from the physical keycode + modifiers. + // The QByteArray below is stack-local; ghostty_surface_key is + // synchronous and copies any text it needs internally, so the buffer + // only has to live across this call. const QByteArray text = ev->text().toUtf8(); const bool printable = !text.isEmpty() && @@ -745,40 +753,21 @@ void GhosttySurface::mouseReleaseEvent(QMouseEvent *ev) { } // The keybind bound to `action` in the live config, as a QKeySequence -// for a context-menu hint. Empty if unbound or not displayable. +// for a context-menu hint. Empty if unbound or not displayable +// (CATCH_ALL, an unmapped physical key, etc.). QKeySequence GhosttySurface::shortcutFor(const char *action) const { if (!m_owner || !m_owner->config()) return {}; const ghostty_input_trigger_s t = ghostty_config_trigger(m_owner->config(), action, qstrlen(action)); - QString key; - switch (t.tag) { - case GHOSTTY_TRIGGER_UNICODE: - if (t.key.unicode) key = QString(QChar(t.key.unicode)).toUpper(); - break; - case GHOSTTY_TRIGGER_PHYSICAL: { - const ghostty_input_key_e k = t.key.physical; - if (k >= GHOSTTY_KEY_A && k <= GHOSTTY_KEY_Z) - key = QChar('A' + (k - GHOSTTY_KEY_A)); - else if (k >= GHOSTTY_KEY_DIGIT_0 && k <= GHOSTTY_KEY_DIGIT_9) - key = QChar('0' + (k - GHOSTTY_KEY_DIGIT_0)); - else if (k == GHOSTTY_KEY_ENTER) - key = QStringLiteral("Return"); - else if (k == GHOSTTY_KEY_SPACE) - key = QStringLiteral("Space"); - else if (k == GHOSTTY_KEY_TAB) - key = QStringLiteral("Tab"); - break; - } - default: - break; // CATCH_ALL etc. — nothing displayable - } + const QString key = triggerKeyName(t); if (key.isEmpty()) return {}; QString seq; if (t.mods & GHOSTTY_MODS_CTRL) seq += QStringLiteral("Ctrl+"); if (t.mods & GHOSTTY_MODS_ALT) seq += QStringLiteral("Alt+"); if (t.mods & GHOSTTY_MODS_SHIFT) seq += QStringLiteral("Shift+"); + // QKeySequence parses Meta+ as the Super/Logo key on Linux. if (t.mods & GHOSTTY_MODS_SUPER) seq += QStringLiteral("Meta+"); return QKeySequence(seq + key); } diff --git a/qt/src/GhosttySurface.h b/qt/src/GhosttySurface.h index 9b3d0c879..6e1cf9506 100644 --- a/qt/src/GhosttySurface.h +++ b/qt/src/GhosttySurface.h @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -187,7 +188,10 @@ private: int m_lastCols = 0; // last grid size, to detect changes int m_lastRows = 0; SearchBar *m_searchBar = nullptr; // in-terminal search; lazily made - InspectorWindow *m_inspectorWindow = nullptr; // terminal inspector; lazily made + // Terminal inspector window; lazily made. QPointer so a WM-driven + // close (treated as hide) or a parent-destroyed cascade leaves the + // pointer null instead of dangling. + QPointer m_inspectorWindow; OverlayScrollbar *m_scrollbar = nullptr; // floating scrollback scrollbar bool m_scrollAtBottom = true; // viewport is following the buffer tail bool m_notifyOnCommand = false; // one-shot: notify on next cmd finish diff --git a/qt/src/GlobalShortcuts.cpp b/qt/src/GlobalShortcuts.cpp index f9b575f65..9c7a00eb3 100644 --- a/qt/src/GlobalShortcuts.cpp +++ b/qt/src/GlobalShortcuts.cpp @@ -95,7 +95,7 @@ void GlobalShortcuts::portalCall(const QString &method, QVariantList args, [method](QDBusPendingCallWatcher *w) { QDBusPendingReply reply = *w; if (reply.isError()) - std::fprintf(stderr, "[ghostty] portal %s failed: %s\n", + std::fprintf(stderr, "[ghastty] portal %s failed: %s\n", method.toUtf8().constData(), reply.error().message().toUtf8().constData()); w->deleteLater(); @@ -111,7 +111,7 @@ void GlobalShortcuts::onResponse(const QDBusMessage &message) { const QVariantMap results = args.size() > 1 ? qdbus_cast(args.at(1)) : QVariantMap(); if (code != 0) - std::fprintf(stderr, "[ghostty] portal %s response code=%u\n", + std::fprintf(stderr, "[ghastty] portal %s response code=%u\n", method.toUtf8().constData(), code); if (method == QLatin1String("CreateSession")) handleCreateSession(code, results); diff --git a/qt/src/InspectorWindow.cpp b/qt/src/InspectorWindow.cpp index 54c16cda8..d1f33f7a1 100644 --- a/qt/src/InspectorWindow.cpp +++ b/qt/src/InspectorWindow.cpp @@ -12,16 +12,9 @@ #include #include -namespace { +#include "Util.h" -ghostty_input_mods_e translateMods(Qt::KeyboardModifiers m) { - int r = GHOSTTY_MODS_NONE; - if (m & Qt::ShiftModifier) r |= GHOSTTY_MODS_SHIFT; - if (m & Qt::ControlModifier) r |= GHOSTTY_MODS_CTRL; - if (m & Qt::AltModifier) r |= GHOSTTY_MODS_ALT; - if (m & Qt::MetaModifier) r |= GHOSTTY_MODS_SUPER; - return static_cast(r); -} +namespace { // The editing/navigation keys an ImGui text field needs; other keys // arrive as text via ghostty_inspector_text. @@ -133,6 +126,14 @@ void InspectorWindow::paintEvent(QPaintEvent *) { void InspectorWindow::resizeEvent(QResizeEvent *) { syncSize(); } +void InspectorWindow::closeEvent(QCloseEvent *e) { + // Hide rather than destroy: the owning GhosttySurface keeps a + // QPointer to this window across show/hide cycles. The window is + // deleted only when the surface is destroyed. + hide(); + e->ignore(); +} + void InspectorWindow::sendMouseButton(QMouseEvent *ev, ghostty_input_mouse_state_e state) { if (!m_inspector) return; diff --git a/qt/src/InspectorWindow.h b/qt/src/InspectorWindow.h index dfd90e2ff..175357cc2 100644 --- a/qt/src/InspectorWindow.h +++ b/qt/src/InspectorWindow.h @@ -15,6 +15,11 @@ class QTimer; // into an offscreen framebuffer owned by a private QOpenGLContext; each // frame is read back into a QImage and painted, mirroring how // GhosttySurface composites the terminal. +// +// The inspector window is shown via a normal Qt::Widget close (WM +// close button), which is treated as "hide", not "destroy" — the +// owning surface keeps a QPointer to it and toggles visibility. The +// window only deletes when its owning GhosttySurface is destroyed. class InspectorWindow : public QWidget { Q_OBJECT @@ -24,6 +29,10 @@ public: ~InspectorWindow() override; protected: + // Treat the WM close button as a hide rather than a destroy. The + // GhosttySurface owns the inspector's lifetime; closing here would + // dangle its QPointer and skip libghostty inspector teardown. + void closeEvent(QCloseEvent *) override; void paintEvent(QPaintEvent *) override; void resizeEvent(QResizeEvent *) override; void mouseMoveEvent(QMouseEvent *) override; diff --git a/qt/src/MainWindow.cpp b/qt/src/MainWindow.cpp index f61a88180..6f20aa358 100644 --- a/qt/src/MainWindow.cpp +++ b/qt/src/MainWindow.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -41,6 +42,7 @@ #include "CommandPalette.h" #include "GhosttySurface.h" #include "TabWidget.h" +#include "Util.h" #include "WindowBlur.h" // Prefix marking a tab with an unacknowledged bell (bell-features title). @@ -54,6 +56,7 @@ QList MainWindow::s_windows; QTimer *MainWindow::s_quitTimer = nullptr; int MainWindow::s_quitDelayMs = 0; MainWindow *MainWindow::s_quickTerminal = nullptr; +QTimer *MainWindow::s_frameTimer = nullptr; std::atomic MainWindow::s_tickPending{false}; MainWindow::MainWindow() { @@ -101,6 +104,14 @@ MainWindow::~MainWindow() { // The shared app and config outlive every window but the last. if (s_windows.isEmpty()) { + if (s_frameTimer) { + // The timer is parented to qApp; stop it so a final tick can't + // fire after s_app is freed below. delete leaves qApp's child + // list to clean up at process exit. + s_frameTimer->stop(); + delete s_frameTimer; + s_frameTimer = nullptr; + } if (s_quitTimer) { delete s_quitTimer; s_quitTimer = nullptr; @@ -220,7 +231,7 @@ bool MainWindow::initialize() { s_app = ghostty_app_new(&rt, s_config); if (!s_app) { - std::fprintf(stderr, "[ghostty] ghostty_app_new failed\n"); + std::fprintf(stderr, "[ghastty] ghostty_app_new failed\n"); return false; } @@ -229,9 +240,7 @@ bool MainWindow::initialize() { // through the libghostty quit_timer action (see handleQuitTimer). const bool quitAfter = configBool("quit-after-last-window-closed", true); unsigned long long delayNs = 0; - ghostty_config_get(s_config, &delayNs, - "quit-after-last-window-closed-delay", - qstrlen("quit-after-last-window-closed-delay")); + configGet(s_config, &delayNs, "quit-after-last-window-closed-delay"); s_quitDelayMs = quitAfter ? static_cast(delayNs / 1000000ULL) : 0; QApplication::setQuitOnLastWindowClosed(quitAfter && s_quitDelayMs == 0); } @@ -255,11 +264,17 @@ bool MainWindow::initialize() { // Tab-bar policy and colour scheme. applyWindowConfig(); - // 60fps frame timer: a backstop tick plus rendering. onWakeup drives - // extra ticks between frames for input responsiveness. - auto *timer = new QTimer(this); - connect(timer, &QTimer::timeout, this, &MainWindow::frame); - timer->start(16); + // Process-wide 60fps frame timer (created on the first window): a + // backstop tick plus rendering. onWakeup drives extra ticks between + // frames for input responsiveness. One timer covers every window — + // N windows would otherwise produce N ticks per 16ms for the same + // shared ghostty_app_t. + if (!s_frameTimer) { + s_frameTimer = new QTimer(qApp); + QObject::connect(s_frameTimer, &QTimer::timeout, qApp, + &MainWindow::frame); + s_frameTimer->start(16); + } // The first tab is created in showEvent, not here: see below. return true; @@ -442,6 +457,7 @@ void MainWindow::adoptTab(MainWindow *src, QWidget *page) { } const QString text = src->m_tabs->tabText(srcIndex); + // QVariant carrying the typed TabData; copies cleanly across windows. const QVariant data = src->m_tabs->tabBar()->tabData(srcIndex); src->m_tabs->removeTab(srcIndex); // page is now parentless const int index = m_tabs->addTab(page, text); // reparents page here @@ -477,10 +493,9 @@ void MainWindow::setSurfaceTitle(GhosttySurface *surface, if (index < 0) return; // Store the terminal title as the tab's base; updateTabText decides // whether it or a manual override is shown. - QStringList data = m_tabs->tabBar()->tabData(index).toStringList(); - while (data.size() < 2) data.append(QString()); - data[0] = title; - m_tabs->tabBar()->setTabData(index, data); + TabData data = m_tabs->tabBar()->tabData(index).value(); + data.base = title; + m_tabs->tabBar()->setTabData(index, QVariant::fromValue(data)); updateTabText(index); } @@ -488,19 +503,18 @@ void MainWindow::setTabTitleOverride(GhosttySurface *surface, const QString &title) { const int index = tabIndexForSurface(surface); if (index < 0) return; - QStringList data = m_tabs->tabBar()->tabData(index).toStringList(); - while (data.size() < 2) data.append(QString()); - data[1] = title; // empty clears the override - m_tabs->tabBar()->setTabData(index, data); + TabData data = m_tabs->tabBar()->tabData(index).value(); + data.override_ = title; // empty clears the override + m_tabs->tabBar()->setTabData(index, QVariant::fromValue(data)); updateTabText(index); } void MainWindow::copyTitleToClipboard() { const int tab = m_tabs->currentIndex(); if (tab < 0) return; - const QStringList data = m_tabs->tabBar()->tabData(tab).toStringList(); + const TabData data = m_tabs->tabBar()->tabData(tab).value(); const QString title = - !data.value(1).isEmpty() ? data.value(1) : data.value(0); + !data.override_.isEmpty() ? data.override_ : data.base; if (!title.isEmpty()) QGuiApplication::clipboard()->setText(title); } @@ -509,7 +523,10 @@ void MainWindow::frame() { ghostty_app_tick(s_app); // Rendering happens only here, so a flood of RENDER actions cannot // saturate the GUI thread — each surface renders at most once a frame. - for (GhosttySurface *s : m_surfaces) s->renderIfDirty(); + // One pass across every window: the shared ghostty_app_t was already + // ticked once above. + for (MainWindow *w : s_windows) + for (GhosttySurface *s : w->m_surfaces) s->renderIfDirty(); } void MainWindow::onTabCloseRequested(int index) { @@ -627,8 +644,7 @@ void MainWindow::setupLayerShell() { // quick-terminal-size: primary is the edge-perpendicular extent. ghostty_config_quick_terminal_size_s qsz = {}; - ghostty_config_get(s_config, &qsz, "quick-terminal-size", - qstrlen("quick-terminal-size")); + configGet(s_config, &qsz, "quick-terminal-size"); const auto toPx = [](const ghostty_quick_terminal_size_s &s, int dim, int fallback) -> int { switch (s.tag) { @@ -666,7 +682,9 @@ void MainWindow::setupLayerShell() { size = {scr.width(), toPx(qsz.primary, scr.height(), 400)}; } ls->setAnchors(anchors); - ls->setDesiredSize(size); + // The layer-shell protocol takes the size from the underlying + // wl_surface (i.e. the QWindow's size); LayerShellQt has no + // setDesiredSize on this Qt branch. resize(size); } @@ -845,18 +863,17 @@ void MainWindow::moveTab(int amount) { } void MainWindow::ringBell(GhosttySurface *surface) { - // bell-features is a packed struct, returned by ghostty_config_get as - // a bitfield: bit 0 system, 1 audio, 2 attention, 3 title, 4 border. - unsigned int features = 1u << 2; // fall back to `attention` - ghostty_config_get(s_config, &features, "bell-features", - qstrlen("bell-features")); - if (features & (1u << 2)) QApplication::alert(this); // attention - if (features & (1u << 0)) QApplication::beep(); // system - if (features & (1u << 1)) playBellAudio(); // audio + // bell-features is a packed struct returned by ghostty_config_get as + // a bitfield (see BellFeature in Util.h). + unsigned int features = BellAttention; // fallback if config-get fails + configGet(s_config, &features, "bell-features"); + if (features & BellAttention) QApplication::alert(this); + if (features & BellSystem) QApplication::beep(); + if (features & BellAudio) playBellAudio(); if (!surface) return; - if (features & (1u << 4)) surface->flashBorder(); // border - if (features & (1u << 3)) { // title + if (features & BellBorder) surface->flashBorder(); + if (features & BellTitle) { const int tab = tabIndexForSurface(surface); // Marking the current tab is pointless — you are looking at it. if (tab >= 0 && tab != m_tabs->currentIndex()) { @@ -874,12 +891,10 @@ bool MainWindow::tabBellMarked(int tab) const { void MainWindow::updateTabText(int tab) { if (tab < 0 || tab >= m_tabs->count()) return; - const QStringList data = m_tabs->tabBar()->tabData(tab).toStringList(); - const QString base = data.value(0); - const QString override = data.value(1); - QString text = !override.isEmpty() ? override - : !base.isEmpty() ? base - : QStringLiteral("Ghastty"); + const TabData data = m_tabs->tabBar()->tabData(tab).value(); + QString text = !data.override_.isEmpty() ? data.override_ + : !data.base.isEmpty() ? data.base + : QStringLiteral("Ghastty"); m_tabs->setTabText(tab, tabBellMarked(tab) ? kBellMark + text : text); if (tab == m_tabs->currentIndex()) setWindowTitle(text + QStringLiteral(" — Ghastty")); @@ -984,7 +999,7 @@ void MainWindow::applyWindowConfig() { scheme = Qt::ColorScheme::Light; } else if (theme == QLatin1String("ghostty")) { ghostty_config_color_s bg{}; - if (ghostty_config_get(s_config, &bg, "background", qstrlen("background"))) { + if (configGet(s_config, &bg, "background")) { const double luma = 0.299 * bg.r + 0.587 * bg.g + 0.114 * bg.b; scheme = luma < 128.0 ? Qt::ColorScheme::Dark : Qt::ColorScheme::Light; } @@ -1003,9 +1018,7 @@ void MainWindow::applyBlur() { // macOS-only negatives) means off, a positive radius means on. KWin // uses its own configured radius, so only on/off matters here. short blur = 0; - if (s_config) - ghostty_config_get(s_config, &blur, "background-blur", - qstrlen("background-blur")); + configGet(s_config, &blur, "background-blur"); applyWindowBlur(this, blur > 0); } @@ -1057,7 +1070,9 @@ void MainWindow::onWakeup(void *) { // Coalesce: queue a shared-app tick only when one is not already // pending, so a chatty surface cannot flood the event loop. May be // called off-thread, so it marshals onto qApp (always alive) rather - // than any particular window. + // than any particular window. The s_app check inside the lambda + // guards against the last window being destroyed (which frees s_app) + // between this wakeup and the queued tick draining. if (s_tickPending.exchange(true)) return; QMetaObject::invokeMethod( qApp, @@ -1068,6 +1083,13 @@ void MainWindow::onWakeup(void *) { Qt::QueuedConnection); } +bool MainWindow::surfaceAlive(GhosttySurface *s) { + if (!s) return false; + for (MainWindow *w : s_windows) + if (w->m_surfaces.contains(s)) return true; + return false; +} + // Map a libghostty mouse shape to the nearest Qt cursor. static Qt::CursorShape mouseShapeToCursor(ghostty_action_mouse_shape_e s) { switch (s) { @@ -1105,32 +1127,14 @@ static Qt::CursorShape mouseShapeToCursor(ghostty_action_mouse_shape_e s) { } } -// Format a keybind trigger as a human-readable chord, e.g. "Ctrl+B". -static QString formatTrigger(const ghostty_input_trigger_s &t) { - QString s; - if (t.mods & GHOSTTY_MODS_CTRL) s += QStringLiteral("Ctrl+"); - if (t.mods & GHOSTTY_MODS_ALT) s += QStringLiteral("Alt+"); - if (t.mods & GHOSTTY_MODS_SHIFT) s += QStringLiteral("Shift+"); - if (t.mods & GHOSTTY_MODS_SUPER) s += QStringLiteral("Super+"); - switch (t.tag) { - case GHOSTTY_TRIGGER_UNICODE: - s += QString(QChar(t.key.unicode)).toUpper(); - break; - case GHOSTTY_TRIGGER_PHYSICAL: { - const ghostty_input_key_e k = t.key.physical; - if (k >= GHOSTTY_KEY_DIGIT_0 && k <= GHOSTTY_KEY_DIGIT_9) - s += QChar('0' + (k - GHOSTTY_KEY_DIGIT_0)); - else if (k >= GHOSTTY_KEY_A && k <= GHOSTTY_KEY_Z) - s += QChar('A' + (k - GHOSTTY_KEY_A)); - else - s += QStringLiteral("•"); // an unmapped physical key - break; - } - default: - s += QStringLiteral("…"); // catch-all - break; - } - return s; +// Queue `f` on `target`'s thread, but only if `target` is still alive +// when the slot runs (Qt cancels queued slots whose receiver was +// deleted). Cross-captured pointers must be wrapped in QPointer +// separately — `target` only protects itself. +template +static void post(Target *target, F &&f) { + if (!target) return; + QMetaObject::invokeMethod(target, std::forward(f), Qt::QueuedConnection); } bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, @@ -1143,9 +1147,14 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, // The window the action applies to: the target surface's window, or // (for app-level actions) any live window. Surface/window work is - // marshalled onto `win` so it is cancelled if that window goes away. + // marshalled onto `win` so it is cancelled if that window goes away; + // *cross*-captured pointers (e.g. `src` when posting to `win`) are + // wrapped in QPointer so they're checked at lambda-execution time — + // a multi-window + tear-off + close race could otherwise UAF. MainWindow *win = src ? src->owner() : (s_windows.isEmpty() ? nullptr : s_windows.first()); + QPointer winp(win); + QPointer srcp(src); // Actions may be dispatched from non-GUI threads, so window-touching // work is marshalled onto the GUI thread. @@ -1158,47 +1167,46 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, case GHOSTTY_ACTION_NEW_TAB: { if (!win) return false; - ghostty_surface_t parent = src ? src->surface() : nullptr; - QMetaObject::invokeMethod( - win, [win, parent]() { win->newTab(parent); }, - Qt::QueuedConnection); + // `parent` is a libghostty handle whose lifetime tracks `src`'s. + // If `src` is gone by the time the lambda runs, drop the parent + // and create an unparented tab. + post(win, [winp, srcp]() { + if (!winp) return; + winp->newTab(srcp ? srcp->surface() : nullptr); + }); return true; } - case GHOSTTY_ACTION_NEW_WINDOW: { - ghostty_surface_t parent = src ? src->surface() : nullptr; - QMetaObject::invokeMethod( - qApp, [parent]() { MainWindow::newWindow(parent); }, - Qt::QueuedConnection); + case GHOSTTY_ACTION_NEW_WINDOW: + post(qApp, [srcp]() { + MainWindow::newWindow(srcp ? srcp->surface() : nullptr); + }); return true; - } case GHOSTTY_ACTION_NEW_SPLIT: { if (!src) return false; const ghostty_action_split_direction_e dir = action.action.new_split; - QMetaObject::invokeMethod( - win, [win, src, dir]() { win->splitSurface(src, dir); }, - Qt::QueuedConnection); + post(win, [winp, srcp, dir]() { + if (winp && srcp) winp->splitSurface(srcp, dir); + }); return true; } case GHOSTTY_ACTION_CLOSE_TAB: if (src) - QMetaObject::invokeMethod( - win, - [win, src]() { - if (win->confirmCloseSurfaces({src})) win->removeSurface(src); - }, - Qt::QueuedConnection); + post(win, [winp, srcp]() { + if (!winp || !srcp) return; + if (winp->confirmCloseSurfaces({srcp})) winp->removeSurface(srcp); + }); return true; case GHOSTTY_ACTION_SET_TITLE: { const char *title = action.action.set_title.title; if (!title || !src) return true; const QString t = QString::fromUtf8(title); - QMetaObject::invokeMethod( - win, [win, src, t]() { win->setSurfaceTitle(src, t); }, - Qt::QueuedConnection); + post(win, [winp, srcp, t]() { + if (winp && srcp) winp->setSurfaceTitle(srcp, t); + }); return true; } @@ -1207,9 +1215,9 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, if (!src) return true; const char *title = action.action.set_tab_title.title; const QString t = QString::fromUtf8(title ? title : ""); - QMetaObject::invokeMethod( - win, [win, src, t]() { win->setTabTitleOverride(src, t); }, - Qt::QueuedConnection); + post(win, [winp, srcp, t]() { + if (winp && srcp) winp->setTabTitleOverride(srcp, t); + }); return true; } @@ -1217,117 +1225,108 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, if (!src) return true; const bool tabScope = action.action.prompt_title == GHOSTTY_PROMPT_TITLE_TAB; - QMetaObject::invokeMethod( - src, [src, tabScope]() { src->promptTitle(tabScope); }, - Qt::QueuedConnection); + post(src, [srcp, tabScope]() { + if (srcp) srcp->promptTitle(tabScope); + }); return true; } case GHOSTTY_ACTION_COPY_TITLE_TO_CLIPBOARD: - if (win) - QMetaObject::invokeMethod( - win, [win]() { win->copyTitleToClipboard(); }, - Qt::QueuedConnection); + post(win, [winp]() { + if (winp) winp->copyTitleToClipboard(); + }); return true; case GHOSTTY_ACTION_RESET_WINDOW_SIZE: - if (win) - QMetaObject::invokeMethod( - win, - [win]() { - win->resize(win->m_defaultWindowSize.isValid() - ? win->m_defaultWindowSize - : QSize(800, 600)); - }, - Qt::QueuedConnection); + post(win, [winp]() { + if (!winp) return; + winp->resize(winp->m_defaultWindowSize.isValid() + ? winp->m_defaultWindowSize + : QSize(800, 600)); + }); return true; case GHOSTTY_ACTION_KEY_SEQUENCE: { if (!src) return true; const ghostty_action_key_sequence_s ks = action.action.key_sequence; if (!ks.active) { - QMetaObject::invokeMethod(src, [src]() { src->endKeySequence(); }, - Qt::QueuedConnection); + post(src, [srcp]() { + if (srcp) srcp->endKeySequence(); + }); return true; } const QString chord = formatTrigger(ks.trigger); - QMetaObject::invokeMethod( - src, [src, chord]() { src->pushKeySequence(chord); }, - Qt::QueuedConnection); + post(src, [srcp, chord]() { + if (srcp) srcp->pushKeySequence(chord); + }); return true; } case GHOSTTY_ACTION_GOTO_TAB: { if (!win) return false; const ghostty_action_goto_tab_e tab = action.action.goto_tab; - QMetaObject::invokeMethod( - win, [win, tab]() { win->gotoTab(tab); }, Qt::QueuedConnection); + post(win, [winp, tab]() { + if (winp) winp->gotoTab(tab); + }); return true; } case GHOSTTY_ACTION_GOTO_SPLIT: { if (!src) return false; const ghostty_action_goto_split_e dir = action.action.goto_split; - QMetaObject::invokeMethod( - win, [win, src, dir]() { win->gotoSplit(src, dir); }, - Qt::QueuedConnection); + post(win, [winp, srcp, dir]() { + if (winp && srcp) winp->gotoSplit(srcp, dir); + }); return true; } case GHOSTTY_ACTION_RESIZE_SPLIT: { if (!src) return false; const ghostty_action_resize_split_s rs = action.action.resize_split; - QMetaObject::invokeMethod( - win, [win, src, rs]() { win->resizeSplit(src, rs); }, - Qt::QueuedConnection); + post(win, [winp, srcp, rs]() { + if (winp && srcp) winp->resizeSplit(srcp, rs); + }); return true; } case GHOSTTY_ACTION_EQUALIZE_SPLITS: if (src) - QMetaObject::invokeMethod( - win, [win, src]() { win->equalizeSplits(src); }, - Qt::QueuedConnection); + post(win, [winp, srcp]() { + if (winp && srcp) winp->equalizeSplits(srcp); + }); return true; case GHOSTTY_ACTION_TOGGLE_FULLSCREEN: if (!win) return false; - QMetaObject::invokeMethod( - win, - [win]() { - if (win->isFullScreen()) - win->showNormal(); - else - win->showFullScreen(); - }, - Qt::QueuedConnection); + post(win, [winp]() { + if (!winp) return; + if (winp->isFullScreen()) + winp->showNormal(); + else + winp->showFullScreen(); + }); return true; case GHOSTTY_ACTION_TOGGLE_MAXIMIZE: if (!win) return false; - QMetaObject::invokeMethod( - win, - [win]() { - if (win->isMaximized()) - win->showNormal(); - else - win->showMaximized(); - }, - Qt::QueuedConnection); + post(win, [winp]() { + if (!winp) return; + if (winp->isMaximized()) + winp->showNormal(); + else + winp->showMaximized(); + }); return true; case GHOSTTY_ACTION_QUIT: case GHOSTTY_ACTION_CLOSE_ALL_WINDOWS: - QMetaObject::invokeMethod(qApp, []() { MainWindow::closeAllWindows(); }, - Qt::QueuedConnection); + post(qApp, []() { MainWindow::closeAllWindows(); }); return true; case GHOSTTY_ACTION_QUIT_TIMER: { const bool start = action.action.quit_timer == GHOSTTY_QUIT_TIMER_START; - QMetaObject::invokeMethod( - qApp, [start]() { MainWindow::handleQuitTimer(start); }, - Qt::QueuedConnection); + post(qApp, [start]() { MainWindow::handleQuitTimer(start); }); return true; } @@ -1335,17 +1334,17 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, if (!src) return false; const int code = static_cast(action.action.child_exited.exit_code); - QMetaObject::invokeMethod( - src, [src, code]() { src->showChildExited(code); }, - Qt::QueuedConnection); + post(src, [srcp, code]() { + if (srcp) srcp->showChildExited(code); + }); return true; } case GHOSTTY_ACTION_TOGGLE_SPLIT_ZOOM: if (src) - QMetaObject::invokeMethod( - win, [win, src]() { win->toggleSplitZoom(src); }, - Qt::QueuedConnection); + post(win, [winp, srcp]() { + if (winp && srcp) winp->toggleSplitZoom(srcp); + }); return true; case GHOSTTY_ACTION_OPEN_CONFIG: { @@ -1355,67 +1354,60 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, if (path.ptr && path.len) { const QString p = QString::fromUtf8(path.ptr, static_cast(path.len)); - QMetaObject::invokeMethod( - qApp, - [p]() { - QDesktopServices::openUrl(QUrl::fromLocalFile(p)); - }, - Qt::QueuedConnection); + post(qApp, + [p]() { QDesktopServices::openUrl(QUrl::fromLocalFile(p)); }); } ghostty_string_free(path); return true; } case GHOSTTY_ACTION_RELOAD_CONFIG: - if (win) - QMetaObject::invokeMethod( - win, [win]() { win->reloadConfig(); }, Qt::QueuedConnection); + post(win, [winp]() { + if (winp) winp->reloadConfig(); + }); return true; case GHOSTTY_ACTION_CONFIG_CHANGE: // A notification: libghostty already holds the new config (this // often fires as the echo of our own ghostty_app_update_config). // Re-pushing it would loop, so just refresh window chrome. - QMetaObject::invokeMethod(qApp, []() { MainWindow::refreshChrome(); }, - Qt::QueuedConnection); + post(qApp, []() { MainWindow::refreshChrome(); }); return true; case GHOSTTY_ACTION_INITIAL_SIZE: { if (!win) return false; const ghostty_action_initial_size_s sz = action.action.initial_size; - QMetaObject::invokeMethod( - win, - [win, sz]() { - // The action carries device pixels; resize() takes logical. - const double dpr = win->devicePixelRatioF(); - const QSize logical(static_cast(sz.width / dpr), - static_cast(sz.height / dpr)); - win->m_defaultWindowSize = logical; // for RESET_WINDOW_SIZE - win->resize(logical); - }, - Qt::QueuedConnection); + post(win, [winp, sz]() { + if (!winp) return; + // The action carries device pixels; resize() takes logical. + const double dpr = winp->devicePixelRatioF(); + const QSize logical(static_cast(sz.width / dpr), + static_cast(sz.height / dpr)); + winp->m_defaultWindowSize = logical; // for RESET_WINDOW_SIZE + winp->resize(logical); + }); return true; } case GHOSTTY_ACTION_CLOSE_WINDOW: - if (win) - QMetaObject::invokeMethod(win, [win]() { win->close(); }, - Qt::QueuedConnection); + post(win, [winp]() { + if (winp) winp->close(); + }); return true; case GHOSTTY_ACTION_RING_BELL: - if (win) - QMetaObject::invokeMethod(win, [win, src]() { win->ringBell(src); }, - Qt::QueuedConnection); + post(win, [winp, srcp]() { + if (winp) winp->ringBell(srcp); + }); return true; case GHOSTTY_ACTION_MOUSE_SHAPE: { if (!src) return false; const Qt::CursorShape shape = mouseShapeToCursor(action.action.mouse_shape); - QMetaObject::invokeMethod( - src, [src, shape]() { src->setCursor(shape); }, - Qt::QueuedConnection); + post(src, [srcp, shape]() { + if (srcp) srcp->setCursor(shape); + }); return true; } @@ -1424,8 +1416,9 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, const ghostty_action_mouse_over_link_s l = action.action.mouse_over_link; const QString url = l.url && l.len ? QString::fromUtf8(l.url, l.len) : QString(); - QMetaObject::invokeMethod( - src, [src, url]() { src->setToolTip(url); }, Qt::QueuedConnection); + post(src, [srcp, url]() { + if (srcp) srcp->setToolTip(url); + }); return true; } @@ -1433,13 +1426,10 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, const ghostty_action_open_url_s u = action.action.open_url; if (!u.url || !u.len) return true; const QString s = QString::fromUtf8(u.url, static_cast(u.len)); - QMetaObject::invokeMethod( - qApp, - [s]() { - QDesktopServices::openUrl( - QUrl::fromUserInput(s, QString(), QUrl::AssumeLocalFile)); - }, - Qt::QueuedConnection); + post(qApp, [s]() { + QDesktopServices::openUrl( + QUrl::fromUserInput(s, QString(), QUrl::AssumeLocalFile)); + }); return true; } @@ -1448,34 +1438,28 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, action.action.desktop_notification; const QString title = QString::fromUtf8(n.title ? n.title : ""); const QString body = QString::fromUtf8(n.body ? n.body : ""); - QMetaObject::invokeMethod( - qApp, [title, body]() { postNotification(title, body); }, - Qt::QueuedConnection); + post(qApp, [title, body]() { postNotification(title, body); }); return true; } case GHOSTTY_ACTION_COMMAND_FINISHED: { if (!src) return true; const int code = action.action.command_finished.exit_code; - QMetaObject::invokeMethod( - src, - [src, code]() { - if (!src->consumeCommandNotify()) return; - postNotification( - QStringLiteral("Command finished"), - code >= 0 ? QStringLiteral("Exited with code %1").arg(code) - : QStringLiteral("The command completed.")); - }, - Qt::QueuedConnection); + post(src, [srcp, code]() { + if (!srcp || !srcp->consumeCommandNotify()) return; + postNotification( + QStringLiteral("Command finished"), + code >= 0 ? QStringLiteral("Exited with code %1").arg(code) + : QStringLiteral("The command completed.")); + }); return true; } case GHOSTTY_ACTION_MOVE_TAB: { const int amount = static_cast(action.action.move_tab.amount); - if (win) - QMetaObject::invokeMethod( - win, [win, amount]() { win->moveTab(amount); }, - Qt::QueuedConnection); + post(win, [winp, amount]() { + if (winp) winp->moveTab(amount); + }); return true; } @@ -1483,27 +1467,23 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, if (!src) return false; const bool hidden = action.action.mouse_visibility == GHOSTTY_MOUSE_HIDDEN; - QMetaObject::invokeMethod( - src, - [src, hidden]() { - src->setCursor(hidden ? Qt::BlankCursor : Qt::ArrowCursor); - }, - Qt::QueuedConnection); + post(src, [srcp, hidden]() { + if (srcp) srcp->setCursor(hidden ? Qt::BlankCursor : Qt::ArrowCursor); + }); return true; } case GHOSTTY_ACTION_RENDERER_HEALTH: if (action.action.renderer_health == GHOSTTY_RENDERER_HEALTH_UNHEALTHY) - std::fprintf(stderr, "[ghostty] renderer reported unhealthy\n"); + std::fprintf(stderr, "[ghastty] renderer reported unhealthy\n"); return true; case GHOSTTY_ACTION_SCROLLBAR: { if (!src) return false; const ghostty_action_scrollbar_s s = action.action.scrollbar; - QMetaObject::invokeMethod( - src, - [src, s]() { src->updateScrollbar(s.total, s.offset, s.len); }, - Qt::QueuedConnection); + post(src, [srcp, s]() { + if (srcp) srcp->updateScrollbar(s.total, s.offset, s.len); + }); return true; } @@ -1511,52 +1491,47 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, const ghostty_action_progress_report_s p = action.action.progress_report; const bool visible = p.state != GHOSTTY_PROGRESS_STATE_REMOVE; const double fraction = p.progress >= 0 ? p.progress / 100.0 : 0.0; - QMetaObject::invokeMethod( - qApp, [visible, fraction]() { postProgress(visible, fraction); }, - Qt::QueuedConnection); + post(qApp, [visible, fraction]() { postProgress(visible, fraction); }); return true; } case GHOSTTY_ACTION_TOGGLE_VISIBILITY: - QMetaObject::invokeMethod(qApp, - []() { MainWindow::toggleVisibility(); }, - Qt::QueuedConnection); + post(qApp, []() { MainWindow::toggleVisibility(); }); return true; case GHOSTTY_ACTION_TOGGLE_QUICK_TERMINAL: - QMetaObject::invokeMethod(qApp, - []() { MainWindow::toggleQuickTerminal(); }, - Qt::QueuedConnection); + post(qApp, []() { MainWindow::toggleQuickTerminal(); }); return true; case GHOSTTY_ACTION_TOGGLE_COMMAND_PALETTE: - if (win) - QMetaObject::invokeMethod( - win, [win, src]() { win->toggleCommandPalette(src); }, - Qt::QueuedConnection); + post(win, [winp, srcp]() { + if (winp) winp->toggleCommandPalette(srcp); + }); return true; case GHOSTTY_ACTION_START_SEARCH: { if (!src) return true; const char *needle = action.action.start_search.needle; const QString n = QString::fromUtf8(needle ? needle : ""); - QMetaObject::invokeMethod(src, [src, n]() { src->openSearch(n); }, - Qt::QueuedConnection); + post(src, [srcp, n]() { + if (srcp) srcp->openSearch(n); + }); return true; } case GHOSTTY_ACTION_END_SEARCH: if (src) - QMetaObject::invokeMethod(src, [src]() { src->closeSearch(); }, - Qt::QueuedConnection); + post(src, [srcp]() { + if (srcp) srcp->closeSearch(); + }); return true; case GHOSTTY_ACTION_SEARCH_TOTAL: { if (!src) return true; const int total = static_cast(action.action.search_total.total); - QMetaObject::invokeMethod( - src, [src, total]() { src->setSearchTotal(total); }, - Qt::QueuedConnection); + post(src, [srcp, total]() { + if (srcp) srcp->setSearchTotal(total); + }); return true; } @@ -1564,18 +1539,18 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, if (!src) return true; const int sel = static_cast(action.action.search_selected.selected); - QMetaObject::invokeMethod( - src, [src, sel]() { src->setSearchSelected(sel); }, - Qt::QueuedConnection); + post(src, [srcp, sel]() { + if (srcp) srcp->setSearchSelected(sel); + }); return true; } case GHOSTTY_ACTION_INSPECTOR: { if (!src) return true; const ghostty_action_inspector_e mode = action.action.inspector; - QMetaObject::invokeMethod( - src, [src, mode]() { src->toggleInspector(mode); }, - Qt::QueuedConnection); + post(src, [srcp, mode]() { + if (srcp) srcp->toggleInspector(mode); + }); return true; } @@ -1587,9 +1562,11 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, bool MainWindow::onReadClipboard(void *ud, ghostty_clipboard_e loc, void *state) { // surface userdata. Called synchronously when libghostty needs - // clipboard contents (paste). + // clipboard contents (paste). May arrive on a worker thread, so + // surfaceAlive validates the pointer first — the GhosttySurface + // could be mid-destruction. auto *surface = static_cast(ud); - if (!surface || !surface->surface()) return false; + if (!surfaceAlive(surface) || !surface->surface()) return false; const QClipboard::Mode mode = loc == GHOSTTY_CLIPBOARD_SELECTION ? QClipboard::Selection @@ -1608,24 +1585,30 @@ void MainWindow::onConfirmReadClipboard(void *ud, const char *str, void *state, // through the render tick — a crash/freeze. `state` is a completion // token valid until used; `str` is not, so copy it. auto *surface = static_cast(ud); - if (!surface || !surface->surface()) return; + if (!surfaceAlive(surface) || !surface->surface()) return; + QPointer sp(surface); const QByteArray content(str); QMetaObject::invokeMethod( surface->owner(), - [surface, content, state]() { - if (!surface->surface()) return; + [sp, content, state]() { + if (!sp || !sp->surface()) return; QString preview = QString::fromUtf8(content); - if (preview.size() > 200) - preview = preview.left(200) + QStringLiteral("…"); + // Truncate by code unit but back off to a non-surrogate boundary + // so we don't slice a surrogate pair half. + if (preview.size() > 200) { + int cut = 200; + while (cut > 0 && preview.at(cut - 1).isHighSurrogate()) --cut; + preview = preview.left(cut) + QStringLiteral("…"); + } const auto reply = QMessageBox::warning( - surface->owner(), QStringLiteral("Confirm Paste"), + sp->owner(), QStringLiteral("Confirm Paste"), QStringLiteral("The text being pasted may be unsafe:\n\n%1\n\n" "Paste anyway?") .arg(preview), QMessageBox::Yes | QMessageBox::No, QMessageBox::No); ghostty_surface_complete_clipboard_request( - surface->surface(), content.constData(), state, + sp->surface(), content.constData(), state, reply == QMessageBox::Yes); }, Qt::QueuedConnection); @@ -1636,14 +1619,16 @@ void MainWindow::onWriteClipboard(void *ud, ghostty_clipboard_e loc, size_t n, bool) { if (n == 0 || !content[0].data) return; auto *surface = static_cast(ud); - if (!surface) return; + if (!surfaceAlive(surface)) return; const QClipboard::Mode mode = loc == GHOSTTY_CLIPBOARD_SELECTION ? QClipboard::Selection : QClipboard::Clipboard; const QString text = QString::fromUtf8(content[0].data); + // The clipboard is process-global; route via qApp so a window dying + // mid-flight does not strand the write. QMetaObject::invokeMethod( - surface->owner(), + qApp, [text, mode]() { QGuiApplication::clipboard()->setText(text, mode); }, Qt::QueuedConnection); } @@ -1652,13 +1637,15 @@ void MainWindow::onCloseSurface(void *ud, bool) { // surface userdata. Deferred out of this callback so the confirm // dialog cannot spin a nested event loop back into libghostty. auto *surface = static_cast(ud); - if (!surface) return; + if (!surfaceAlive(surface)) return; MainWindow *self = surface->owner(); + QPointer selfp(self); + QPointer sp(surface); QMetaObject::invokeMethod( self, - [self, surface]() { - if (self->confirmCloseSurfaces({surface})) - self->removeSurface(surface); + [selfp, sp]() { + if (!selfp || !sp) return; + if (selfp->confirmCloseSurfaces({sp})) selfp->removeSurface(sp); }, Qt::QueuedConnection); } diff --git a/qt/src/MainWindow.h b/qt/src/MainWindow.h index 33baed960..d609df8b4 100644 --- a/qt/src/MainWindow.h +++ b/qt/src/MainWindow.h @@ -91,8 +91,11 @@ private: // Create the first tab once the device pixel ratio has settled. void createFirstTab(); - // 60fps frame timer: ticks libghostty and renders any dirty surface. - void frame(); + // 60fps frame timer body. Static because there is only one timer + // per process — N windows pointing at the same shared ghostty_app_t. + // Ticks libghostty once and renders any dirty surface across every + // window. + static void frame(); void closeTab(int index); // Tear tab `index` out into a new window (tabTornOff signal). @@ -184,6 +187,12 @@ private: bool); static void onCloseSurface(void *ud, bool process_active); + // True if `s` is still owned by some live MainWindow. The surface + // userdata callbacks above use this to validate a libghostty-supplied + // pointer before dereferencing — a worker-thread callback can race + // the GhosttySurface destructor. + static bool surfaceAlive(GhosttySurface *s); + TabWidget *m_tabs = nullptr; QList m_surfaces; // every live surface in this window bool m_firstTabPending = true; // first tab is created on show() @@ -202,6 +211,10 @@ private: static QTimer *s_quitTimer; // delayed quit-after-last-window static int s_quitDelayMs; // 0 = no delay configured static MainWindow *s_quickTerminal; // the one quick terminal, if any + // Process-wide 60Hz frame timer. Replaces a per-window timer, so N + // windows do not produce N ghostty_app_tick calls every 16ms for the + // same shared app. + static QTimer *s_frameTimer; // Coalesces wakeup-driven ticks: a tick is queued at most once at a // time, so a busy surface can't flood the event loop. diff --git a/qt/src/OverlayScrollbar.cpp b/qt/src/OverlayScrollbar.cpp index d33dd09a9..293c624c4 100644 --- a/qt/src/OverlayScrollbar.cpp +++ b/qt/src/OverlayScrollbar.cpp @@ -55,7 +55,10 @@ void OverlayScrollbar::setMetrics(quint64 total, quint64 offset, m_total = total; m_offset = offset; m_len = len; - if (isVisible()) update(); + // Repaint when visible OR while a fade-out is in flight; the handle + // position changes constantly with output, and skipping the update + // makes the fading scrollbar lag behind the actual scrollback. + if (isVisible() || m_opacity > 0.0) update(); } void OverlayScrollbar::fadeTo(qreal target, int ms) { diff --git a/qt/src/TabWidget.cpp b/qt/src/TabWidget.cpp index 5d32ae9a4..6bc27ee54 100644 --- a/qt/src/TabWidget.cpp +++ b/qt/src/TabWidget.cpp @@ -1,18 +1,40 @@ #include "TabWidget.h" +#include + +#include +#include #include #include #include #include #include #include +#include #include namespace { -// Set by a TabBar::dropEvent during an in-flight tear-off. It is the -// reliable "released on a tab bar" signal: QDrag::exec()'s return value -// cannot be trusted across surfaces on Wayland. -bool g_tabDropHandled = false; +// MIME role carrying a pointer-to-originating-TabBar so a receiving +// bar's dropEvent can mark the originator's m_dropHandled. We can't +// rely on QDrag::exec()'s return value on Wayland, and a process-wide +// "drop handled" flag races with simultaneous tear-offs in different +// windows. +constexpr char kTearOffOriginRole[] = "application/x-ghastty-tab-origin"; + +QByteArray encodeOrigin(TabBar *bar) { + // Pack the raw pointer; the source process owns it for the lifetime + // of the drag. We never dereference unless we're in the same process, + // and a tear-off across processes is meaningless. + QByteArray bytes(reinterpret_cast(&bar), sizeof(bar)); + return bytes; +} + +TabBar *decodeOrigin(const QByteArray &bytes) { + if (bytes.size() != sizeof(TabBar *)) return nullptr; + TabBar *bar; + std::memcpy(&bar, bytes.constData(), sizeof(bar)); + return bar; +} } // namespace void TabBar::mousePressEvent(QMouseEvent *e) { @@ -64,6 +86,10 @@ void TabBar::startTearOff(QMouseEvent *e) { QDrag *drag = new QDrag(this); auto *mime = new QMimeData; mime->setData(QString::fromLatin1(kGhosttyTabMime), QByteArray()); + // Tag the drag with a pointer to this bar so the receiving bar's + // dropEvent can mark *our* m_dropHandled — a process-global flag + // would race with simultaneous tear-offs in other windows. + mime->setData(QString::fromLatin1(kTearOffOriginRole), encodeOrigin(this)); drag->setMimeData(mime); drag->setPixmap(grab(tabBox)); drag->setHotSpot(m_pressPos - tabBox.topLeft()); @@ -77,15 +103,15 @@ void TabBar::startTearOff(QMouseEvent *e) { // Released on a tab bar cancels the tear-off; released anywhere else // (the terminal, another window, the desktop) tears it into a new - // window. g_tabDropHandled — set by TabBar::dropEvent — is the - // signal, since QDrag::exec()'s result is unreliable across surfaces - // on Wayland. - g_tabDropHandled = false; + // window. m_dropHandled — set by a TabBar::dropEvent on the + // originating bar — is the signal, since QDrag::exec()'s result is + // unreliable across surfaces on Wayland. + m_dropHandled = false; drag->exec(Qt::MoveAction); m_tearing = false; m_pressIndex = -1; - if (!g_tabDropHandled) emit tabTornOff(index); + if (!m_dropHandled) emit tabTornOff(index); } void TabBar::dragEnterEvent(QDragEnterEvent *e) { @@ -94,9 +120,15 @@ void TabBar::dragEnterEvent(QDragEnterEvent *e) { } void TabBar::dropEvent(QDropEvent *e) { - // Dropping a tear-off back on a tab bar cancels it. + // Dropping a tear-off back on a tab bar cancels it. Mark the flag on + // the *originating* bar (carried in the MIME payload), not this one + // — a tear-off can be dropped onto a different window's bar. if (e->mimeData()->hasFormat(QString::fromLatin1(kGhosttyTabMime))) { - g_tabDropHandled = true; + if (TabBar *origin = decodeOrigin( + e->mimeData()->data(QString::fromLatin1(kTearOffOriginRole)))) + origin->m_dropHandled = true; + else + m_dropHandled = true; // fallback: mark ourselves e->acceptProposedAction(); } } diff --git a/qt/src/WindowBlur.cpp b/qt/src/WindowBlur.cpp index bf1f68314..5dea0d3ba 100644 --- a/qt/src/WindowBlur.cpp +++ b/qt/src/WindowBlur.cpp @@ -60,6 +60,14 @@ org_kde_kwin_blur_manager *blurManager(wl_display *display) { return globals.manager; } +// The live blur object per window — kept so it can be released when +// blur is turned off, re-applied on a config change, or the window +// itself is destroyed. +static QHash &waylandBlurs() { + static QHash blurs; + return blurs; +} + void applyWayland(QWindow *window, bool enabled) { QPlatformNativeInterface *native = QGuiApplication::platformNativeInterface(); if (!native) return; @@ -72,9 +80,7 @@ void applyWayland(QWindow *window, bool enabled) { org_kde_kwin_blur_manager *manager = blurManager(display); if (!manager) return; // compositor advertises no blur support - // The live blur object per window — kept so it can be released when - // blur is turned off or re-applied on a config change. - static QHash blurs; + auto &blurs = waylandBlurs(); if (org_kde_kwin_blur *old = blurs.take(window)) org_kde_kwin_blur_release(old); @@ -84,6 +90,15 @@ void applyWayland(QWindow *window, bool enabled) { org_kde_kwin_blur_set_region(blur, nullptr); // null = whole surface org_kde_kwin_blur_commit(blur); blurs.insert(window, blur); + + // Release the blur object when the window goes away. Without this, + // a closed window leaves its org_kde_kwin_blur leaked and the + // QWindow* key in the hash dangles. + QObject::connect(window, &QWindow::destroyed, qApp, [window]() { + auto &b = waylandBlurs(); + if (org_kde_kwin_blur *old = b.take(window)) + org_kde_kwin_blur_release(old); + }); } else { org_kde_kwin_blur_manager_unset(manager, surface); } diff --git a/qt/src/main.cpp b/qt/src/main.cpp index 4271eb8ed..462645b9a 100644 --- a/qt/src/main.cpp +++ b/qt/src/main.cpp @@ -49,14 +49,14 @@ int main(int argc, char **argv) { // re-scans that array for CLI config — scanning the pre-strip array // would walk past its end into freed/null entries. if (ghostty_init(static_cast(argc), argv) != GHOSTTY_SUCCESS) { - std::fprintf(stderr, "[ghostty] ghostty_init failed\n"); + std::fprintf(stderr, "[ghastty] ghostty_init failed\n"); return 1; } // The first window; further windows are opened on demand by the // new_window action. Each window owns itself (WA_DeleteOnClose). if (!MainWindow::newWindow(nullptr)) { - std::fprintf(stderr, "[ghostty] window initialization failed\n"); + std::fprintf(stderr, "[ghastty] window initialization failed\n"); return 1; }