From 8cb886838d74275dbc84cc846edcc7f4fc974a4a Mon Sep 17 00:00:00 2001 From: ntomsic Date: Sat, 23 May 2026 11:14:04 -0500 Subject: [PATCH 1/7] =?UTF-8?q?qt:=20phase=201.0=20=E2=80=94=20extract=20G?= =?UTF-8?q?hosttyApp=20singleton=20for=20libghostty=20handles?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 1 of the modularization plan to break MainWindow.cpp's 3000-line god-class apart. This step is intentionally minimal: only the ghostty_app_t and ghostty_config_t handles, plus the bring-up / teardown lifecycle that owns them, move to a new GhosttyApp singleton at qt/src/app/. Specifically moved: - ghostty_app_new + the runtime callback registration → ensureInitialized - ghostty_app_free + qApp MetaCall drain + ghostty_config_free → teardown - ghostty_config_t swap from reloadConfigGlobal → replaceConfig - configHasCustomShader (custom-shader is an app-level fact) Stays on MainWindow: - All six runtime callbacks (onWakeup, onAction, onReadClipboard, onConfirmReadClipboard, onWriteClipboard, onCloseSurface). They access private MainWindow members today; phase 1.1+ will handle them when the rest of the per-window state is sorted. - s_windows registry, undo stack, frame timer, quit timer - The window-flag / applyWindowConfig / blur half of initialize - All per-window state and lifecycle The MainWindow s_app / s_config / s_needsPremultiply statics now mirror the singleton's values so the existing call sites across GhosttySurface.cpp + the unmoved half of MainWindow.cpp keep working unchanged. Phase 1.1+ retire them. GhosttyApp is friended by MainWindow so it can register the still- private MainWindow::onWakeup et al. callbacks with libghostty. The friendship is documented as phase-1.0-only — it dies with the next step that moves the callbacks themselves. Net diff: ~75 lines deleted from MainWindow.cpp, ~125 lines added in GhosttyApp.{h,cpp}. No behaviour changes; pure code motion. Build verification: not run on this Mac (no Qt toolchain). Needs a docker compile (qt/docker/Dockerfile.qt) before merge. Co-Authored-By: claude-flow --- qt/CMakeLists.txt | 1 + qt/src/MainWindow.cpp | 110 +++++++++++----------------------- qt/src/MainWindow.h | 5 ++ qt/src/app/GhosttyApp.cpp | 122 ++++++++++++++++++++++++++++++++++++++ qt/src/app/GhosttyApp.h | 59 ++++++++++++++++++ 5 files changed, 222 insertions(+), 75 deletions(-) create mode 100644 qt/src/app/GhosttyApp.cpp create mode 100644 qt/src/app/GhosttyApp.h diff --git a/qt/CMakeLists.txt b/qt/CMakeLists.txt index 78628823d..dc48acda3 100644 --- a/qt/CMakeLists.txt +++ b/qt/CMakeLists.txt @@ -98,6 +98,7 @@ add_custom_target(ghostty_link DEPENDS "${GHOSTTY_LINK_SO}") add_executable(ghastty src/main.cpp + src/app/GhosttyApp.cpp src/CommandPalette.cpp src/GhosttySurface.cpp src/GlobalShortcuts.cpp diff --git a/qt/src/MainWindow.cpp b/qt/src/MainWindow.cpp index 30cb1feac..0b87cb869 100644 --- a/qt/src/MainWindow.cpp +++ b/qt/src/MainWindow.cpp @@ -54,6 +54,7 @@ #include +#include "app/GhosttyApp.h" #include "CommandPalette.h" #include "GhosttySurface.h" #include "TabWidget.h" @@ -179,7 +180,7 @@ MainWindow::~MainWindow() { 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. The QObject destructor + // fire after the app is freed below. The QObject destructor // unparents it from qApp. s_frameTimer->stop(); delete s_frameTimer; @@ -189,48 +190,19 @@ MainWindow::~MainWindow() { delete s_quitTimer; s_quitTimer = nullptr; } - // Drain qApp-targeted MetaCalls posted by worker-thread libghostty - // callbacks (closeAllWindows, refreshChrome, OPEN_URL, postProgress, - // handleQuitTimer, NEW_WINDOW, CONFIG_CHANGE, ...) — these are the - // ones that can still touch s_app/s_config after their original - // window has gone. Lambdas posted to per-window/per-surface - // receivers are auto-cancelled by Qt when those receivers were - // deleted above (qDeleteAll above and the broader Qt object tree - // teardown), so they don't need draining. - // - // sendPostedEvents only drains the named receiver, not its - // children — which is exactly what we want here. - QCoreApplication::sendPostedEvents(qApp, QEvent::MetaCall); - if (s_app) { - ghostty_app_free(s_app); - s_app = nullptr; - } - if (s_config) { - ghostty_config_free(s_config); - s_config = nullptr; - } + // GhosttyApp::teardown drains qApp-targeted MetaCalls (so worker + // callbacks can't touch a freed app) and ghostty_app_frees / + // ghostty_config_frees the live handles. The local s_app / + // s_config statics are mirrors that still source from the + // singleton; null them so they match the singleton state. + GhosttyApp::instance().teardown(); + s_app = nullptr; + s_config = nullptr; } } -// Whether the Ghostty config enables a custom shader. libghostty does -// not expose this through ghostty_config_get (`custom-shader` is a -// repeatable path), so scan the primary config file directly. -static bool configHasCustomShader() { - QString dir = qEnvironmentVariable("XDG_CONFIG_HOME"); - if (dir.isEmpty()) dir = QDir::homePath() + QStringLiteral("/.config"); - - QFile f(dir + QStringLiteral("/ghostty/config")); - if (!f.open(QIODevice::ReadOnly | QIODevice::Text)) return false; - - while (!f.atEnd()) { - const QByteArray line = f.readLine().trimmed(); - if (!line.startsWith("custom-shader")) continue; - // Require a non-empty value: `custom-shader =` alone clears it. - const int eq = line.indexOf('='); - if (eq >= 0 && !line.mid(eq + 1).trimmed().isEmpty()) return true; - } - return false; -} +// configHasCustomShader moved to GhosttyApp.cpp (custom-shader is an +// app-level fact: it drives needsPremultiply for every surface). // Parse a libghostty duration string into nanoseconds. The format is // concatenated `` segments per Config.zig's Duration.parseCLI: @@ -421,37 +393,25 @@ static void openUrlByKind(const QString &url, } bool MainWindow::initialize() { + // First-call: build libghostty app + config via the singleton. The + // singleton holds m_app / m_config / m_needsPremultiply; the + // MainWindow s_app / s_config / s_needsPremultiply statics are now + // forwarders that read it. + if (!GhosttyApp::instance().ensureInitialized()) return false; + // Mirror the singleton's pointers into the legacy statics so the + // existing call sites (the rest of MainWindow.cpp + GhosttySurface) + // keep working unchanged. Phases 1.1+ swap them out. + s_app = GhosttyApp::instance().app(); + s_config = GhosttyApp::instance().config(); + s_needsPremultiply = GhosttyApp::instance().needsPremultiply(); + s_windows.append(this); - // The first window builds the shared libghostty app and config; every - // later window reuses them. - if (!s_app) { - // Load configuration in the same order as the reference apprt. - s_config = ghostty_config_new(); - ghostty_config_load_default_files(s_config); - ghostty_config_load_cli_args(s_config); - ghostty_config_load_recursive_files(s_config); - ghostty_config_finalize(s_config); - s_needsPremultiply = configHasCustomShader(); - - ghostty_runtime_config_s rt = {}; - // No app userdata: actions are routed to a window via their target - // surface, and app-level actions via the s_windows registry. - rt.userdata = nullptr; - rt.supports_selection_clipboard = true; - rt.wakeup_cb = onWakeup; - rt.action_cb = onAction; - rt.read_clipboard_cb = onReadClipboard; - rt.confirm_read_clipboard_cb = onConfirmReadClipboard; - rt.write_clipboard_cb = onWriteClipboard; - rt.close_surface_cb = onCloseSurface; - - s_app = ghostty_app_new(&rt, s_config); - if (!s_app) { - std::fprintf(stderr, "[ghastty] ghostty_app_new failed\n"); - return false; - } - + // First window also caches the quit-after-last-window-closed state. + // Subsequent windows skip it (the singleton already holds the live + // value via its config; only the QApplication quit strategy is set + // once here). + if (s_windows.size() == 1) { // quit-after-last-window-closed: Qt's native "quit on last window" // covers the common (no-delay) case; a configured delay is honored // through the libghostty quit_timer action (see handleQuitTimer). @@ -1646,12 +1606,12 @@ void MainWindow::reloadConfigGlobal() { // chrome, never re-pushes, so this does not loop. ghostty_app_update_config(s_app, config); - // Adopt the new config. libghostty keeps borrowed references to it - // (the surface message queue), so it must outlive this call — which - // it does, as the live s_config. - if (s_config && s_config != config) ghostty_config_free(s_config); - s_config = config; - s_needsPremultiply = configHasCustomShader(); + // Hand the new config to the singleton, which frees the previous one + // (in the right order — libghostty borrows the previous until update + // completes) and recomputes needsPremultiply. + GhosttyApp::instance().replaceConfig(config); + s_config = GhosttyApp::instance().config(); + s_needsPremultiply = GhosttyApp::instance().needsPremultiply(); refreshChrome(); diff --git a/qt/src/MainWindow.h b/qt/src/MainWindow.h index c05a847b5..1120128c9 100644 --- a/qt/src/MainWindow.h +++ b/qt/src/MainWindow.h @@ -125,6 +125,11 @@ private slots: void onCurrentChanged(int index); private: + // GhosttyApp registers our static runtime callbacks (onWakeup, + // onAction, ...) with libghostty. Phase 1.0 only — phase 1.1 + // moves the callbacks onto GhosttyApp itself and drops this. + friend class GhosttyApp; + // Create the first tab once the device pixel ratio has settled. void createFirstTab(); diff --git a/qt/src/app/GhosttyApp.cpp b/qt/src/app/GhosttyApp.cpp new file mode 100644 index 000000000..4371b8e86 --- /dev/null +++ b/qt/src/app/GhosttyApp.cpp @@ -0,0 +1,122 @@ +#include "GhosttyApp.h" + +#include + +#include +#include +#include +#include +#include +#include + +#include "../MainWindow.h" + +// Process-wide libghostty state. Only the libghostty handles + their +// bring-up / teardown lifecycle live here in phase 1.0; the runtime +// callbacks (onWakeup, onAction, onReadClipboard, ...) and the window +// registry, undo stack, frame timer, etc. all still live on +// MainWindow and migrate in subsequent phases. + +// Whether the Ghostty config enables a custom shader. libghostty does +// not expose this through ghostty_config_get (`custom-shader` is a +// repeatable path), so scan the primary config file directly. Same +// implementation MainWindow had before — moved here because +// needsPremultiply is now an app-level fact. +static bool configHasCustomShader() { + QString dir = qEnvironmentVariable("XDG_CONFIG_HOME"); + if (dir.isEmpty()) dir = QDir::homePath() + QStringLiteral("/.config"); + + QFile f(dir + QStringLiteral("/ghostty/config")); + if (!f.open(QIODevice::ReadOnly | QIODevice::Text)) return false; + + while (!f.atEnd()) { + const QByteArray line = f.readLine().trimmed(); + if (!line.startsWith("custom-shader")) continue; + // Require a non-empty value: `custom-shader =` alone clears it. + const int eq = line.indexOf('='); + if (eq >= 0 && !line.mid(eq + 1).trimmed().isEmpty()) return true; + } + return false; +} + +GhosttyApp &GhosttyApp::instance() { + // Static-local singleton: deterministic destruction at process exit + // (after Qt has already torn down QObject children). Construction + // is deferred until the first call so QApplication exists by then. + static GhosttyApp self; + return self; +} + +GhosttyApp::~GhosttyApp() { + // Backstop for an early-exit path (ghostty_init failure inside + // main()). The normal teardown runs from the last MainWindow's dtor. + teardown(); +} + +bool GhosttyApp::ensureInitialized() { + if (m_app) return true; + + // Load configuration in the same order as the reference apprt. + m_config = ghostty_config_new(); + ghostty_config_load_default_files(m_config); + ghostty_config_load_cli_args(m_config); + ghostty_config_load_recursive_files(m_config); + ghostty_config_finalize(m_config); + m_needsPremultiply = configHasCustomShader(); + + ghostty_runtime_config_s rt = {}; + // No app userdata: actions are routed to a window via their target + // surface, and app-level actions via the MainWindow window registry. + rt.userdata = nullptr; + rt.supports_selection_clipboard = true; + // Phase 1.0: every callback still lives on MainWindow. Phase 1.1 + // moves them onto GhosttyApp and the registration switches to + // GhosttyApp::onWakeup et al. + rt.wakeup_cb = MainWindow::onWakeup; + rt.action_cb = MainWindow::onAction; + rt.read_clipboard_cb = MainWindow::onReadClipboard; + rt.confirm_read_clipboard_cb = MainWindow::onConfirmReadClipboard; + rt.write_clipboard_cb = MainWindow::onWriteClipboard; + rt.close_surface_cb = MainWindow::onCloseSurface; + + m_app = ghostty_app_new(&rt, m_config); + if (!m_app) { + std::fprintf(stderr, "[ghastty] ghostty_app_new failed\n"); + ghostty_config_free(m_config); + m_config = nullptr; + return false; + } + return true; +} + +void GhosttyApp::replaceConfig(ghostty_config_t new_config) { + // libghostty keeps borrowed references to the previous config (the + // surface message queue), so the new must be installed and the old + // freed in this order. + if (m_config && m_config != new_config) ghostty_config_free(m_config); + m_config = new_config; + m_needsPremultiply = configHasCustomShader(); +} + +void GhosttyApp::teardown() { + // Drain qApp-targeted MetaCalls posted by worker-thread libghostty + // callbacks (closeAllWindows, refreshChrome, OPEN_URL, postProgress, + // handleQuitTimer, NEW_WINDOW, CONFIG_CHANGE, ...) — these are the + // ones that can still touch m_app / m_config after their original + // window has gone. Lambdas posted to per-window/per-surface + // receivers are auto-cancelled by Qt when those receivers are + // deleted, so they don't need draining. + // + // sendPostedEvents only drains the named receiver, not its + // children — which is exactly what we want here. + QCoreApplication::sendPostedEvents(qApp, QEvent::MetaCall); + if (m_app) { + ghostty_app_free(m_app); + m_app = nullptr; + } + if (m_config) { + ghostty_config_free(m_config); + m_config = nullptr; + } + m_needsPremultiply = false; +} diff --git a/qt/src/app/GhosttyApp.h b/qt/src/app/GhosttyApp.h new file mode 100644 index 000000000..dfea7bad0 --- /dev/null +++ b/qt/src/app/GhosttyApp.h @@ -0,0 +1,59 @@ +#pragma once + +#include "ghostty.h" + +class MainWindow; + +// Process-wide libghostty integration. +// +// Owns the single ghostty_app_t and ghostty_config_t instances that +// drive every window in the process, plus the derived needsPremultiply +// flag that the surfaces' renderer reads when blitting frames. +// +// Singleton — there is never more than one libghostty app per +// process. Construction is deferred to the first instance() call so +// QApplication can exist before the singleton is built. +// +// Phase 1.0 scope: only the libghostty handles + bring-up / teardown +// live here. The frame timer, runtime callbacks, window registry, +// undo stack, quit-timer state, and action dispatch all stay on +// MainWindow for now; subsequent phases migrate them. +class GhosttyApp { +public: + static GhosttyApp &instance(); + + // libghostty handles. Null until ensureInitialized() succeeds. + ghostty_app_t app() const { return m_app; } + ghostty_config_t config() const { return m_config; } + bool needsPremultiply() const { return m_needsPremultiply; } + + // Builds the libghostty config + app the first time it's called, + // wiring the runtime callback bundle that MainWindow currently + // hosts (onWakeup / onAction / onReadClipboard / ... — all still + // implemented on MainWindow during phase 1.0). + // + // Re-entrant: subsequent calls early-return true. + // Returns false on libghostty init failure. + bool ensureInitialized(); + + // Refresh m_config + m_needsPremultiply from disk (called from + // MainWindow::reloadConfigGlobal). The caller is responsible for + // pushing the new config to libghostty (ghostty_app_update_config) + // and refreshing window chrome — those iterate the window list, + // which still lives on MainWindow during phase 1.0. + void replaceConfig(ghostty_config_t new_config); + + // Free m_app + m_config. Called from MainWindow::~MainWindow when + // the last window goes away. Idempotent. + void teardown(); + +private: + GhosttyApp() = default; + ~GhosttyApp(); + GhosttyApp(const GhosttyApp &) = delete; + GhosttyApp &operator=(const GhosttyApp &) = delete; + + ghostty_app_t m_app = nullptr; + ghostty_config_t m_config = nullptr; + bool m_needsPremultiply = false; +}; From 8417d81279a389cf6ee37b72ddbb6b702f6f5650 Mon Sep 17 00:00:00 2001 From: ntomsic Date: Sat, 23 May 2026 11:43:59 -0500 Subject: [PATCH 2/7] =?UTF-8?q?qt:=20phase=201.1=20=E2=80=94=20move=20s=5F?= =?UTF-8?q?windows=20registry=20into=20GhosttyApp?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 2 of the modularization. The live MainWindow list now lives on GhosttyApp::m_windows; MainWindow registers/unregisters itself in initialize() / ~MainWindow() and reads the list via GhosttyApp::instance().windows() at every callsite. ~25 callsite rewrites, all mechanical. Behaviour preserved exactly: the list is the same QList in the same registration order, just hosted on the singleton. Wherever the previous code took a const-ref to s_windows (frame, toggleVisibility, closeAllWindows, etc.), the new code does the same with the singleton's accessor. Still on MainWindow for now: the frame timer (s_frameTimer), quit timer (s_quitTimer + s_quitDelayMs), tick coalescer (s_tickPending), quick-terminal pointer (s_quickTerminal), undo/redo stacks, and the six runtime callbacks. Phase 1.2+ migrate those. Build verification: not run on this Mac. Needs a docker compile before merge. Co-Authored-By: claude-flow --- qt/src/MainWindow.cpp | 88 +++++++++++++++++++++++---------------- qt/src/MainWindow.h | 11 +++-- qt/src/app/GhosttyApp.cpp | 8 ++++ qt/src/app/GhosttyApp.h | 18 ++++++++ 4 files changed, 86 insertions(+), 39 deletions(-) diff --git a/qt/src/MainWindow.cpp b/qt/src/MainWindow.cpp index 0b87cb869..48e5ebdda 100644 --- a/qt/src/MainWindow.cpp +++ b/qt/src/MainWindow.cpp @@ -88,10 +88,14 @@ static QIcon bellAttentionIcon() { } // Process-shared libghostty state — see MainWindow.h. +// +// s_app / s_config / s_needsPremultiply mirror GhosttyApp::instance() +// so the rest of this file (and GhosttySurface) keep their existing +// access patterns. Phase 1.2+ will retire them as call sites move +// over to the singleton directly. ghostty_app_t MainWindow::s_app = nullptr; ghostty_config_t MainWindow::s_config = nullptr; bool MainWindow::s_needsPremultiply = false; -QList MainWindow::s_windows; QTimer *MainWindow::s_quitTimer = nullptr; int MainWindow::s_quitDelayMs = 0; MainWindow *MainWindow::s_quickTerminal = nullptr; @@ -157,7 +161,7 @@ MainWindow::MainWindow() { } MainWindow::~MainWindow() { - s_windows.removeOne(this); + GhosttyApp::instance().unregisterWindow(this); if (this == s_quickTerminal) s_quickTerminal = nullptr; // Destroy this window's surfaces (freeing their ghostty_surface_t) @@ -171,13 +175,14 @@ MainWindow::~MainWindow() { // so the delay can run), so without this the process would stay // alive forever after closing the final window via the WM. // Mirrors GTK's application.zig:820-862 startQuitTimer wiring. - if (s_windows.isEmpty() && s_quitDelayMs > 0) { + const bool wasLast = GhosttyApp::instance().windows().isEmpty(); + if (wasLast && s_quitDelayMs > 0) { handleQuitTimer(true); - return; // keep s_app + s_config alive until the timer fires + return; // keep the app + config alive until the timer fires } // The shared app and config outlive every window but the last. - if (s_windows.isEmpty()) { + if (wasLast) { if (s_frameTimer) { // The timer is parented to qApp; stop it so a final tick can't // fire after the app is freed below. The QObject destructor @@ -405,13 +410,13 @@ bool MainWindow::initialize() { s_config = GhosttyApp::instance().config(); s_needsPremultiply = GhosttyApp::instance().needsPremultiply(); - s_windows.append(this); + GhosttyApp::instance().registerWindow(this); // First window also caches the quit-after-last-window-closed state. // Subsequent windows skip it (the singleton already holds the live // value via its config; only the QApplication quit strategy is set // once here). - if (s_windows.size() == 1) { + if (GhosttyApp::instance().windows().size() == 1) { // quit-after-last-window-closed: Qt's native "quit on last window" // covers the common (no-delay) case; a configured delay is honored // through the libghostty quit_timer action (see handleQuitTimer). @@ -498,9 +503,11 @@ MainWindow *MainWindow::newWindow(ghostty_surface_t parent) { const bool haveY = configGet(s_config, &posY, "window-position-y"); if (haveX && haveY) { w->move(posX, posY); - } else if (s_windows.size() > 1) { - if (MainWindow *prev = s_windows.value(s_windows.size() - 2)) { - w->move(prev->pos() + QPoint(30, 30)); + } else { + const QList &all = GhosttyApp::instance().windows(); + if (all.size() > 1) { + if (MainWindow *prev = all.value(all.size() - 2)) + w->move(prev->pos() + QPoint(30, 30)); } } @@ -867,8 +874,9 @@ void MainWindow::frame() { // destroys a window or surface mid-frame can't UAF the iterator // or the inner-loop receiver. QList> windows; - windows.reserve(s_windows.size()); - for (MainWindow *w : s_windows) windows.append(w); + const QList &live = GhosttyApp::instance().windows(); + windows.reserve(live.size()); + for (MainWindow *w : live) windows.append(w); for (const QPointer &wp : windows) { if (!wp) continue; QList> surfaces; @@ -954,7 +962,8 @@ void MainWindow::closeAllWindows(bool thenQuit) { : QStringLiteral("Close All Windows"); const QString verb = thenQuit ? QStringLiteral("Quit") : QStringLiteral("Close All"); - QMessageBox box(s_windows.isEmpty() ? nullptr : s_windows.first()); + const QList &live = GhosttyApp::instance().windows(); + QMessageBox box(live.isEmpty() ? nullptr : live.first()); box.setIcon(QMessageBox::Warning); box.setWindowTitle(title); box.setText(QStringLiteral("There are still running processes.")); @@ -967,8 +976,8 @@ void MainWindow::closeAllWindows(bool thenQuit) { box.exec(); if (box.clickedButton() != go) return; } - // Copy: each close() may delete the window and mutate s_windows. - const QList windows = s_windows; + // Copy: each close() may delete the window and mutate the live list. + const QList windows = GhosttyApp::instance().windows(); for (MainWindow *w : windows) { w->m_skipCloseConfirm = true; w->close(); @@ -997,13 +1006,14 @@ void MainWindow::closeAllWindows(bool thenQuit) { void MainWindow::toggleVisibility() { // If anything is showing, hide everything; otherwise reveal it all. + const QList &live = GhosttyApp::instance().windows(); bool anyVisible = false; - for (MainWindow *w : s_windows) + for (MainWindow *w : live) if (w->isVisible()) { anyVisible = true; break; } - for (MainWindow *w : s_windows) { + for (MainWindow *w : live) { if (anyVisible) { w->hide(); } else { @@ -1544,7 +1554,7 @@ void MainWindow::refreshChrome() { QApplication::setQuitOnLastWindowClosed(quitAfter && s_quitDelayMs == 0); } - for (MainWindow *w : s_windows) { + for (MainWindow *w : GhosttyApp::instance().windows()) { w->applyWindowConfig(); w->applyBlur(); @@ -1663,18 +1673,20 @@ void MainWindow::presentTerminal(GhosttySurface *surface) { if (surface) surface->setFocus(); } -// Cycle through s_windows. The libghostty target picks a starting -// window (the one whose surface fired the action); GOTO_WINDOW_NEXT -// goes forward, PREVIOUS goes backward, wrapping at the ends. +// Cycle through the live window list. The libghostty target picks a +// starting window (the one whose surface fired the action); +// GOTO_WINDOW_NEXT goes forward, PREVIOUS goes backward, wrapping at +// the ends. void MainWindow::gotoWindow(MainWindow *from, ghostty_action_goto_window_e dir) { - const int n = s_windows.size(); + const QList &live = GhosttyApp::instance().windows(); + const int n = live.size(); if (n <= 1) return; - const int idx = from ? s_windows.indexOf(from) : 0; + const int idx = from ? live.indexOf(from) : 0; if (idx < 0) return; const int step = (dir == GHOSTTY_GOTO_WINDOW_NEXT) ? 1 : -1; const int next = (idx + step + n) % n; - if (MainWindow *w = s_windows.value(next)) w->presentTerminal(nullptr); + if (MainWindow *w = live.value(next)) w->presentTerminal(nullptr); } // FLOAT_WINDOW: keep this window above other windows (Qt:: @@ -1799,16 +1811,17 @@ void MainWindow::undoLastClose() { // The active window picks the new tab's parent surface for cwd // inheritance. Skip the quick terminal — it doesn't push undo // entries and isn't a meaningful target. Fall back to the most - // recent regular window in s_windows order. + // recent regular window in registration order. auto isUndoTarget = [](MainWindow *w) { return w && !w->m_quickTerminal; }; MainWindow *active = qobject_cast(qApp->activeWindow()); if (!isUndoTarget(active)) { active = nullptr; - for (int i = s_windows.size() - 1; i >= 0; --i) { - if (isUndoTarget(s_windows.at(i))) { - active = s_windows.at(i); + const QList &live = GhosttyApp::instance().windows(); + for (int i = live.size() - 1; i >= 0; --i) { + if (isUndoTarget(live.at(i))) { + active = live.at(i); break; } } @@ -1871,7 +1884,10 @@ void MainWindow::redoLastClose() { UndoEntry e = s_redoStack.takeLast(); MainWindow *active = qobject_cast(qApp->activeWindow()); - if (!active && !s_windows.isEmpty()) active = s_windows.last(); + if (!active) { + const QList &live = GhosttyApp::instance().windows(); + if (!live.isEmpty()) active = live.last(); + } if (!active) { // No window to act on — restore the entry so the user can retry. s_redoStack.append(std::move(e)); @@ -2049,7 +2065,7 @@ void MainWindow::onWakeup(void *) { bool MainWindow::surfaceAlive(GhosttySurface *s) { if (!s) return false; - for (MainWindow *w : s_windows) + for (MainWindow *w : GhosttyApp::instance().windows()) if (w->m_surfaces.contains(s)) return true; return false; } @@ -2115,8 +2131,9 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, // *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. + const QList &live_ = GhosttyApp::instance().windows(); MainWindow *win = src ? src->owner() - : (s_windows.isEmpty() ? nullptr : s_windows.first()); + : (live_.isEmpty() ? nullptr : live_.first()); QPointer winp(win); QPointer srcp(src); @@ -2194,9 +2211,10 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, // global keybind can rename even when no surface is the action's // explicit target. Mirrors macOS NSApp.mainWindow promotion. GhosttySurface *target = src; - if (!target && !s_windows.isEmpty()) { + const QList &live_pt = GhosttyApp::instance().windows(); + if (!target && !live_pt.isEmpty()) { MainWindow *active = qobject_cast(qApp->activeWindow()); - if (!active) active = s_windows.first(); + if (!active) active = live_pt.first(); if (active) target = active->surfaceAt(active->m_tabs->currentIndex()); } if (!target) return false; @@ -2516,7 +2534,7 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, case GHOSTTY_ACTION_MOVE_TAB: { // Surface-target only: an app-target MOVE_TAB has no meaningful - // window to apply to (we'd just pick s_windows.first() arbitrarily). + // window to apply to (we'd just pick the first live one arbitrarily). // macOS returns false here — performable falls through to the // running terminal on no live window. if (target.tag != GHOSTTY_TARGET_SURFACE || !src) return false; @@ -2660,7 +2678,7 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, case GHOSTTY_ACTION_GOTO_WINDOW: { // Performable: return false on a single window so the chord // falls through to the terminal. - if (s_windows.size() <= 1) return false; + if (GhosttyApp::instance().windows().size() <= 1) return false; const ghostty_action_goto_window_e dir = action.action.goto_window; post(qApp, [winp, dir]() { MainWindow::gotoWindow(winp.data(), dir); }); diff --git a/qt/src/MainWindow.h b/qt/src/MainWindow.h index 1120128c9..a25fcd65e 100644 --- a/qt/src/MainWindow.h +++ b/qt/src/MainWindow.h @@ -87,7 +87,7 @@ public: // PRESENT_TERMINAL: bring this window to front and focus the surface. void presentTerminal(GhosttySurface *surface); - // GOTO_WINDOW: cycle to the previous/next window in s_windows order. + // GOTO_WINDOW: cycle to the previous/next window in registration order. static void gotoWindow(MainWindow *from, ghostty_action_goto_window_e dir); // FLOAT_WINDOW / TOGGLE_WINDOW_DECORATIONS / TOGGLE_BACKGROUND_OPACITY: @@ -233,7 +233,8 @@ private: void toggleSplitZoom(GhosttySurface *surface); // Runtime callbacks dispatched by libghostty. wakeup/action are - // app-level (routed via the target surface or s_windows); clipboard/ + // app-level (routed via the target surface or the GhosttyApp window + // registry); clipboard/ // close carry the surface userdata. static void onWakeup(void *ud); static bool onAction(ghostty_app_t, ghostty_target_s, ghostty_action_s); @@ -279,11 +280,13 @@ private: // Process-shared libghostty state: one app and config drive every // window. Created by the first initialize(), freed with the last - // window. s_windows tracks every live window. + // window. The live window list lives on GhosttyApp; the s_app / + // s_config / s_needsPremultiply statics here are mirror caches kept + // in sync with GhosttyApp::instance() to limit phase-1 callsite + // churn — they retire as call sites move to the singleton. static ghostty_app_t s_app; static ghostty_config_t s_config; static bool s_needsPremultiply; // a custom shader is configured - static QList s_windows; 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 diff --git a/qt/src/app/GhosttyApp.cpp b/qt/src/app/GhosttyApp.cpp index 4371b8e86..e2bc9fc00 100644 --- a/qt/src/app/GhosttyApp.cpp +++ b/qt/src/app/GhosttyApp.cpp @@ -98,6 +98,14 @@ void GhosttyApp::replaceConfig(ghostty_config_t new_config) { m_needsPremultiply = configHasCustomShader(); } +void GhosttyApp::registerWindow(MainWindow *w) { + m_windows.append(w); +} + +void GhosttyApp::unregisterWindow(MainWindow *w) { + m_windows.removeOne(w); +} + void GhosttyApp::teardown() { // Drain qApp-targeted MetaCalls posted by worker-thread libghostty // callbacks (closeAllWindows, refreshChrome, OPEN_URL, postProgress, diff --git a/qt/src/app/GhosttyApp.h b/qt/src/app/GhosttyApp.h index dfea7bad0..70b940359 100644 --- a/qt/src/app/GhosttyApp.h +++ b/qt/src/app/GhosttyApp.h @@ -1,5 +1,7 @@ #pragma once +#include + #include "ghostty.h" class MainWindow; @@ -47,6 +49,16 @@ public: // the last window goes away. Idempotent. void teardown(); + // ---- window registry -------------------------------------------- + // + // Every live MainWindow registers itself here at construction and + // removes itself at destruction. Replaces the MainWindow::s_windows + // static. + + void registerWindow(MainWindow *w); + void unregisterWindow(MainWindow *w); + const QList &windows() const { return m_windows; } + private: GhosttyApp() = default; ~GhosttyApp(); @@ -56,4 +68,10 @@ private: ghostty_app_t m_app = nullptr; ghostty_config_t m_config = nullptr; bool m_needsPremultiply = false; + + // Live MainWindow list. Order is registration order; MainWindow + // relies on that for cascade-position fallback (see newWindow), the + // GOTO_WINDOW cycle, and the "most recent regular window" lookup + // in undoLastClose. Migrated wholesale from MainWindow::s_windows. + QList m_windows; }; From 2dcfb631281d496644d72036c97b17dab4892c17 Mon Sep 17 00:00:00 2001 From: ntomsic Date: Sat, 23 May 2026 11:49:41 -0500 Subject: [PATCH 3/7] =?UTF-8?q?qt:=20phase=201.2=20=E2=80=94=20move=20fram?= =?UTF-8?q?e=20+=20quit=20timers=20+=20onWakeup=20to=20GhosttyApp?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 3 of the modularization. The 60Hz frame timer, the quit-after-last-window-closed timer, the wakeup tick coalescer, and the libghostty onWakeup callback all move from MainWindow's static state onto the GhosttyApp singleton. Specifically migrated: - s_frameTimer / MainWindow::frame() → m_frameTimer / GhosttyApp::frame - s_quitTimer + s_quitDelayMs / MainWindow::handleQuitTimer → m_quitTimer + m_quitDelayMs / GhosttyApp::handleQuitTimer - s_tickPending / MainWindow::onWakeup → m_tickPending / GhosttyApp::onWakeup - The frame-timer cleanup that lived in ~MainWindow now happens in GhosttyApp::teardown (timers stopped + freed before the qApp MetaCall drain so a final timeout can't dispatch a tick on the about-to-be-freed app). The remaining s_app / s_config / s_needsPremultiply / s_quitDelayMs mirrors stay on MainWindow as caches kept in sync with the singleton; phase 1.3 retires them as the per-window initialize() / config call sites move to GhosttyApp directly. ensureInitialized now registers GhosttyApp::onWakeup as the libghostty wakeup_cb. The other five callbacks (onAction, onReadClipboard, onConfirmReadClipboard, onWriteClipboard, onCloseSurface) still live on MainWindow and migrate in phase 1.3 alongside their state. New public accessor: MainWindow::surfaces() — read-only access to m_surfaces so GhosttyApp::frame can walk every surface for renderIfDirty without poking private members. Build verification: not run on this Mac. Needs a docker compile before merge. Co-Authored-By: claude-flow --- qt/src/MainWindow.cpp | 132 ++++++++------------------------------ qt/src/MainWindow.h | 36 +++++------ qt/src/app/GhosttyApp.cpp | 107 ++++++++++++++++++++++++++++-- qt/src/app/GhosttyApp.h | 49 ++++++++++++++ 4 files changed, 192 insertions(+), 132 deletions(-) diff --git a/qt/src/MainWindow.cpp b/qt/src/MainWindow.cpp index 48e5ebdda..d6bf6e2d0 100644 --- a/qt/src/MainWindow.cpp +++ b/qt/src/MainWindow.cpp @@ -96,11 +96,8 @@ static QIcon bellAttentionIcon() { ghostty_app_t MainWindow::s_app = nullptr; ghostty_config_t MainWindow::s_config = nullptr; bool MainWindow::s_needsPremultiply = false; -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() { setWindowTitle(QStringLiteral("Ghastty")); @@ -176,30 +173,18 @@ MainWindow::~MainWindow() { // alive forever after closing the final window via the WM. // Mirrors GTK's application.zig:820-862 startQuitTimer wiring. const bool wasLast = GhosttyApp::instance().windows().isEmpty(); - if (wasLast && s_quitDelayMs > 0) { - handleQuitTimer(true); + if (wasLast && GhosttyApp::instance().quitDelayMs() > 0) { + GhosttyApp::instance().handleQuitTimer(true); return; // keep the app + config alive until the timer fires } // The shared app and config outlive every window but the last. if (wasLast) { - if (s_frameTimer) { - // The timer is parented to qApp; stop it so a final tick can't - // fire after the app is freed below. The QObject destructor - // unparents it from qApp. - s_frameTimer->stop(); - delete s_frameTimer; - s_frameTimer = nullptr; - } - if (s_quitTimer) { - delete s_quitTimer; - s_quitTimer = nullptr; - } - // GhosttyApp::teardown drains qApp-targeted MetaCalls (so worker - // callbacks can't touch a freed app) and ghostty_app_frees / - // ghostty_config_frees the live handles. The local s_app / - // s_config statics are mirrors that still source from the - // singleton; null them so they match the singleton state. + // GhosttyApp::teardown stops + frees the frame and quit timers, + // drains qApp-targeted MetaCalls (so worker callbacks can't touch + // a freed app), and ghostty_app_frees + ghostty_config_frees the + // live handles. The local s_app / s_config mirrors are still in + // use across this file; null them so they match the singleton. GhosttyApp::instance().teardown(); s_app = nullptr; s_config = nullptr; @@ -427,10 +412,12 @@ bool MainWindow::initialize() { const uint64_t delayNs = parseDurationNs( configValue(QStringLiteral("quit-after-last-window-closed-delay")), 0); const uint64_t delayMs = delayNs / 1000000ULL; - s_quitDelayMs = quitAfter + const int delayMsInt = quitAfter ? static_cast(std::min(delayMs, uint64_t(INT_MAX))) : 0; - QApplication::setQuitOnLastWindowClosed(quitAfter && s_quitDelayMs == 0); + GhosttyApp::instance().setQuitDelayMs(delayMsInt); + s_quitDelayMs = delayMsInt; // mirror; phase 1.3 retires the static + QApplication::setQuitOnLastWindowClosed(quitAfter && delayMsInt == 0); } // Per-window startup window state, applied before show(). None of it @@ -452,17 +439,9 @@ bool MainWindow::initialize() { // Tab-bar policy and colour scheme. applyWindowConfig(); - // 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); - } + // Process-wide 60fps frame timer + libghostty wakeup coalescing + // both live on GhosttyApp now. + GhosttyApp::instance().ensureFrameTimer(); // The first tab is created in showEvent, not here: see below. return true; @@ -472,8 +451,9 @@ MainWindow *MainWindow::newWindow(ghostty_surface_t parent) { // If the natural-close quit timer is running (because the last // window was closed and we're inside the configured delay), cancel // it now: the process is no longer headless. macOS/GTK do the - // same. - if (s_quitTimer) handleQuitTimer(false); + // same. handleQuitTimer is a no-op when no delay is configured, so + // calling it unconditionally is safe. + GhosttyApp::instance().handleQuitTimer(false); auto *w = new MainWindow; w->setAttribute(Qt::WA_DeleteOnClose); // self-destruct when closed @@ -861,34 +841,6 @@ void MainWindow::copyTitleToClipboard(GhosttySurface *src) { if (!title.isEmpty()) QGuiApplication::clipboard()->setText(title); } -void MainWindow::frame() { - if (!s_app) return; - 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. One pass across every window: the shared ghostty_app_t - // was already ticked once above. - // - // Iterate via QPointer snapshots so a render-driven close - // (renderer-unhealthy chain, child-exited press, etc.) that - // destroys a window or surface mid-frame can't UAF the iterator - // or the inner-loop receiver. - QList> windows; - const QList &live = GhosttyApp::instance().windows(); - windows.reserve(live.size()); - for (MainWindow *w : live) windows.append(w); - for (const QPointer &wp : windows) { - if (!wp) continue; - QList> surfaces; - surfaces.reserve(wp->m_surfaces.size()); - for (GhosttySurface *s : wp->m_surfaces) surfaces.append(s); - for (const QPointer &sp : surfaces) { - if (!wp || !sp) continue; - sp->renderIfDirty(); - } - } -} - void MainWindow::onTabCloseRequested(int index) { if (!confirmCloseSurfaces(surfacesInTab(index))) return; closeTab(index); @@ -994,8 +946,9 @@ void MainWindow::closeAllWindows(bool thenQuit) { // Qt's quitOnLastWindowClosed terminates as the last window's // close event runs. We read both decisions off the *cached* // QApplication state so they stay consistent: refreshChrome - // sets quitOnLastWindowClosed and s_quitDelayMs together. - if (QApplication::quitOnLastWindowClosed() && s_quitDelayMs == 0) { + // sets quitOnLastWindowClosed and the singleton's delay together. + if (QApplication::quitOnLastWindowClosed() && + GhosttyApp::instance().quitDelayMs() == 0) { qApp->quit(); } // Else: the close loop above already triggered the natural-close @@ -1217,25 +1170,6 @@ void MainWindow::changeEvent(QEvent *e) { QWidget::changeEvent(e); } -void MainWindow::handleQuitTimer(bool start) { - // Only meaningful when a delay is configured; otherwise Qt's - // quitOnLastWindowClosed already handles the quit. - if (s_quitDelayMs <= 0) return; - if (start) { - if (!s_quitTimer) { - // Parent to qApp for consistency with s_frameTimer; the dtor - // still deletes it explicitly when the last window closes. - s_quitTimer = new QTimer(qApp); - s_quitTimer->setSingleShot(true); - QObject::connect(s_quitTimer, &QTimer::timeout, qApp, - &QApplication::quit); - } - s_quitTimer->start(s_quitDelayMs); - } else if (s_quitTimer) { - s_quitTimer->stop(); - } -} - void MainWindow::onCurrentChanged(int index) { GhosttySurface *s = surfaceAt(index); if (!s) return; @@ -1548,10 +1482,12 @@ void MainWindow::refreshChrome() { const uint64_t delayNs = parseDurationNs( configValue(QStringLiteral("quit-after-last-window-closed-delay")), 0); const uint64_t delayMs = delayNs / 1000000ULL; - s_quitDelayMs = quitAfter + const int delayMsInt = quitAfter ? static_cast(std::min(delayMs, uint64_t(INT_MAX))) : 0; - QApplication::setQuitOnLastWindowClosed(quitAfter && s_quitDelayMs == 0); + GhosttyApp::instance().setQuitDelayMs(delayMsInt); + s_quitDelayMs = delayMsInt; // mirror; phase 1.3 retires the static + QApplication::setQuitOnLastWindowClosed(quitAfter && delayMsInt == 0); } for (MainWindow *w : GhosttyApp::instance().windows()) { @@ -2046,23 +1982,6 @@ void MainWindow::toggleSplitZoom(GhosttySurface *surface) { // --- libghostty runtime callbacks ------------------------------------ -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. 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, - []() { - s_tickPending.store(false); - if (s_app) ghostty_app_tick(s_app); - }, - Qt::QueuedConnection); -} - bool MainWindow::surfaceAlive(GhosttySurface *s) { if (!s) return false; for (MainWindow *w : GhosttyApp::instance().windows()) @@ -2338,7 +2257,8 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, case GHOSTTY_ACTION_QUIT_TIMER: { const bool start = action.action.quit_timer == GHOSTTY_QUIT_TIMER_START; - post(qApp, [start]() { MainWindow::handleQuitTimer(start); }); + post(qApp, + [start]() { GhosttyApp::instance().handleQuitTimer(start); }); return true; } diff --git a/qt/src/MainWindow.h b/qt/src/MainWindow.h index a25fcd65e..74c362423 100644 --- a/qt/src/MainWindow.h +++ b/qt/src/MainWindow.h @@ -111,6 +111,11 @@ public: // focus when the pointer enters it. bool focusFollowsMouse() const; + // Live surface list owned by this window. Read by GhosttyApp::frame + // to walk every surface for renderIfDirty without exposing the + // private m_surfaces member. + const QList &surfaces() const { return m_surfaces; } + protected: bool event(QEvent *) override; void showEvent(QShowEvent *) override; @@ -133,11 +138,7 @@ private: // Create the first tab once the device pixel ratio has settled. void createFirstTab(); - // 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(); + // The frame-timer body lives on GhosttyApp::frame (process-wide). void closeTab(int index); // Honor close-tab-mode (THIS / OTHER / RIGHT) from libghostty. @@ -224,19 +225,17 @@ private: // matching macOS where close-all and quit are distinct. static void closeAllWindows(bool thenQuit); - // Wire the libghostty quit_timer action to a delayed QApplication - // quit, gated on `quit-after-last-window-closed`. - static void handleQuitTimer(bool start); + // The quit-after-last-window-closed timer lives on + // GhosttyApp::handleQuitTimer. // Toggle a split pane filling its tab. Re-parents the surface out of // / back into the splitter tree. void toggleSplitZoom(GhosttySurface *surface); - // Runtime callbacks dispatched by libghostty. wakeup/action are - // app-level (routed via the target surface or the GhosttyApp window - // registry); clipboard/ - // close carry the surface userdata. - static void onWakeup(void *ud); + // Runtime callbacks dispatched by libghostty. action is app-level + // (routed via the target surface or the GhosttyApp window + // registry); clipboard/close carry the surface userdata. wakeup + // moved to GhosttyApp::onWakeup in phase 1.2. static bool onAction(ghostty_app_t, ghostty_target_s, ghostty_action_s); static bool onReadClipboard(void *ud, ghostty_clipboard_e, void *state); static void onConfirmReadClipboard(void *ud, const char *, void *state, @@ -287,13 +286,10 @@ private: static ghostty_app_t s_app; static ghostty_config_t s_config; static bool s_needsPremultiply; // a custom shader is configured - static QTimer *s_quitTimer; // delayed quit-after-last-window + // Mirror of GhosttyApp::quitDelayMs; phase 1.3 retires it when the + // remaining call site (closeAllWindows) moves to the singleton. 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; // Snapshot of a closed tab or window for undo/redo. `pageTitles` // holds each tab's last-known title (window snapshots have N tabs; @@ -323,9 +319,7 @@ private: // single Window entry; called from closeAllWindows / closeEvent. void pushWindowUndo(); - // Coalesces wakeup-driven ticks: a tick is queued at most once at a - // time, so a busy surface can't flood the event loop. - static std::atomic s_tickPending; + // Wakeup tick coalescing lives on GhosttyApp::m_tickPending. // Split-zoom state: the surface temporarily filling its tab, the // splitter it came from, its index there, and the stashed tree root. diff --git a/qt/src/app/GhosttyApp.cpp b/qt/src/app/GhosttyApp.cpp index e2bc9fc00..985bbe0b7 100644 --- a/qt/src/app/GhosttyApp.cpp +++ b/qt/src/app/GhosttyApp.cpp @@ -2,13 +2,17 @@ #include +#include #include #include #include #include #include +#include #include +#include +#include "../GhosttySurface.h" #include "../MainWindow.h" // Process-wide libghostty state. Only the libghostty handles + their @@ -66,13 +70,13 @@ bool GhosttyApp::ensureInitialized() { ghostty_runtime_config_s rt = {}; // No app userdata: actions are routed to a window via their target - // surface, and app-level actions via the MainWindow window registry. + // surface, and app-level actions via the GhosttyApp window registry. rt.userdata = nullptr; rt.supports_selection_clipboard = true; - // Phase 1.0: every callback still lives on MainWindow. Phase 1.1 - // moves them onto GhosttyApp and the registration switches to - // GhosttyApp::onWakeup et al. - rt.wakeup_cb = MainWindow::onWakeup; + // onWakeup migrated in phase 1.2; the rest still live on + // MainWindow and migrate alongside the action dispatcher in + // phase 1.3+. + rt.wakeup_cb = GhosttyApp::onWakeup; rt.action_cb = MainWindow::onAction; rt.read_clipboard_cb = MainWindow::onReadClipboard; rt.confirm_read_clipboard_cb = MainWindow::onConfirmReadClipboard; @@ -106,7 +110,100 @@ void GhosttyApp::unregisterWindow(MainWindow *w) { m_windows.removeOne(w); } +void GhosttyApp::ensureFrameTimer() { + if (m_frameTimer) return; + // Process-wide 60fps frame timer: 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. + m_frameTimer = new QTimer(qApp); + QObject::connect(m_frameTimer, &QTimer::timeout, qApp, + [this]() { frame(); }); + m_frameTimer->start(16); +} + +void GhosttyApp::handleQuitTimer(bool start) { + // Only meaningful when a delay is configured; otherwise Qt's + // quitOnLastWindowClosed already handles the quit. + if (m_quitDelayMs <= 0) return; + if (start) { + if (!m_quitTimer) { + // Parent to qApp for consistency with m_frameTimer; teardown() + // still deletes it explicitly when the last window closes. + m_quitTimer = new QTimer(qApp); + m_quitTimer->setSingleShot(true); + QObject::connect(m_quitTimer, &QTimer::timeout, qApp, + &QApplication::quit); + } + m_quitTimer->start(m_quitDelayMs); + } else if (m_quitTimer) { + m_quitTimer->stop(); + } +} + +void GhosttyApp::frame() { + if (!m_app) return; + ghostty_app_tick(m_app); + // Rendering happens only here, so a flood of RENDER actions cannot + // saturate the GUI thread — each surface renders at most once a + // frame. One pass across every window: the shared ghostty_app_t + // was already ticked once above. + // + // Iterate via QPointer snapshots so a render-driven close + // (renderer-unhealthy chain, child-exited press, etc.) that + // destroys a window or surface mid-frame can't UAF the iterator + // or the inner-loop receiver. + QList> windows; + windows.reserve(m_windows.size()); + for (MainWindow *w : m_windows) windows.append(w); + for (const QPointer &wp : windows) { + if (!wp) continue; + QList> surfaces; + const QList &surfList = wp->surfaces(); + surfaces.reserve(surfList.size()); + for (GhosttySurface *s : surfList) surfaces.append(s); + for (const QPointer &sp : surfaces) { + if (!wp || !sp) continue; + sp->renderIfDirty(); + } + } +} + +void GhosttyApp::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. The m_app check inside the lambda + // guards against the last window being destroyed (which calls + // teardown and frees m_app) between this wakeup and the queued + // tick draining. + GhosttyApp &self = instance(); + if (self.m_tickPending.exchange(true)) return; + QMetaObject::invokeMethod( + qApp, + []() { + GhosttyApp &s = instance(); + s.m_tickPending.store(false); + if (s.m_app) ghostty_app_tick(s.m_app); + }, + Qt::QueuedConnection); +} + void GhosttyApp::teardown() { + // Stop and free the timers BEFORE draining queued events: a final + // frame timeout could otherwise dispatch through the queue and + // tick the about-to-be-freed app. + if (m_frameTimer) { + m_frameTimer->stop(); + delete m_frameTimer; + m_frameTimer = nullptr; + } + if (m_quitTimer) { + delete m_quitTimer; + m_quitTimer = nullptr; + } + // Drain qApp-targeted MetaCalls posted by worker-thread libghostty // callbacks (closeAllWindows, refreshChrome, OPEN_URL, postProgress, // handleQuitTimer, NEW_WINDOW, CONFIG_CHANGE, ...) — these are the diff --git a/qt/src/app/GhosttyApp.h b/qt/src/app/GhosttyApp.h index 70b940359..296c791a5 100644 --- a/qt/src/app/GhosttyApp.h +++ b/qt/src/app/GhosttyApp.h @@ -1,10 +1,13 @@ #pragma once +#include + #include #include "ghostty.h" class MainWindow; +class QTimer; // Process-wide libghostty integration. // @@ -59,12 +62,41 @@ public: void unregisterWindow(MainWindow *w); const QList &windows() const { return m_windows; } + // The dropdown quick terminal, if it exists. There is at most one + // per process. Owned by Qt (WA_DeleteOnClose); GhosttyApp holds a + // non-owning pointer so toggleQuickTerminal can find it. + MainWindow *quickTerminal() const { return m_quickTerminal; } + void setQuickTerminal(MainWindow *w) { m_quickTerminal = w; } + + // ---- frame + quit timers ---------------------------------------- + + // Build the process-wide 60Hz frame timer if not already running. + // Idempotent. Called from MainWindow::initialize() on first window. + void ensureFrameTimer(); + + // Start / stop the natural-close quit timer per + // quit-after-last-window-closed-delay. No-op when delay is 0. + void handleQuitTimer(bool start); + + // quit-after-last-window-closed-delay (ms). 0 means no delay. + int quitDelayMs() const { return m_quitDelayMs; } + void setQuitDelayMs(int ms) { m_quitDelayMs = ms; } + + // ---- libghostty runtime callbacks (registered in ensureInitialized). + // Phase 1.2 ports onWakeup; the others still live on MainWindow. + static void onWakeup(void *ud); + private: GhosttyApp() = default; ~GhosttyApp(); GhosttyApp(const GhosttyApp &) = delete; GhosttyApp &operator=(const GhosttyApp &) = delete; + // Frame-timer body: ticks libghostty once and renders every dirty + // surface across every window. Process-wide so N windows don't + // produce N ticks per 16ms for the same shared app. + void frame(); + ghostty_app_t m_app = nullptr; ghostty_config_t m_config = nullptr; bool m_needsPremultiply = false; @@ -74,4 +106,21 @@ private: // GOTO_WINDOW cycle, and the "most recent regular window" lookup // in undoLastClose. Migrated wholesale from MainWindow::s_windows. QList m_windows; + + // The dropdown quick terminal, if any. Non-owning. + MainWindow *m_quickTerminal = nullptr; + + // Process-wide 60Hz frame timer (parented to qApp). Replaces a + // per-window timer so N windows don't fire N ticks at the same + // shared ghostty_app_t. + QTimer *m_frameTimer = nullptr; + + // Delayed quit-after-last-window-closed timer (parented to qApp). + // m_quitDelayMs is the configured delay in milliseconds; 0 disables. + QTimer *m_quitTimer = nullptr; + int m_quitDelayMs = 0; + + // Coalesces wakeup-driven ticks: at most one tick is queued at a + // time so a busy surface can't flood the event loop. + std::atomic m_tickPending{false}; }; From 7ea8c87a6387f8fd6aabb6498801ce99a7850e65 Mon Sep 17 00:00:00 2001 From: ntomsic Date: Sat, 23 May 2026 11:51:51 -0500 Subject: [PATCH 4/7] =?UTF-8?q?qt:=20phase=201.3a=20=E2=80=94=20quickTermi?= =?UTF-8?q?nal=20pointer=20+=20visibility/quickterm=20toggles?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The s_quickTerminal pointer and the toggleVisibility / toggleQuickTerminal entry points all move from MainWindow to the GhosttyApp singleton. - s_quickTerminal → GhosttyApp::m_quickTerminal. The clearing on close now lives in GhosttyApp::unregisterWindow alongside the window list removal. - MainWindow::toggleVisibility → GhosttyApp::toggleVisibility - MainWindow::toggleQuickTerminal → GhosttyApp::toggleQuickTerminal, which calls a new MainWindow::makeQuickTerminal() factory for first-use construction (it owns the m_quickTerminal=true + setupLayerShell + animateIn dance). Two callsites in MainWindow::onAction (TOGGLE_VISIBILITY, TOGGLE_QUICK_TERMINAL) and two in main.cpp (the global-shortcut handlers) updated to call the singleton. New public accessor: MainWindow::isQuickTerminal() — read-only flag so the singleton can identify the dropdown without poking the private m_quickTerminal field. (Currently unused by the singleton since unregisterWindow checks pointer equality, but kept since phase 1.4+ will need it for the undoLastClose target filter.) Build verification: not run on this Mac. Needs a docker compile before merge. Co-Authored-By: claude-flow --- qt/src/MainWindow.cpp | 43 +++++++-------------------------------- qt/src/MainWindow.h | 13 ++++++------ qt/src/app/GhosttyApp.cpp | 35 +++++++++++++++++++++++++++++++ qt/src/app/GhosttyApp.h | 9 +++++++- qt/src/main.cpp | 5 +++-- 5 files changed, 59 insertions(+), 46 deletions(-) diff --git a/qt/src/MainWindow.cpp b/qt/src/MainWindow.cpp index d6bf6e2d0..ad0bfabc4 100644 --- a/qt/src/MainWindow.cpp +++ b/qt/src/MainWindow.cpp @@ -97,7 +97,6 @@ ghostty_app_t MainWindow::s_app = nullptr; ghostty_config_t MainWindow::s_config = nullptr; bool MainWindow::s_needsPremultiply = false; int MainWindow::s_quitDelayMs = 0; -MainWindow *MainWindow::s_quickTerminal = nullptr; MainWindow::MainWindow() { setWindowTitle(QStringLiteral("Ghastty")); @@ -158,8 +157,9 @@ MainWindow::MainWindow() { } MainWindow::~MainWindow() { + // unregisterWindow also clears GhosttyApp's quick-terminal pointer + // if this was it. GhosttyApp::instance().unregisterWindow(this); - if (this == s_quickTerminal) s_quickTerminal = nullptr; // Destroy this window's surfaces (freeing their ghostty_surface_t) // before any app teardown; Qt's own child cleanup runs after this body. @@ -957,46 +957,17 @@ void MainWindow::closeAllWindows(bool thenQuit) { } } -void MainWindow::toggleVisibility() { - // If anything is showing, hide everything; otherwise reveal it all. - const QList &live = GhosttyApp::instance().windows(); - bool anyVisible = false; - for (MainWindow *w : live) - if (w->isVisible()) { - anyVisible = true; - break; - } - for (MainWindow *w : live) { - if (anyVisible) { - w->hide(); - } else { - w->show(); - w->raise(); - w->activateWindow(); - } - } -} - -void MainWindow::toggleQuickTerminal() { - if (s_quickTerminal) { - if (s_quickTerminal->isVisible()) { - s_quickTerminal->animateQuickTerminalOut(); - } else { - s_quickTerminal->animateQuickTerminalIn(); - } - return; - } - // First use: build the dedicated quick-terminal window. +MainWindow *MainWindow::makeQuickTerminal() { auto *w = new MainWindow; w->m_quickTerminal = true; w->setAttribute(Qt::WA_DeleteOnClose); if (!w->initialize()) { delete w; - return; + return nullptr; } - s_quickTerminal = w; w->setupLayerShell(); w->animateQuickTerminalIn(); + return w; } // Read quick-terminal-animation-duration (seconds) and convert to ms. @@ -2520,11 +2491,11 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, } case GHOSTTY_ACTION_TOGGLE_VISIBILITY: - post(qApp, []() { MainWindow::toggleVisibility(); }); + post(qApp, []() { GhosttyApp::instance().toggleVisibility(); }); return true; case GHOSTTY_ACTION_TOGGLE_QUICK_TERMINAL: - post(qApp, []() { MainWindow::toggleQuickTerminal(); }); + post(qApp, []() { GhosttyApp::instance().toggleQuickTerminal(); }); return true; case GHOSTTY_ACTION_TOGGLE_COMMAND_PALETTE: diff --git a/qt/src/MainWindow.h b/qt/src/MainWindow.h index 74c362423..020f3da3e 100644 --- a/qt/src/MainWindow.h +++ b/qt/src/MainWindow.h @@ -43,18 +43,18 @@ public: // tab whose surface inherits from `parent` (may be null). static MainWindow *newWindow(ghostty_surface_t parent); - // Show or hide every window at once (TOGGLE_VISIBILITY). - static void toggleVisibility(); - - // Show/hide the dropdown quick terminal, creating it on first use - // (TOGGLE_QUICK_TERMINAL). There is at most one per process. - static void toggleQuickTerminal(); + // Build the process's single quick-terminal MainWindow on demand: + // a layer-shell dropdown anchored to a screen edge, faded in + // immediately. Called from GhosttyApp::toggleQuickTerminal on first + // use. Returns nullptr on init failure. + static MainWindow *makeQuickTerminal(); // Quick-terminal slide/fade animation per quick-terminal-animation- // duration. Implemented as a windowOpacity fade because Qt's layer- // shell doesn't expose a usable position-based slide API. void animateQuickTerminalIn(); void animateQuickTerminalOut(); + bool isQuickTerminal() const { return m_quickTerminal; } // Open a new tab. `parent` (may be null) is the surface whose working // directory etc. the new surface should inherit. @@ -289,7 +289,6 @@ private: // Mirror of GhosttyApp::quitDelayMs; phase 1.3 retires it when the // remaining call site (closeAllWindows) moves to the singleton. static int s_quitDelayMs; // 0 = no delay configured - static MainWindow *s_quickTerminal; // the one quick terminal, if any // Snapshot of a closed tab or window for undo/redo. `pageTitles` // holds each tab's last-known title (window snapshots have N tabs; diff --git a/qt/src/app/GhosttyApp.cpp b/qt/src/app/GhosttyApp.cpp index 985bbe0b7..87dbfe816 100644 --- a/qt/src/app/GhosttyApp.cpp +++ b/qt/src/app/GhosttyApp.cpp @@ -108,6 +108,41 @@ void GhosttyApp::registerWindow(MainWindow *w) { void GhosttyApp::unregisterWindow(MainWindow *w) { m_windows.removeOne(w); + if (m_quickTerminal == w) m_quickTerminal = nullptr; +} + +void GhosttyApp::toggleVisibility() { + // If anything is showing, hide everything; otherwise reveal it all. + bool anyVisible = false; + for (MainWindow *w : m_windows) + if (w->isVisible()) { + anyVisible = true; + break; + } + for (MainWindow *w : m_windows) { + if (anyVisible) { + w->hide(); + } else { + w->show(); + w->raise(); + w->activateWindow(); + } + } +} + +void GhosttyApp::toggleQuickTerminal() { + if (m_quickTerminal) { + if (m_quickTerminal->isVisible()) + m_quickTerminal->animateQuickTerminalOut(); + else + m_quickTerminal->animateQuickTerminalIn(); + return; + } + // First use: build the dedicated quick-terminal window. It registers + // itself via the standard registerWindow path; we additionally + // remember it as the singleton dropdown so a second toggle-call + // animates rather than building another window. + m_quickTerminal = MainWindow::makeQuickTerminal(); } void GhosttyApp::ensureFrameTimer() { diff --git a/qt/src/app/GhosttyApp.h b/qt/src/app/GhosttyApp.h index 296c791a5..d9db0d54d 100644 --- a/qt/src/app/GhosttyApp.h +++ b/qt/src/app/GhosttyApp.h @@ -66,7 +66,14 @@ public: // per process. Owned by Qt (WA_DeleteOnClose); GhosttyApp holds a // non-owning pointer so toggleQuickTerminal can find it. MainWindow *quickTerminal() const { return m_quickTerminal; } - void setQuickTerminal(MainWindow *w) { m_quickTerminal = w; } + + // App-scoped show/hide of every regular window. Replaces + // MainWindow::toggleVisibility(). + void toggleVisibility(); + + // Show/hide the dropdown, creating it on first use. Replaces + // MainWindow::toggleQuickTerminal(). + void toggleQuickTerminal(); // ---- frame + quit timers ---------------------------------------- diff --git a/qt/src/main.cpp b/qt/src/main.cpp index b7917524f..9bdb8d2c0 100644 --- a/qt/src/main.cpp +++ b/qt/src/main.cpp @@ -5,6 +5,7 @@ #include #include +#include "app/GhosttyApp.h" #include "GlobalShortcuts.h" #include "MainWindow.h" #include "ghostty.h" @@ -121,9 +122,9 @@ int main(int argc, char **argv) { QObject::connect(&globalShortcuts, &GlobalShortcuts::activated, [](const QString &id) { if (id == QLatin1String("toggle-quick-terminal")) - MainWindow::toggleQuickTerminal(); + GhosttyApp::instance().toggleQuickTerminal(); else if (id == QLatin1String("toggle-visibility")) - MainWindow::toggleVisibility(); + GhosttyApp::instance().toggleVisibility(); }); return app.exec(); From f669aa0d7acc177f3f73e5e341d52fe6a7fb97a0 Mon Sep 17 00:00:00 2001 From: ntomsic Date: Sat, 23 May 2026 11:55:15 -0500 Subject: [PATCH 5/7] =?UTF-8?q?qt:=20phase=201.3b=20=E2=80=94=20move=20cli?= =?UTF-8?q?pboard=20+=20closeSurface=20callbacks=20to=20GhosttyApp?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Final piece of phase 1: the four remaining libghostty runtime callbacks (onReadClipboard, onConfirmReadClipboard, onWriteClipboard, onCloseSurface) plus the surfaceAlive predicate that gates them all move from MainWindow to GhosttyApp. - All four callback bodies move verbatim. The only edits are `surfaceAlive(...)` → `instance().surfaceAlive(...)` (the predicate is now a member of the singleton). - surfaceAlive itself iterates m_windows on the singleton, calling a new public MainWindow::ownsSurface(GhosttySurface*) accessor instead of directly poking the private m_surfaces field. - ensureInitialized now registers GhosttyApp::onReadClipboard et al. with libghostty's runtime config bundle. After this commit, MainWindow::onAction is the only libghostty runtime callback still living on MainWindow; phase 2 retires it for an ActionDispatcher. Phase 1 closeout summary: - Phase 1.0: libghostty handles + bring-up/teardown - Phase 1.1: window registry (s_windows) - Phase 1.2: frame timer + quit timer + onWakeup + tickPending - Phase 1.3a: quickTerminal pointer + visibility / quick-term toggles - Phase 1.3b: read/confirm/write clipboard + closeSurface The s_app / s_config / s_needsPremultiply / s_quitDelayMs mirrors on MainWindow are deliberately retained — they're read by ~50 call sites across MainWindow.cpp and retiring them is a focused phase 1.4 follow-up that can land independently. Build verification: not run on this Mac. Needs a docker compile before merge. Co-Authored-By: claude-flow --- qt/src/MainWindow.cpp | 114 ++------------------------------- qt/src/MainWindow.h | 29 ++++----- qt/src/app/GhosttyApp.cpp | 128 +++++++++++++++++++++++++++++++++++--- qt/src/app/GhosttyApp.h | 13 +++- 4 files changed, 151 insertions(+), 133 deletions(-) diff --git a/qt/src/MainWindow.cpp b/qt/src/MainWindow.cpp index ad0bfabc4..1d1967071 100644 --- a/qt/src/MainWindow.cpp +++ b/qt/src/MainWindow.cpp @@ -742,7 +742,7 @@ void MainWindow::showTabContextMenu(int index, const QPoint &globalPos) { QAction *aRename = menu.addAction(QStringLiteral("Rename Tab…")); QAction *chosen = menu.exec(globalPos); - if (!chosen || !surfaceAlive(src)) return; + if (!chosen || !GhosttyApp::instance().surfaceAlive(src)) return; if (chosen == aClose) closeTabsByMode(src, GHOSTTY_ACTION_CLOSE_TAB_MODE_THIS); else if (chosen == aOther) @@ -1952,13 +1952,8 @@ void MainWindow::toggleSplitZoom(GhosttySurface *surface) { } // --- libghostty runtime callbacks ------------------------------------ - -bool MainWindow::surfaceAlive(GhosttySurface *s) { - if (!s) return false; - for (MainWindow *w : GhosttyApp::instance().windows()) - if (w->m_surfaces.contains(s)) return true; - return false; -} +// All five non-action callbacks (onWakeup + the clipboard / close +// quartet) live on GhosttyApp now. onAction stays here until phase 2. // Map a libghostty mouse shape to the nearest Qt cursor. static Qt::CursorShape mouseShapeToCursor(ghostty_action_mouse_shape_e s) { @@ -2743,104 +2738,5 @@ 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 by libghostty when a - // surface needs clipboard contents (paste). This runs on the GUI - // thread by construction: every libghostty entry point that - // surfaces a paste lives behind ghostty_app_tick, which the - // process-wide frame timer drives — and that timer is on the GUI - // thread. QClipboard is GUI-thread-only, so reading directly here - // is safe; surfaceAlive still validates the pointer in case a - // surface is mid-destruction on this same thread. - auto *surface = static_cast(ud); - if (!surfaceAlive(surface) || !surface->surface()) return false; - - const QClipboard::Mode mode = loc == GHOSTTY_CLIPBOARD_SELECTION - ? QClipboard::Selection - : QClipboard::Clipboard; - const QByteArray text = QGuiApplication::clipboard()->text(mode).toUtf8(); - ghostty_surface_complete_clipboard_request(surface->surface(), - text.constData(), state, true); - return true; -} - -void MainWindow::onConfirmReadClipboard(void *ud, const char *str, void *state, - ghostty_clipboard_request_e) { - // libghostty asks for confirmation when a paste looks unsafe. The - // dialog MUST be deferred: this callback runs inside libghostty, and a - // modal dialog here spins a nested event loop that re-enters libghostty - // 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 (!surfaceAlive(surface) || !surface->surface()) return; - - QPointer sp(surface); - const QByteArray content(str); - QMetaObject::invokeMethod( - surface->owner(), - [sp, content, state]() { - if (!sp || !sp->surface()) return; - QString preview = QString::fromUtf8(content); - // 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("…"); - } - // Destructive Paste / Cancel buttons, default Cancel — - // mirrors the close-confirmation styling. - QMessageBox box(sp->owner()); - box.setIcon(QMessageBox::Warning); - box.setWindowTitle(QStringLiteral("Confirm Paste")); - box.setText(QStringLiteral("The text being pasted may be unsafe.")); - box.setInformativeText(preview); - QPushButton *paste = box.addButton(QStringLiteral("Paste"), - QMessageBox::DestructiveRole); - QPushButton *cancel = box.addButton(QStringLiteral("Cancel"), - QMessageBox::RejectRole); - box.setDefaultButton(cancel); - box.exec(); - ghostty_surface_complete_clipboard_request( - sp->surface(), content.constData(), state, - box.clickedButton() == paste); - }, - Qt::QueuedConnection); -} - -void MainWindow::onWriteClipboard(void *ud, ghostty_clipboard_e loc, - const ghostty_clipboard_content_s *content, - size_t n, bool) { - if (n == 0 || !content[0].data) return; - auto *surface = static_cast(ud); - 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( - qApp, - [text, mode]() { QGuiApplication::clipboard()->setText(text, mode); }, - Qt::QueuedConnection); -} - -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 (!surfaceAlive(surface)) return; - MainWindow *self = surface->owner(); - QPointer selfp(self); - QPointer sp(surface); - QMetaObject::invokeMethod( - self, - [selfp, sp]() { - if (!selfp || !sp) return; - if (selfp->confirmCloseSurfaces({sp})) selfp->removeSurface(sp); - }, - Qt::QueuedConnection); -} +// onReadClipboard / onConfirmReadClipboard / onWriteClipboard / +// onCloseSurface all moved to GhosttyApp in phase 1.3b. diff --git a/qt/src/MainWindow.h b/qt/src/MainWindow.h index 020f3da3e..5405aaf3e 100644 --- a/qt/src/MainWindow.h +++ b/qt/src/MainWindow.h @@ -116,6 +116,13 @@ public: // private m_surfaces member. const QList &surfaces() const { return m_surfaces; } + // Whether `s` is one of this window's surfaces. Used by + // GhosttyApp::surfaceAlive to validate libghostty userdata pointers + // against a destruction race on worker-thread callbacks. + bool ownsSurface(GhosttySurface *s) const { + return m_surfaces.contains(s); + } + protected: bool event(QEvent *) override; void showEvent(QShowEvent *) override; @@ -232,24 +239,14 @@ private: // / back into the splitter tree. void toggleSplitZoom(GhosttySurface *surface); - // Runtime callbacks dispatched by libghostty. action is app-level - // (routed via the target surface or the GhosttyApp window - // registry); clipboard/close carry the surface userdata. wakeup - // moved to GhosttyApp::onWakeup in phase 1.2. + // The libghostty action callback. Stays here because its switch + // body still needs private MainWindow access; phase 2 retires it + // for an ActionDispatcher. The other five runtime callbacks + // (onWakeup + clipboard quartet) live on GhosttyApp. static bool onAction(ghostty_app_t, ghostty_target_s, ghostty_action_s); - static bool onReadClipboard(void *ud, ghostty_clipboard_e, void *state); - static void onConfirmReadClipboard(void *ud, const char *, void *state, - ghostty_clipboard_request_e); - static void onWriteClipboard(void *ud, ghostty_clipboard_e, - const ghostty_clipboard_content_s *, size_t, - 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); + // surfaceAlive moved to GhosttyApp::surfaceAlive (it iterates the + // live window registry, which is owned by the singleton). TabWidget *m_tabs = nullptr; QList m_surfaces; // every live surface in this window diff --git a/qt/src/app/GhosttyApp.cpp b/qt/src/app/GhosttyApp.cpp index 87dbfe816..a30176413 100644 --- a/qt/src/app/GhosttyApp.cpp +++ b/qt/src/app/GhosttyApp.cpp @@ -4,11 +4,16 @@ #include #include +#include #include #include #include #include +#include +#include +#include #include +#include #include #include @@ -73,15 +78,14 @@ bool GhosttyApp::ensureInitialized() { // surface, and app-level actions via the GhosttyApp window registry. rt.userdata = nullptr; rt.supports_selection_clipboard = true; - // onWakeup migrated in phase 1.2; the rest still live on - // MainWindow and migrate alongside the action dispatcher in - // phase 1.3+. + // onAction stays on MainWindow until phase 2 introduces the + // ActionDispatcher; the rest are owned by GhosttyApp. rt.wakeup_cb = GhosttyApp::onWakeup; rt.action_cb = MainWindow::onAction; - rt.read_clipboard_cb = MainWindow::onReadClipboard; - rt.confirm_read_clipboard_cb = MainWindow::onConfirmReadClipboard; - rt.write_clipboard_cb = MainWindow::onWriteClipboard; - rt.close_surface_cb = MainWindow::onCloseSurface; + rt.read_clipboard_cb = GhosttyApp::onReadClipboard; + rt.confirm_read_clipboard_cb = GhosttyApp::onConfirmReadClipboard; + rt.write_clipboard_cb = GhosttyApp::onWriteClipboard; + rt.close_surface_cb = GhosttyApp::onCloseSurface; m_app = ghostty_app_new(&rt, m_config); if (!m_app) { @@ -260,3 +264,113 @@ void GhosttyApp::teardown() { } m_needsPremultiply = false; } + +bool GhosttyApp::surfaceAlive(GhosttySurface *s) const { + if (!s) return false; + for (MainWindow *w : m_windows) + if (w->ownsSurface(s)) return true; + return false; +} + +bool GhosttyApp::onReadClipboard(void *ud, ghostty_clipboard_e loc, + void *state) { + // surface userdata. Called synchronously by libghostty when a + // surface needs clipboard contents (paste). This runs on the GUI + // thread by construction: every libghostty entry point that + // surfaces a paste lives behind ghostty_app_tick, which the + // process-wide frame timer drives — and that timer is on the GUI + // thread. QClipboard is GUI-thread-only, so reading directly here + // is safe; surfaceAlive still validates the pointer in case a + // surface is mid-destruction on this same thread. + auto *surface = static_cast(ud); + if (!instance().surfaceAlive(surface) || !surface->surface()) return false; + + const QClipboard::Mode mode = loc == GHOSTTY_CLIPBOARD_SELECTION + ? QClipboard::Selection + : QClipboard::Clipboard; + const QByteArray text = QGuiApplication::clipboard()->text(mode).toUtf8(); + ghostty_surface_complete_clipboard_request(surface->surface(), + text.constData(), state, true); + return true; +} + +void GhosttyApp::onConfirmReadClipboard(void *ud, const char *str, + void *state, + ghostty_clipboard_request_e) { + // libghostty asks for confirmation when a paste looks unsafe. The + // dialog MUST be deferred: this callback runs inside libghostty, + // and a modal dialog here spins a nested event loop that re-enters + // libghostty 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 (!instance().surfaceAlive(surface) || !surface->surface()) return; + + QPointer sp(surface); + const QByteArray content(str); + QMetaObject::invokeMethod( + surface->owner(), + [sp, content, state]() { + if (!sp || !sp->surface()) return; + QString preview = QString::fromUtf8(content); + // 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("…"); + } + // Destructive Paste / Cancel buttons, default Cancel — + // mirrors the close-confirmation styling. + QMessageBox box(sp->owner()); + box.setIcon(QMessageBox::Warning); + box.setWindowTitle(QStringLiteral("Confirm Paste")); + box.setText(QStringLiteral("The text being pasted may be unsafe.")); + box.setInformativeText(preview); + QPushButton *paste = box.addButton(QStringLiteral("Paste"), + QMessageBox::DestructiveRole); + QPushButton *cancel = box.addButton(QStringLiteral("Cancel"), + QMessageBox::RejectRole); + box.setDefaultButton(cancel); + box.exec(); + ghostty_surface_complete_clipboard_request( + sp->surface(), content.constData(), state, + box.clickedButton() == paste); + }, + Qt::QueuedConnection); +} + +void GhosttyApp::onWriteClipboard(void *ud, ghostty_clipboard_e loc, + const ghostty_clipboard_content_s *content, + size_t n, bool) { + if (n == 0 || !content[0].data) return; + auto *surface = static_cast(ud); + if (!instance().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( + qApp, + [text, mode]() { QGuiApplication::clipboard()->setText(text, mode); }, + Qt::QueuedConnection); +} + +void GhosttyApp::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 (!instance().surfaceAlive(surface)) return; + MainWindow *self = surface->owner(); + QPointer selfp(self); + QPointer sp(surface); + QMetaObject::invokeMethod( + self, + [selfp, sp]() { + if (!selfp || !sp) return; + if (selfp->confirmCloseSurfaces({sp})) selfp->removeSurface(sp); + }, + Qt::QueuedConnection); +} diff --git a/qt/src/app/GhosttyApp.h b/qt/src/app/GhosttyApp.h index d9db0d54d..23f8cc696 100644 --- a/qt/src/app/GhosttyApp.h +++ b/qt/src/app/GhosttyApp.h @@ -90,8 +90,19 @@ public: void setQuitDelayMs(int ms) { m_quitDelayMs = ms; } // ---- libghostty runtime callbacks (registered in ensureInitialized). - // Phase 1.2 ports onWakeup; the others still live on MainWindow. static void onWakeup(void *ud); + static bool onReadClipboard(void *ud, ghostty_clipboard_e, void *state); + static void onConfirmReadClipboard(void *ud, const char *, void *state, + ghostty_clipboard_request_e); + static void onWriteClipboard(void *ud, ghostty_clipboard_e, + const ghostty_clipboard_content_s *, size_t, + bool); + static void onCloseSurface(void *ud, bool process_active); + + // True if the surface pointer (a libghostty userdata) is still owned + // by a live MainWindow. Worker-thread callbacks use this to gate + // against a destruction race. + bool surfaceAlive(GhosttySurface *s) const; private: GhosttyApp() = default; From 687d4b05b9695e9aa11f41d007b53671b3eacc73 Mon Sep 17 00:00:00 2001 From: ntomsic Date: Sat, 23 May 2026 12:21:10 -0500 Subject: [PATCH 6/7] =?UTF-8?q?qt:=20phase=201.4=20=E2=80=94=20retire=20Ma?= =?UTF-8?q?inWindow's=20mirror=20statics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The s_app / s_config / s_needsPremultiply / s_quitDelayMs mirror caches that phase 1.0-1.3 kept on MainWindow are gone. Every read goes through GhosttyApp::instance().{app,config,needsPremultiply, quitDelayMs}() now. - 28 sed-rewrites of the form `s_config` → `GhosttyApp::instance(). config()` (and friends) across the remaining MainWindow.cpp call sites. - configString and configBool cache the singleton's config() in a local so the hot path doesn't double-call. closeAllWindows does the same with app(). - MainWindow::config() and MainWindow::needsPremultiply() are now out-of-line forwarders to the singleton; they keep the same signature so external callers (GhosttySurface, InspectorWindow) don't need to take a dependency on app/GhosttyApp.h. - Drop the four static declarations from MainWindow.h, the four definitions from MainWindow.cpp, the priming block in initialize(), the mirror sync in reloadConfigGlobal, and the nullify-on-teardown in ~MainWindow. - GhosttyApp.h: forward-declare GhosttySurface — needed by surfaceAlive(GhosttySurface *). Caught by the docker build (gcc was implicitly typing it as `int *`). Build: docker build --target qt SUCCEEDED. Phases 1.0 through 1.4 compile cleanly on Fedora 42 / Qt 6.10 with no warnings. Co-Authored-By: claude-flow --- qt/src/MainWindow.cpp | 91 ++++++++++++++++++----------------------- qt/src/MainWindow.h | 27 +++++------- qt/src/app/GhosttyApp.h | 1 + 3 files changed, 50 insertions(+), 69 deletions(-) diff --git a/qt/src/MainWindow.cpp b/qt/src/MainWindow.cpp index 1d1967071..40d325e73 100644 --- a/qt/src/MainWindow.cpp +++ b/qt/src/MainWindow.cpp @@ -87,16 +87,17 @@ static QIcon bellAttentionIcon() { return cached; } -// Process-shared libghostty state — see MainWindow.h. -// -// s_app / s_config / s_needsPremultiply mirror GhosttyApp::instance() -// so the rest of this file (and GhosttySurface) keep their existing -// access patterns. Phase 1.2+ will retire them as call sites move -// over to the singleton directly. -ghostty_app_t MainWindow::s_app = nullptr; -ghostty_config_t MainWindow::s_config = nullptr; -bool MainWindow::s_needsPremultiply = false; -int MainWindow::s_quitDelayMs = 0; +// All process-shared libghostty state lives on GhosttyApp::instance(). +// MainWindow's config() and needsPremultiply() forward there so +// external consumers (GhosttySurface, InspectorWindow) don't have to +// take a dependency on app/GhosttyApp.h. +ghostty_config_t MainWindow::config() const { + return GhosttyApp::instance().config(); +} + +bool MainWindow::needsPremultiply() const { + return GhosttyApp::instance().needsPremultiply(); +} MainWindow::MainWindow() { setWindowTitle(QStringLiteral("Ghastty")); @@ -183,11 +184,8 @@ MainWindow::~MainWindow() { // GhosttyApp::teardown stops + frees the frame and quit timers, // drains qApp-targeted MetaCalls (so worker callbacks can't touch // a freed app), and ghostty_app_frees + ghostty_config_frees the - // live handles. The local s_app / s_config mirrors are still in - // use across this file; null them so they match the singleton. + // live handles. GhosttyApp::instance().teardown(); - s_app = nullptr; - s_config = nullptr; } } @@ -383,17 +381,8 @@ static void openUrlByKind(const QString &url, } bool MainWindow::initialize() { - // First-call: build libghostty app + config via the singleton. The - // singleton holds m_app / m_config / m_needsPremultiply; the - // MainWindow s_app / s_config / s_needsPremultiply statics are now - // forwarders that read it. + // First-call: build libghostty app + config via the singleton. if (!GhosttyApp::instance().ensureInitialized()) return false; - // Mirror the singleton's pointers into the legacy statics so the - // existing call sites (the rest of MainWindow.cpp + GhosttySurface) - // keep working unchanged. Phases 1.1+ swap them out. - s_app = GhosttyApp::instance().app(); - s_config = GhosttyApp::instance().config(); - s_needsPremultiply = GhosttyApp::instance().needsPremultiply(); GhosttyApp::instance().registerWindow(this); @@ -416,7 +405,6 @@ bool MainWindow::initialize() { ? static_cast(std::min(delayMs, uint64_t(INT_MAX))) : 0; GhosttyApp::instance().setQuitDelayMs(delayMsInt); - s_quitDelayMs = delayMsInt; // mirror; phase 1.3 retires the static QApplication::setQuitOnLastWindowClosed(quitAfter && delayMsInt == 0); } @@ -479,8 +467,8 @@ MainWindow *MainWindow::newWindow(ghostty_surface_t parent) { // NSWindow.cascadeTopLeft. Wayland compositors typically ignore // window placement requests; this is a hint at most. int16_t posX = 0, posY = 0; - const bool haveX = configGet(s_config, &posX, "window-position-x"); - const bool haveY = configGet(s_config, &posY, "window-position-y"); + const bool haveX = configGet(GhosttyApp::instance().config(), &posX, "window-position-x"); + const bool haveY = configGet(GhosttyApp::instance().config(), &posY, "window-position-y"); if (haveX && haveY) { w->move(posX, posY); } else { @@ -502,7 +490,7 @@ MainWindow *MainWindow::newWindow(ghostty_surface_t parent) { if (!s_initialWindowConsumed) { s_initialWindowConsumed = true; bool initialWindow = true; - configGet(s_config, &initialWindow, "initial-window"); + configGet(GhosttyApp::instance().config(), &initialWindow, "initial-window"); wantsShow = initialWindow; } if (wantsShow) w->show(); @@ -543,7 +531,7 @@ void MainWindow::createFirstTab() { } GhosttySurface *MainWindow::newTab(ghostty_surface_t parent) { - auto *surface = new GhosttySurface(s_app, this, parent); + auto *surface = new GhosttySurface(GhosttyApp::instance().app(), this, parent); m_surfaces.append(surface); // The tab page hosts the tab's split tree (initially one surface). @@ -576,7 +564,7 @@ GhosttySurface *MainWindow::splitSurface( const bool newAfter = dir == GHOSTTY_SPLIT_DIRECTION_RIGHT || dir == GHOSTTY_SPLIT_DIRECTION_DOWN; - auto *surface = new GhosttySurface(s_app, this, target->surface()); + auto *surface = new GhosttySurface(GhosttyApp::instance().app(), this, target->surface()); auto *splitter = new QSplitter(horizontal ? Qt::Horizontal : Qt::Vertical); splitter->setChildrenCollapsible(false); @@ -909,7 +897,8 @@ void MainWindow::closeAllWindows(bool thenQuit) { // + Cancel default — same style as confirmCloseSurfaces. Title / // verb track whether this is a Quit (process ends) or a // Close All Windows (process may stay alive). - if (s_app && ghostty_app_needs_confirm_quit(s_app)) { + ghostty_app_t app = GhosttyApp::instance().app(); + if (app && ghostty_app_needs_confirm_quit(app)) { const QString title = thenQuit ? QStringLiteral("Quit") : QStringLiteral("Close All Windows"); const QString verb = thenQuit ? QStringLiteral("Quit") @@ -986,7 +975,7 @@ void MainWindow::animateQuickTerminalIn() { show(); raise(); activateWindow(); - const int ms = quickTerminalAnimationMs(s_config); + const int ms = quickTerminalAnimationMs(GhosttyApp::instance().config()); if (ms <= 0) { setWindowOpacity(1.0); return; @@ -1004,7 +993,7 @@ void MainWindow::animateQuickTerminalIn() { } void MainWindow::animateQuickTerminalOut() { - const int ms = quickTerminalAnimationMs(s_config); + const int ms = quickTerminalAnimationMs(GhosttyApp::instance().config()); if (ms <= 0) { hide(); return; @@ -1086,7 +1075,7 @@ void MainWindow::setupLayerShell() { // quick-terminal-size: primary is the edge-perpendicular extent. ghostty_config_quick_terminal_size_s qsz = {}; - configGet(s_config, &qsz, "quick-terminal-size"); + configGet(GhosttyApp::instance().config(), &qsz, "quick-terminal-size"); const auto toPx = [](const ghostty_quick_terminal_size_s &s, int dim, int fallback) -> int { switch (s.tag) { @@ -1217,7 +1206,7 @@ void MainWindow::gotoSplit(GhosttySurface *from, // `bool`==1) under-sized the buffer and corrupted adjacent stack; // read into c_uint and mask the bits. unsigned int pzBits = 0; - configGet(s_config, &pzBits, "split-preserve-zoom"); + configGet(GhosttyApp::instance().config(), &pzBits, "split-preserve-zoom"); const bool preserveZoom = (pzBits & 0x1) != 0 && m_zoomed == from; const auto centerOf = [](GhosttySurface *s) { @@ -1381,7 +1370,7 @@ void MainWindow::ringBell(GhosttySurface *surface) { // If config-get succeeds with features=0, the user explicitly opted // out of every bell feature and we honor that. unsigned int features = 0; - if (!configGet(s_config, &features, "bell-features")) { + if (!configGet(GhosttyApp::instance().config(), &features, "bell-features")) { features = BellAttention; } if (features & BellAttention) QApplication::alert(this); @@ -1438,7 +1427,7 @@ void MainWindow::playBellAudio() { m_bellPlayer->play(); } -// Refresh every window's chrome from the current s_config: tab-bar +// Refresh every window's chrome from the current GhosttyApp config: tab-bar // policy, colour scheme, blur — plus window-level state that // previously only applied at startup (window-decoration, fullscreen, // maximize) and the quit-after-last-window-closed delay. @@ -1446,9 +1435,9 @@ void MainWindow::refreshChrome() { // Refresh app-scoped state. quit-after-last-window-closed[-delay] // can change the delay or the quitOnLastWindowClosed strategy at // runtime; mirrors the calculation in initialize(). - if (s_config) { + if (GhosttyApp::instance().config()) { bool quitAfter = true; - configGet(s_config, &quitAfter, "quit-after-last-window-closed"); + configGet(GhosttyApp::instance().config(), &quitAfter, "quit-after-last-window-closed"); // Same Duration-decode workaround as initialize(). const uint64_t delayNs = parseDurationNs( configValue(QStringLiteral("quit-after-last-window-closed-delay")), 0); @@ -1457,7 +1446,6 @@ void MainWindow::refreshChrome() { ? static_cast(std::min(delayMs, uint64_t(INT_MAX))) : 0; GhosttyApp::instance().setQuitDelayMs(delayMsInt); - s_quitDelayMs = delayMsInt; // mirror; phase 1.3 retires the static QApplication::setQuitOnLastWindowClosed(quitAfter && delayMsInt == 0); } @@ -1510,7 +1498,7 @@ void MainWindow::refreshChrome() { void MainWindow::reloadConfig() { reloadConfigGlobal(); } void MainWindow::reloadConfigGlobal() { - if (!s_app) return; + if (!GhosttyApp::instance().app()) return; // Re-read the config from disk in the same order as initialize(). ghostty_config_t config = ghostty_config_new(); ghostty_config_load_default_files(config); @@ -1521,14 +1509,12 @@ void MainWindow::reloadConfigGlobal() { // Push to libghostty. App.updateConfig propagates the config to every // surface and fires CONFIG_CHANGE back at us — which only refreshes // chrome, never re-pushes, so this does not loop. - ghostty_app_update_config(s_app, config); + ghostty_app_update_config(GhosttyApp::instance().app(), config); // Hand the new config to the singleton, which frees the previous one // (in the right order — libghostty borrows the previous until update // completes) and recomputes needsPremultiply. GhosttyApp::instance().replaceConfig(config); - s_config = GhosttyApp::instance().config(); - s_needsPremultiply = GhosttyApp::instance().needsPremultiply(); refreshChrome(); @@ -1542,7 +1528,7 @@ void MainWindow::reloadConfigGlobal() { // forward compatibility — Qt doesn't currently post a copy // toast, but a future one will pick up the same gate. unsigned int notifBits = 0; - const bool notifOk = configGet(s_config, ¬ifBits, "app-notifications"); + const bool notifOk = configGet(GhosttyApp::instance().config(), ¬ifBits, "app-notifications"); // configGet failure → defaults (both bits set) so the feature // still works as documented. if (!notifOk) notifBits = 0x3; @@ -1553,16 +1539,17 @@ void MainWindow::reloadConfigGlobal() { } QString MainWindow::configString(const char *key) const { + ghostty_config_t cfg = GhosttyApp::instance().config(); const char *value = nullptr; - if (!s_config || - !ghostty_config_get(s_config, &value, key, qstrlen(key)) || !value) + if (!cfg || !ghostty_config_get(cfg, &value, key, qstrlen(key)) || !value) return {}; return QString::fromUtf8(value); } bool MainWindow::configBool(const char *key, bool fallback) const { bool value = fallback; // ghostty_config_get leaves it untouched if absent - if (s_config) ghostty_config_get(s_config, &value, key, qstrlen(key)); + if (ghostty_config_t cfg = GhosttyApp::instance().config()) + ghostty_config_get(cfg, &value, key, qstrlen(key)); return value; } @@ -1887,7 +1874,7 @@ void MainWindow::applyWindowConfig() { scheme = Qt::ColorScheme::Light; } else if (theme == QLatin1String("ghostty")) { ghostty_config_color_s bg{}; - if (configGet(s_config, &bg, "background")) { + if (configGet(GhosttyApp::instance().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; } @@ -1905,7 +1892,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; - configGet(s_config, &blur, "background-blur"); + configGet(GhosttyApp::instance().config(), &blur, "background-blur"); applyWindowBlur(this, blur > 0); } @@ -2238,7 +2225,7 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, // abnormal threshold (default 250ms). Banner = "the process // died unexpectedly," not "the process exited." uint32_t threshold = 250; - configGet(s_config, &threshold, "abnormal-command-exit-runtime"); + configGet(GhosttyApp::instance().config(), &threshold, "abnormal-command-exit-runtime"); if (ce.runtime_ms < threshold) return true; const int code = static_cast(ce.exit_code); post(src, [srcp, code]() { @@ -2397,7 +2384,7 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, // "configGet failed; nothing to do" semantics. unsigned int actBits = 0; const bool actOk = - configGet(s_config, &actBits, "notify-on-command-finish-action"); + configGet(GhosttyApp::instance().config(), &actBits, "notify-on-command-finish-action"); // configGet failure → fall back to the documented defaults // (bell=true, notify=false) so the feature still works. if (!actOk) actBits = 0x1; diff --git a/qt/src/MainWindow.h b/qt/src/MainWindow.h index 5405aaf3e..1243a96d7 100644 --- a/qt/src/MainWindow.h +++ b/qt/src/MainWindow.h @@ -1,7 +1,5 @@ #pragma once -#include - #include #include #include @@ -71,8 +69,11 @@ public: // Update the tab label and window title for `surface`. void setSurfaceTitle(GhosttySurface *surface, const QString &title); - // The live libghostty config (for keybind lookups, etc.). - ghostty_config_t config() const { return s_config; } + // The live libghostty config (for keybind lookups, etc.). Forwards + // to GhosttyApp::instance().config(); kept on MainWindow as a thin + // shim so external callers (GhosttySurface, InspectorWindow) don't + // need to take a dependency on app/GhosttyApp.h. + ghostty_config_t config() const; // UNDO / REDO close-tab/window. The libghostty actions carry no // payload — the apprt is responsible for tracking what was closed @@ -105,7 +106,8 @@ public: // Whether a custom shader is configured. With one, libghostty's final // framebuffer is non-premultiplied and surfaces must premultiply it // before Qt composites (see GhosttySurface::premultiplyFramebuffer). - bool needsPremultiply() const { return s_needsPremultiply; } + // Forwards to GhosttyApp::instance().needsPremultiply(). + bool needsPremultiply() const; // Whether `focus-follows-mouse` is enabled — a GhosttySurface grabs // focus when the pointer enters it. @@ -274,18 +276,9 @@ private: // of `background-opacity`). bool m_opacityForcedOpaque = false; - // Process-shared libghostty state: one app and config drive every - // window. Created by the first initialize(), freed with the last - // window. The live window list lives on GhosttyApp; the s_app / - // s_config / s_needsPremultiply statics here are mirror caches kept - // in sync with GhosttyApp::instance() to limit phase-1 callsite - // churn — they retire as call sites move to the singleton. - static ghostty_app_t s_app; - static ghostty_config_t s_config; - static bool s_needsPremultiply; // a custom shader is configured - // Mirror of GhosttyApp::quitDelayMs; phase 1.3 retires it when the - // remaining call site (closeAllWindows) moves to the singleton. - static int s_quitDelayMs; // 0 = no delay configured + // The libghostty app + config + derived state all live on + // GhosttyApp::instance(). MainWindow's config() / needsPremultiply() + // accessors forward to it. // Snapshot of a closed tab or window for undo/redo. `pageTitles` // holds each tab's last-known title (window snapshots have N tabs; diff --git a/qt/src/app/GhosttyApp.h b/qt/src/app/GhosttyApp.h index 23f8cc696..e5078aaed 100644 --- a/qt/src/app/GhosttyApp.h +++ b/qt/src/app/GhosttyApp.h @@ -6,6 +6,7 @@ #include "ghostty.h" +class GhosttySurface; class MainWindow; class QTimer; From 7502e1842981072ad820282ddc62fe33c11a4005 Mon Sep 17 00:00:00 2001 From: ntomsic Date: Sat, 23 May 2026 12:47:54 -0500 Subject: [PATCH 7/7] =?UTF-8?q?fix(audit):=20pass=201=20=E2=80=94=20UB=20a?= =?UTF-8?q?t=20exit=20+=20stale=20docs=20+=20cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two HIGH findings: 1. ~GhosttyApp called teardown() at process exit. The static-local singleton's destructor runs AFTER main() returns and the stack-allocated QApplication is destroyed, so qApp is null and the Qt event system is gone. Inside teardown(), QCoreApplication::sendPostedEvents(qApp, MetaCall) was being called on a dangling qApp — UB on every shutdown. Empty the destructor body: by the time normal shutdown reaches it, every MainWindow has already run teardown() through ~MainWindow (while qApp is still alive); a second call here is redundant and unsafe. The only path where instance() is ever called also runs teardown() through that route, so no leaks. 2. replaceConfig's comment claimed "the new must be installed and the old freed in this order" but the code does the opposite. The actual ordering invariant is on the caller (must call ghostty_app_update_config first); reword the comment as a PRECONDITION block describing what the caller must have done. The eight LOW/NIT findings are all stale-doc fixes from the phase-1.x progression — class-level comments that described phase-1.0 scope as if no further migration had happened, plus a few sed-pass artifacts (`live_`, `live_pt`, repeated `GhosttyApp::instance().config()` in the same expression). Nothing behavioural. Build: docker build --target qt PASS, zero warnings, zero errors. Co-Authored-By: claude-flow --- qt/src/MainWindow.cpp | 24 +++++++++++------------- qt/src/MainWindow.h | 19 +++++++++++-------- qt/src/app/GhosttyApp.cpp | 33 ++++++++++++++++++++++----------- qt/src/app/GhosttyApp.h | 12 ++++++------ 4 files changed, 50 insertions(+), 38 deletions(-) diff --git a/qt/src/MainWindow.cpp b/qt/src/MainWindow.cpp index 40d325e73..ab171ad20 100644 --- a/qt/src/MainWindow.cpp +++ b/qt/src/MainWindow.cpp @@ -189,9 +189,6 @@ MainWindow::~MainWindow() { } } -// configHasCustomShader moved to GhosttyApp.cpp (custom-shader is an -// app-level fact: it drives needsPremultiply for every surface). - // Parse a libghostty duration string into nanoseconds. The format is // concatenated `` segments per Config.zig's Duration.parseCLI: // y w d h m s ms µs us ns @@ -466,9 +463,10 @@ MainWindow *MainWindow::newWindow(ghostty_surface_t parent) { // window at the same origin — macOS does this via // NSWindow.cascadeTopLeft. Wayland compositors typically ignore // window placement requests; this is a hint at most. + ghostty_config_t cfg = GhosttyApp::instance().config(); int16_t posX = 0, posY = 0; - const bool haveX = configGet(GhosttyApp::instance().config(), &posX, "window-position-x"); - const bool haveY = configGet(GhosttyApp::instance().config(), &posY, "window-position-y"); + const bool haveX = configGet(cfg, &posX, "window-position-x"); + const bool haveY = configGet(cfg, &posY, "window-position-y"); if (haveX && haveY) { w->move(posX, posY); } else { @@ -490,7 +488,7 @@ MainWindow *MainWindow::newWindow(ghostty_surface_t parent) { if (!s_initialWindowConsumed) { s_initialWindowConsumed = true; bool initialWindow = true; - configGet(GhosttyApp::instance().config(), &initialWindow, "initial-window"); + configGet(cfg, &initialWindow, "initial-window"); wantsShow = initialWindow; } if (wantsShow) w->show(); @@ -1435,9 +1433,9 @@ void MainWindow::refreshChrome() { // Refresh app-scoped state. quit-after-last-window-closed[-delay] // can change the delay or the quitOnLastWindowClosed strategy at // runtime; mirrors the calculation in initialize(). - if (GhosttyApp::instance().config()) { + if (ghostty_config_t cfg = GhosttyApp::instance().config()) { bool quitAfter = true; - configGet(GhosttyApp::instance().config(), &quitAfter, "quit-after-last-window-closed"); + configGet(cfg, &quitAfter, "quit-after-last-window-closed"); // Same Duration-decode workaround as initialize(). const uint64_t delayNs = parseDurationNs( configValue(QStringLiteral("quit-after-last-window-closed-delay")), 0); @@ -2003,9 +2001,9 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, // *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. - const QList &live_ = GhosttyApp::instance().windows(); + const QList &live = GhosttyApp::instance().windows(); MainWindow *win = src ? src->owner() - : (live_.isEmpty() ? nullptr : live_.first()); + : (live.isEmpty() ? nullptr : live.first()); QPointer winp(win); QPointer srcp(src); @@ -2083,10 +2081,10 @@ bool MainWindow::onAction(ghostty_app_t, ghostty_target_s target, // global keybind can rename even when no surface is the action's // explicit target. Mirrors macOS NSApp.mainWindow promotion. GhosttySurface *target = src; - const QList &live_pt = GhosttyApp::instance().windows(); - if (!target && !live_pt.isEmpty()) { + const QList &allWindows = GhosttyApp::instance().windows(); + if (!target && !allWindows.isEmpty()) { MainWindow *active = qobject_cast(qApp->activeWindow()); - if (!active) active = live_pt.first(); + if (!active) active = allWindows.first(); if (active) target = active->surfaceAt(active->m_tabs->currentIndex()); } if (!target) return false; diff --git a/qt/src/MainWindow.h b/qt/src/MainWindow.h index 1243a96d7..6363e07df 100644 --- a/qt/src/MainWindow.h +++ b/qt/src/MainWindow.h @@ -21,7 +21,8 @@ class GhosttySurface; // A top-level window presenting terminal surfaces as tabs; each tab may // be subdivided into splits. The libghostty app and config are shared -// process-wide across every window (the static s_* members below). +// process-wide via GhosttyApp::instance(); MainWindow's config() and +// needsPremultiply() forward there. // // Widget tree: QTabWidget -> tab page (QWidget) -> split tree, where a // node is either a GhosttySurface (a QOpenGLWidget) or a QSplitter of @@ -139,9 +140,10 @@ private slots: void onCurrentChanged(int index); private: - // GhosttyApp registers our static runtime callbacks (onWakeup, - // onAction, ...) with libghostty. Phase 1.0 only — phase 1.1 - // moves the callbacks onto GhosttyApp itself and drops this. + // GhosttyApp::onCloseSurface needs to call confirmCloseSurfaces / + // removeSurface (both private) on the target window from a deferred + // queued slot. Phase 2's ActionDispatcher refactor will replace + // this with public predicates on the per-window API. friend class GhosttyApp; // Create the first tab once the device pixel ratio has settled. @@ -192,10 +194,11 @@ private: // Rebuild the config from disk and push it to libghostty. void reloadConfig(); - // App-scoped reload entry point. The config is process-wide (statics - // in this class), so reload from any window has the same effect; the - // RELOAD_CONFIG action posts to qApp via this static so the reload - // can't be cancelled by the source window closing mid-dispatch. + // App-scoped reload entry point. The config is process-wide (held + // by GhosttyApp), so a reload from any window has the same effect; + // the RELOAD_CONFIG action posts to qApp via this static so the + // reload can't be cancelled by the source window closing + // mid-dispatch. static void reloadConfigGlobal(); // Refresh every window's chrome from the current config (used after a // reload and on the CONFIG_CHANGE notification). diff --git a/qt/src/app/GhosttyApp.cpp b/qt/src/app/GhosttyApp.cpp index a30176413..c5bc1c56d 100644 --- a/qt/src/app/GhosttyApp.cpp +++ b/qt/src/app/GhosttyApp.cpp @@ -20,11 +20,10 @@ #include "../GhosttySurface.h" #include "../MainWindow.h" -// Process-wide libghostty state. Only the libghostty handles + their -// bring-up / teardown lifecycle live here in phase 1.0; the runtime -// callbacks (onWakeup, onAction, onReadClipboard, ...) and the window -// registry, undo stack, frame timer, etc. all still live on -// MainWindow and migrate in subsequent phases. +// Process-wide libghostty state and the runtime callbacks libghostty +// hands back. onAction stays on MainWindow until phase 2 introduces +// the ActionDispatcher; everything else is here. The undo/redo stack +// stays on MainWindow as well. // Whether the Ghostty config enables a custom shader. libghostty does // not expose this through ghostty_config_get (`custom-shader` is a @@ -57,9 +56,19 @@ GhosttyApp &GhosttyApp::instance() { } GhosttyApp::~GhosttyApp() { - // Backstop for an early-exit path (ghostty_init failure inside - // main()). The normal teardown runs from the last MainWindow's dtor. - teardown(); + // Process-exit ordering: this static-local destructor runs AFTER + // main() returns and the stack-allocated QApplication has been + // destroyed, so qApp is null and the Qt event system is gone. + // Calling teardown() here would invoke + // QCoreApplication::sendPostedEvents on a dangling qApp. + // + // The normal shutdown path runs teardown() from the last + // MainWindow's dtor (while QApplication is still alive); by the + // time we get here, m_app / m_config / m_frameTimer / m_quitTimer + // are already null. A second teardown() would be redundant and + // unsafe. The early-exit case (ghostty_init failure in main, before + // any window is constructed) never calls instance(), so this + // destructor never runs in that path either. } bool GhosttyApp::ensureInitialized() { @@ -98,9 +107,11 @@ bool GhosttyApp::ensureInitialized() { } void GhosttyApp::replaceConfig(ghostty_config_t new_config) { - // libghostty keeps borrowed references to the previous config (the - // surface message queue), so the new must be installed and the old - // freed in this order. + // PRECONDITION: the caller has already pushed `new_config` to + // libghostty via ghostty_app_update_config. libghostty's surface + // message queue may still hold borrowed references to the old + // config until that update completes — by the time we get here, + // the queue has adopted the new config and the old is safe to free. if (m_config && m_config != new_config) ghostty_config_free(m_config); m_config = new_config; m_needsPremultiply = configHasCustomShader(); diff --git a/qt/src/app/GhosttyApp.h b/qt/src/app/GhosttyApp.h index e5078aaed..0af923c59 100644 --- a/qt/src/app/GhosttyApp.h +++ b/qt/src/app/GhosttyApp.h @@ -14,16 +14,16 @@ class QTimer; // // Owns the single ghostty_app_t and ghostty_config_t instances that // drive every window in the process, plus the derived needsPremultiply -// flag that the surfaces' renderer reads when blitting frames. +// flag that the surfaces' renderer reads when blitting frames. Hosts +// the live MainWindow registry, the 60Hz frame timer, the +// quit-after-last-window-closed timer, the wakeup tick coalescer, and +// every libghostty runtime callback except onAction (which still +// lives on MainWindow for private-member access until phase 2's +// ActionDispatcher). // // Singleton — there is never more than one libghostty app per // process. Construction is deferred to the first instance() call so // QApplication can exist before the singleton is built. -// -// Phase 1.0 scope: only the libghostty handles + bring-up / teardown -// live here. The frame timer, runtime callbacks, window registry, -// undo stack, quit-timer state, and action dispatch all stay on -// MainWindow for now; subsequent phases migrate them. class GhosttyApp { public: static GhosttyApp &instance();