From d5fcae874d577fd39e8ae66c2ac59aef3e7cdc7c Mon Sep 17 00:00:00 2001 From: Alexis Dumas Date: Sat, 12 Sep 2026 09:20:34 -0400 Subject: [PATCH] fix: stop destroying sidebar rows mid-handler; async daemon spawn Two changes that together remove a reproducible crash: two or more addWorkspaceDirect calls in one event-loop turn killed the app with "Object ... destroyed while one of its QML signal handlers is in progress". Root cause: DaemonLink::requestSync spun a nested event loop from inside PtyClient::start <- PanePool::createPaneWithId <- TerminalPane Component.onCompleted <- the Repeater that AppModel regenerates on a workspace signal. That loop delivered a QML deferred delete (destroy() is deleteLater) while a sidebar card still had a handler on the stack, and Qt aborts on exactly that. - Pane spawn is now fully asynchronous. DaemonLink::connectAsync() does discovery, process spawn, and the hello/list handshake with timers (spawn poll + handshake watchdog) instead of local event loops, and announces readiness on statusChanged. PtyClient::start() stores the spawn spec and makes the reattach-or-spawn decision when the link reports Ready; a failure surfaces on the new spawnFailed() signal. The spawn callback holds a QPointer and kills the shell when the pane died while the reply was in flight, so no shell leaks in the daemon. - PanePool::connectDaemon() and restartDaemon() are asynchronous too: the restart polls the old daemon pid with a timer, then respawns every pane from the Ready handler. waitForDaemonExit() and ensureConnected() are gone; requestSync() is left for the quit-time batched list only. - SidebarView rebuilds its rows by keyed reconciliation instead of destroying and recreating them all: rows are keyed by workspace id (or family root), reused in place, and only rows that left the layout are destroyed. Cards keep rendering fresh content because their text bindings read the NOTIFY workspaces property through liveWorkspace(). Verified: three and five rapid jj-reseat adds survive (previously fatal); spawn, reattach on relaunch, daemon restart (new daemon, new shell pids), unavailable-daemon and recovery, rename, jj forget, delete, family collapse, mission control, preview streaming, paste, and the add/delete churn path (no leaked shells). ctest 5/5 and static analysis clean. --- app/controllers/PanePool.cpp | 120 ++++++++++------ app/controllers/PanePool.h | 35 ++++- app/qml/components/SidebarView.qml | 141 ++++++++++++++---- app/qml/pages/TerminalPage.qml | 7 +- app/support/DaemonLink.cpp | 221 ++++++++++++++--------------- app/support/DaemonLink.h | 50 ++++--- app/terminal/PtyClient.cpp | 138 +++++++++++++----- app/terminal/PtyClient.h | 47 +++++- app/terminal/TerminalQuickItem.cpp | 11 +- 9 files changed, 520 insertions(+), 250 deletions(-) diff --git a/app/controllers/PanePool.cpp b/app/controllers/PanePool.cpp index ae7de4f..eeb6b38 100644 --- a/app/controllers/PanePool.cpp +++ b/app/controllers/PanePool.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -17,13 +18,17 @@ PanePool* PanePool::s_instance = nullptr; namespace { -// Bounded wait in restartDaemon() for the old daemon process to exit after -// the shutdown request, before spawning a successor. +// Restart orchestration (PanePool::restartDaemon): the old daemon process +// gets this long to disappear after the shutdown request before the +// successor spawn is attempted anyway, and the exit probe runs this often. constexpr int kRestartExitTimeoutMs = 3000; +constexpr int kRestartPollIntervalMs = 100; } // namespace PanePool::PanePool(QObject* parent) - : QObject(parent), m_daemon(new DaemonLink(this)) { + : QObject(parent), + m_restartPollTimer(new QTimer(this)), + m_daemon(new DaemonLink(this)) { s_instance = this; // A newly created pane must attach its data connection promptly. // refreshStreaming() is idempotent (setStreaming no-ops when the state @@ -34,9 +39,28 @@ PanePool::PanePool(QObject* parent) // fire afterwards). connect(this, &PanePool::paneCreated, this, &PanePool::refreshStreaming); // The stats page tracks the link's status too: every transition - // (connecting/ready/unavailable) invalidates daemonStats(). + // (connecting/ready/unavailable) invalidates daemonStats(). Ready is also + // when queued work proceeds, and Unavailable clears a stuck restart guard. connect(m_daemon, &DaemonLink::statusChanged, this, - [this](DaemonLink::Status) { emit statsChanged(); }); + [this](DaemonLink::Status status) { + if (status == DaemonLink::Status::Ready) { + onDaemonReady(); + } else if (status == DaemonLink::Status::Unavailable && + m_restartConnecting) { + // The reconnect this restart asked for failed (the link's own + // watchdogs report Unavailable on spawn/handshake timeout). + // Release the guard so the user can try again. + qWarning() << "xenomorphic: daemon restart failed to reconnect"; + m_restarting = false; + m_restartConnecting = false; + } + emit statsChanged(); + }); + // Restart-exit poll (restartDaemon): a timer, never a nested event loop, + // because restartDaemon runs from a QML button handler. + m_restartPollTimer->setSingleShot(true); + connect(m_restartPollTimer, &QTimer::timeout, this, + &PanePool::pollRestartExit); } PanePool::~PanePool() { @@ -177,13 +201,14 @@ void PanePool::destroyPane(const QString& paneId) { } void PanePool::connectDaemon() { - QString error; - if (!m_daemon->ensureConnected(&error)) { - // Stay up: panes will fail to start with a visible error instead. - qWarning() << "xenomorphic: daemon connect failed:" << error; - return; - } - refreshStreaming(); + // Non-blocking by contract: discovery, the daemon spawn, and the handshake + // all complete on the GUI thread's event loop. Panes created in the + // meantime queue their spawn inside PtyClient and start when the link + // reports Ready (onDaemonReady pushes the streaming set at the same moment). + // A nested loop here would run under QML handlers (this is called from + // TerminalPage.qml's Component.onCompleted) and would deliver QML deferred + // deletes while a handler is still on the stack, which Qt aborts on. + m_daemon->connectAsync(); } QVariantMap PanePool::daemonStats() const { @@ -223,9 +248,10 @@ void PanePool::refreshDaemonStats() { } void PanePool::restartDaemon() { - // GUI thread + QML button only: waitForDaemonExit() and the - // ensureConnected() handshake spin bounded local event loops, which must - // never run from a socket handler (same reentry rule as ensureConnected). + // Fully asynchronous: no nested event loop runs here, because this is + // called from a QML button handler and a loop there would deliver QML + // deferred deletes (destroy()) while a handler is still on the stack, which + // Qt aborts on. The respawn pass runs from onDaemonReady(). if (m_restarting) { qWarning() << "xenomorphic: daemon restart already in progress"; return; @@ -233,44 +259,52 @@ void PanePool::restartDaemon() { m_restarting = true; // 1) Ask the running daemon to shut down. A 0 return (nothing connected) - // is fine — step 2 has nothing to wait for and ensureConnected() will - // spawn a fresh daemon regardless. + // is fine: the exit poll finds no live process and the successor spawn + // follows. m_daemon->requestShutdown(); - // 2) Bounded wait for the daemon PROCESS to die so the single-instance - // lock and the /ctl listen socket are free before we spawn a successor. - // On timeout proceed anyway: the spawn path will surface a real failure. - // The boolean is diagnostic only — a timed-out wait still reaches the - // spawn path below, which surfaces any real failure. - const bool daemonExited = m_daemon->waitForDaemonExit(kRestartExitTimeoutMs); - if (!daemonExited) { + // 2) Poll for the daemon PROCESS to die so the single-instance lock and the + // /ctl listen socket are free before spawning a successor. On timeout + // proceed anyway: connectAsync() surfaces any real failure. + m_restartElapsed.start(); + m_restartPollTimer->start(kRestartPollIntervalMs); +} + +void PanePool::pollRestartExit() { + const bool stillAlive = m_daemon->daemonProcessAlive(); + if (stillAlive && !m_restartElapsed.hasExpired(kRestartExitTimeoutMs)) { + m_restartPollTimer->start(kRestartPollIntervalMs); + return; + } + if (stillAlive) { qWarning() << "xenomorphic: daemon did not exit within" << kRestartExitTimeoutMs << "ms; attempting respawn anyway"; } + // Discover-or-spawn + handshake, in the background. m_restarting stays set + // until Ready (onDaemonReady runs the respawn pass) or a failed connect + // (the status handler clears the guard). + m_restartConnecting = true; + m_daemon->connectAsync(); +} - // 3) Discover-or-spawn + bounded handshake. On failure leave the pool - // intact — the link is in Unavailable, which the stats page reflects. - QString error; - if (!m_daemon->ensureConnected(&error)) { - qWarning() << "xenomorphic: daemon restart failed to reconnect:" << error; +void PanePool::onDaemonReady() { + if (m_restarting) { + // 4) Respawn every pooled pane against the fresh daemon. respawn() + // re-queues the PTY spawn (same pane id, same spec); the items' vt/render + // state is untouched, so the terminals keep showing their old content + // until the new shell draws. m_restarting = false; - emit statsChanged(); - return; - } - - // 4) Respawn every pooled pane. respawn() re-runs the PTY spawn against - // the fresh daemon (same pane id, same spec); the items' vt/render state - // is untouched, so the terminals keep showing their old content until the - // new shell draws. - for (auto entryIt = m_panes.constBegin(); entryIt != m_panes.constEnd(); - ++entryIt) { - if (auto* term = entryIt->item.data()) { - term->respawn(); + m_restartConnecting = false; + for (auto entryIt = m_panes.constBegin(); entryIt != m_panes.constEnd(); + ++entryIt) { + if (auto* term = entryIt->item.data()) { + term->respawn(); + } } } + // Panes that asked to stream while the link was down attach now; this is + // also the ordinary path, and setStreaming is idempotent. refreshStreaming(); - m_restarting = false; - emit statsChanged(); } void PanePool::setPreviewStreaming(bool on) { diff --git a/app/controllers/PanePool.h b/app/controllers/PanePool.h index 922f673..05753be 100644 --- a/app/controllers/PanePool.h +++ b/app/controllers/PanePool.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -12,6 +13,7 @@ #include +class QTimer; class TerminalQuickItem; class DaemonLink; @@ -65,9 +67,10 @@ class PanePool : public QObject { Q_INVOKABLE void destroyPane(const QString& paneId); // Discover-or-spawn the daemon and open the control connection. Called - // from QML BEFORE AppModel.startup so the link is Ready before the first - // pane spawns. Never crashes on failure: panes will surface a visible - // start error instead. + // from QML during startup. NON-BLOCKING: the daemon comes up in the + // background, panes created meanwhile queue their spawn inside PtyClient, + // and readiness is announced by statsChanged() (the link's status). A + // failure to connect leaves the app up and surfaces on the stats page. Q_INVOKABLE void connectDaemon(); // The daemon settings page's stats surface. All values come from the @@ -80,10 +83,11 @@ class PanePool : public QObject { // the reply lands (or the request fails). Safe from QML. Q_INVOKABLE void refreshDaemonStats(); - // Graceful daemon restart (GUI thread only, QML button only — it spins a - // bounded local event loop and must never run from a socket handler): - // request shutdown, poll for the daemon process to exit, discover-or-spawn - // a fresh one, then respawn every pooled pane's shell. + // Graceful daemon restart: request shutdown, poll for the daemon process to + // exit, discover-or-spawn a fresh one, then respawn every pooled pane's + // shell. Fully asynchronous (every wait is a timer), so it is safe to call + // from QML: it never spins an event loop that could deliver a QML deferred + // delete while a signal handler is on the stack. Q_INVOKABLE void restartDaemon(); // Push the active-tab set into every pane's streaming state (attach the @@ -150,6 +154,12 @@ signals: }; void connectItem(TerminalQuickItem* term, const QString& paneId); + // A Ready link is when queued work can proceed: a restart's respawn pass + // and every pane that wanted to stream while the daemon was down. + void onDaemonReady(); + // Restart-exit poll tick: waits for the old daemon process to disappear, + // then asks for a fresh connect. Never blocks (see restartDaemon). + void pollRestartExit(); QHash m_panes; // Whether background workspaces' active tabs keep their streams attached @@ -157,7 +167,18 @@ signals: // the page pushes its first state. bool m_previewStreaming = true; // Re-entrancy guard for restartDaemon(): one graceful restart at a time. + // Cleared by the respawn pass (onDaemonReady) or when the reconnect that + // follows the exit wait fails. bool m_restarting = false; + // True once the restart has moved past its exit wait and asked for a + // reconnect. The OLD daemon's disconnect lands as Unavailable during the + // wait, which must NOT clear m_restarting: only a failure of the reconnect + // this flag guards may. + bool m_restartConnecting = false; + // Restart-exit poll: the old daemon process gets kRestartExitTimeoutMs to + // disappear after the shutdown request before spawn anyway. + QTimer* m_restartPollTimer = nullptr; + QElapsedTimer m_restartElapsed; QPointer m_parkHost; DaemonLink* m_daemon = nullptr; // owned child; see constructor QQmlEngine* m_engine = nullptr; diff --git a/app/qml/components/SidebarView.qml b/app/qml/components/SidebarView.qml index 121c7c0..082a34a 100644 --- a/app/qml/components/SidebarView.qml +++ b/app/qml/components/SidebarView.qml @@ -33,14 +33,20 @@ Item { property alias addButton: addButton // The rendered cards (one per workspace, in family/display order). - // Rebuilt wholesale on rebuildCards(); each card re-binds its workspace - // object. + // Maintained by rebuildCards()'s keyed reconciliation; each card + // resolves its workspace live by index. property var cardItems: [] // Every rendered row (spacer, family headers, label, cards), in display // order. cardColumnHeight() sums this; card-specific APIs use cardItems. property var rowItems: [] + // Reconciliation keys, parallel to rowItems (same indices; see + // rowKeyOf). A live row cannot carry its own key: QML objects reject + // assignment to a property they do not declare, so the sidebar keeps the + // key beside the object. + property var rowKeys: [] + // RepoRoots of families currently collapsed (array of strings). // Persisted via settings.sidebarFamilyCollapse. property var collapsedFamilies: [] @@ -453,22 +459,90 @@ Item { rebuildCards() } - // Rebuild the row list from `workspaces`. Rows resolve their workspace - // LIVE by index (createObject initial properties COPY plain JS objects, - // so passing `workspace` would freeze a stale snapshot). + // Stable key for a layout row, so rebuildCards() can match a desired + // row against an existing live row and reuse the object. Card rows key + // on the workspace ID (a CONSTANT Q_PROPERTY, stable across the kind + // reseat that replaces the Workspace object), falling back to the + // index when the lookup fails; header rows key on the family root; + // the spacer and label are unique, so the kind alone keys them. + function rowKeyOf(row) { + if (row.kind === "card") { + var j = row.workspaceIndex + if (j >= 0 && j < workspaces.length && workspaces[j] + && workspaces[j].id !== undefined) { + return "card:" + workspaces[j].id + } + return "card:" + j + } + if (row.kind === "familyHeader") { + return "header:" + row.root + } + return row.kind + } + + // Reconcile the row list with the layout computed from `workspaces`: + // reuse every live row whose key still exists (updating only its + // mutable properties), create the missing rows, and destroy only the + // rows that genuinely left the layout. Rows must NOT be destroyed + // wholesale: QML's destroy() is DEFERRED, and a nested event loop + // (for example the daemon spawn when a workspace is added) delivers + // that deferred delete while one of the destroyed card's own QML + // signal handlers is still on the stack, which aborts the app with + // "Object ... destroyed while one of its QML signal handlers is in + // progress". Reuse keeps the handler's owner alive. A reused card + // still renders fresh content: its text bindings read + // sidebar.workspaces (a NOTIFY property) through liveWorkspace(), so + // they re-evaluate on every workspacesChanged. function rebuildCards() { + var rows = computeLayoutRows() + + // Index the current rows by key. rowKeys is parallel to rowItems, so + // the map holds indices rather than objects. + var live = {} for (var i = 0; i < rowItems.length; ++i) { - if (rowItems[i]) rowItems[i].destroy() + if (rowItems[i] !== undefined && rowKeys[i] !== undefined) { + live[rowKeys[i]] = i + } } - rowItems = [] - cardItems = [] - var rows = computeLayoutRows() + var newRows = [] + var newKeys = [] + var newCards = [] for (var r = 0; r < rows.length; ++r) { - const row = rows[r] + var row = rows[r] + var key = rowKeyOf(row) + var at = live[key] + if (at !== undefined) { + // Reuse: drop from the lookup so leftovers can be destroyed, + // refresh the mutable properties, and keep the object's + // position in the display order. + delete live[key] + var item = rowItems[at] + if (row.kind === "card") { + item.workspaceIndex = row.workspaceIndex + item.selected = (row.workspaceIndex === activeWorkspaceIndex) + } else if (row.kind === "familyHeader") { + item.title = row.title + item.rootPath = row.root + item.collapsed = row.collapsed + item.memberCount = row.members.length + } + // The label row's width binding is set at creation and + // needs no update on reuse. + newRows.push(item) + newKeys.push(key) + if (row.kind === "card") { + newCards.push(item) + } + continue + } + + // Create: no live row carries this key. if (row.kind === "spacer") { - rowItems.push(spacerComp.createObject(cardColumn)) + item = spacerComp.createObject(cardColumn) } else if (row.kind === "familyHeader") { + // Block-scoped captures: a `var` closure here would leave + // every handler holding the LAST loop iteration's row. const header = familyHeaderComp.createObject(cardColumn, { title: row.title, rootPath: row.root, @@ -476,19 +550,17 @@ Item { memberCount: row.members.length }) header.width = Qt.binding(function() { return cardColumn.width }) - // Block-scoped captures: a `var` closure here would leave - // every handler holding the LAST loop iteration's row. header.toggled.connect(sidebar, function() { toggleFamily(header.rootPath) }) - rowItems.push(header) + item = header } else if (row.kind === "label") { - rowItems.push(labelComp.createObject(cardColumn, { + item = labelComp.createObject(cardColumn, { width: Qt.binding(function() { return cardColumn.width }) - })) + }) } else if (row.kind === "card") { var j = row.workspaceIndex - var card = cardComp.createObject(cardColumn, { + item = cardComp.createObject(cardColumn, { workspaceIndex: j, selected: (j === activeWorkspaceIndex), page: sidebar.page, @@ -500,12 +572,26 @@ Item { // never covers card content or its rounded corners. // Symmetric 9px side margins (the old 5px left / 13px right // pair read as lopsided next to the sidebar's right border). - card.width = Qt.binding(function() { return cardColumn.width - 18 }) - card.x = 9 - cardItems.push(card) - rowItems.push(card) + item.width = Qt.binding(function() { return cardColumn.width - 18 }) + item.x = 9 + newCards.push(item) + } + if (item) { + newRows.push(item) + newKeys.push(key) } } + + // Destroy only the rows that left the layout. destroy() is deferred, + // so these objects stay alive until the current handler unwinds; no + // reused row is ever destroyed. + for (var staleKey in live) { + rowItems[live[staleKey]].destroy() + } + + rowItems = newRows + rowKeys = newKeys + cardItems = newCards // The card column's height determines the scroll content height. // Set it explicitly from the LIVE row list (a Qt.binding would // capture the array reference at creation and never re-evaluate). @@ -525,11 +611,14 @@ Item { return h } - // Reassigned whenever the workspace array is replaced (rename, kind - // change, add, delete). Rebuilding wholesale re-binds every card to - // fresh instances so title/kind/path re-evaluate even when the - // workspace OBJECTS are unchanged (QML bindings cannot observe plain - // JS-object property edits). Cards are light; this is cheap. + // Fired whenever the workspace array is replaced (rename, kind change, + // add, delete). The rows are RECONCILED, not rebuilt: matching rows are + // reused and only rows that left the layout are destroyed. Wholesale + // destruction is unsafe here because QML's destroy() is deferred, and + // the nested event loop a workspace change can spawn (the daemon + // spawn) delivers that deferred delete while a card's own QML signal + // handler is still on the stack, aborting the app with "Object ... + // destroyed while one of its QML signal handlers is in progress". onWorkspacesChanged: rebuildCards() // The selected card follows the shell's activeWorkspaceIndex (Shell diff --git a/app/qml/pages/TerminalPage.qml b/app/qml/pages/TerminalPage.qml index 0123c2f..aeba697 100644 --- a/app/qml/pages/TerminalPage.qml +++ b/app/qml/pages/TerminalPage.qml @@ -551,9 +551,10 @@ Item { // Seed the streaming-set policy from the initial sidebar/mission // state, before startup spawns the first panes. page.pushPreviewStreaming() - // Bring the daemon link up BEFORE startup: PtyClient::start() needs - // the link Ready for the first pane's spawn (and the streaming - // refresh), so this must precede AppModel.startup below. + // Bring the daemon link up BEFORE startup. The connect is + // asynchronous now, so panes created by startup queue their spawn + // inside PtyClient and run when the link reports Ready; keeping the + // order means the connect attempt starts before the first pane's. PanePool.connectDaemon() // Startup: load session or build the default workspace (root-context // properties from main.cpp), then the proxy tree renders reactively. diff --git a/app/support/DaemonLink.cpp b/app/support/DaemonLink.cpp index 35edf37..640060d 100644 --- a/app/support/DaemonLink.cpp +++ b/app/support/DaemonLink.cpp @@ -44,7 +44,6 @@ namespace { constexpr int kSpawnDiscoveryTimeoutMs = 10000; constexpr int kSpawnPollIntervalMs = 100; constexpr int kHandshakeTimeoutMs = 8000; -constexpr int kWaitPollIntervalMs = 20; [[nodiscard]] bool pidAlive(qint64 pid) { if (pid <= 0) { @@ -87,7 +86,10 @@ void applySpawnResult(QHash& panes, } // namespace -DaemonLink::DaemonLink(QObject* parent) : QObject(parent) { +DaemonLink::DaemonLink(QObject* parent) + : QObject(parent), + m_spawnPollTimer(new QTimer(this)), + m_handshakeTimer(new QTimer(this)) { m_socket = new QWebSocket(QStringLiteral("xenomorphic"), QWebSocketProtocol::VersionLatest, this); connect(m_socket, &QWebSocket::connected, this, &DaemonLink::onConnected); @@ -96,6 +98,20 @@ DaemonLink::DaemonLink(QObject* parent) : QObject(parent) { &DaemonLink::onTextMessage); connect(m_socket, &QWebSocket::errorOccurred, this, &DaemonLink::onErrorOccurred); + + // Connect watchdogs (see connectAsync). Both are single-shot and are + // stopped by setStatus() the moment the link leaves Connecting, so a + // completed or failed connect never leaves a stale timer armed. + m_spawnPollTimer->setSingleShot(true); + connect(m_spawnPollTimer, &QTimer::timeout, this, &DaemonLink::pollSpawn); + + m_handshakeTimer->setSingleShot(true); + connect(m_handshakeTimer, &QTimer::timeout, this, [this]() { + if (m_status == Status::Connecting) { + failConnect(QStringLiteral("daemon handshake did not complete within %1 ms") + .arg(kHandshakeTimeoutMs)); + } + }); } DaemonLink::~DaemonLink() { @@ -110,6 +126,12 @@ void DaemonLink::setStatus(Status status) { return; } m_status = status; + if (status != Status::Connecting) { + // The watchdogs only guard the Connecting window. Ready and Unavailable + // both end it, so neither timer may stay armed into the next connect. + m_spawnPollTimer->stop(); + m_handshakeTimer->stop(); + } emit statusChanged(status); } @@ -176,7 +198,7 @@ bool DaemonLink::discover(QString* errorOut) { return true; } -bool DaemonLink::spawnDaemon(QString* errorOut) { +bool DaemonLink::startDaemon(QString* errorOut) { const QString exe = xeno::paths::daemonExecutable(); if (exe.isEmpty()) { // Clear error naming the resolution order XenoPaths documents. @@ -225,7 +247,9 @@ bool DaemonLink::spawnDaemon(QString* errorOut) { process.setStandardErrorFile(logPath, QIODevice::Append); // startDetached() puts the child in its own session (the setsid() of the - // old recipe) and returns immediately. Do NOT setsid again here. + // old recipe) and returns immediately. Do NOT setsid again here. The wait + // for daemon.json lives in connectAsync()'s poll, never here: this runs on + // paths that may sit under a QML signal handler. qint64 pid = 0; if (!process.startDetached(&pid)) { if (errorOut != nullptr) { @@ -234,29 +258,73 @@ bool DaemonLink::spawnDaemon(QString* errorOut) { } return false; } + return true; +} - // Poll for the daemon to write daemon.json, bounded. It only appears after - // the daemon's listen() succeeds (and after it won the single-instance - // lock), so a successful read means a usable port/token pair. - QElapsedTimer elapsed; - elapsed.start(); - QString discError; - while (!elapsed.hasExpired(kSpawnDiscoveryTimeoutMs)) { - if (discover(&discError)) { - return true; - } - QEventLoop wait; - QTimer::singleShot(kSpawnPollIntervalMs, &wait, &QEventLoop::quit); - wait.exec(); +void DaemonLink::connectAsync() { + // Idempotent: Ready means there is nothing to do, and Connecting means a + // connect (with its watchdogs) is already in flight. + if (m_status == Status::Ready || m_status == Status::Connecting) { + return; } - if (errorOut != nullptr) { - *errorOut = QStringLiteral("spawned daemon did not publish %1 within %2 ms " - "(last discovery error: %3)") - .arg(xeno::paths::daemonJsonPath()) - .arg(kSpawnDiscoveryTimeoutMs) - .arg(discError); + setStatus(Status::Connecting); + + // Discovery is synchronous and cheap: a file read plus a kill(pid, 0) + // probe. Only the waits need to be asynchronous. + QString error; + if (discover(&error)) { + openControlSocket(); + return; + } + if (!startDaemon(&error)) { + failConnect(error); + return; + } + // Wait for the daemon to publish daemon.json: it only appears after the + // daemon's listen() succeeds (and after it won the single-instance lock), + // so a successful read means a usable port/token pair. + m_spawnElapsed.start(); + m_spawnPollTimer->start(kSpawnPollIntervalMs); +} + +void DaemonLink::pollSpawn() { + QString error; + if (discover(&error)) { + openControlSocket(); + return; + } + if (m_spawnElapsed.hasExpired(kSpawnDiscoveryTimeoutMs)) { + failConnect( + QStringLiteral("spawned daemon did not publish %1 within %2 ms " + "(last discovery error: %3)") + .arg(xeno::paths::daemonJsonPath()) + .arg(kSpawnDiscoveryTimeoutMs) + .arg(error)); + return; + } + m_spawnPollTimer->start(kSpawnPollIntervalMs); +} + +void DaemonLink::openControlSocket() { + // /ctl with the token in the handshake header, never in-band. + QNetworkRequest controlRequest(QUrl(QStringLiteral("ws://127.0.0.1:%1/ctl") + .arg(m_port))); + controlRequest.setRawHeader("X-Xeno-Token", m_token.toUtf8()); + m_socket->abort(); + m_socket->open(controlRequest); + + // Bounded wait for connected() -> onConnected() -> hello -> list -> Ready. + // onConnected() drives the handshake non-blocking, so the watchdog only + // has to fail the connect if the daemon never answers. + m_handshakeTimer->start(kHandshakeTimeoutMs); +} + +void DaemonLink::failConnect(const QString& message) { + qWarning() << "DaemonLink:" << message; + setStatus(Status::Unavailable); + if (m_socket != nullptr) { + m_socket->abort(); } - return false; } void DaemonLink::onConnected() { @@ -288,7 +356,8 @@ void DaemonLink::onConnected() { m_boot = daemonBoot; } // The daemon's own pid, cached for the restart orchestration's - // kill(pid, 0) exit poll (PanePool::restartDaemon via waitForDaemonExit). + // kill(pid, 0) liveness probe (daemonProcessAlive, polled on a timer by + // PanePool::restartDaemon). m_daemonPid = static_cast(result.value(QStringLiteral("pid")).toDouble(0)); QJsonObject list; @@ -320,16 +389,16 @@ void DaemonLink::onDisconnected() { failAllPending(QStringLiteral("daemon connection closed (%1)").arg(code)); // Drop the per-connection diagnostics, including the pid: the daemon is - // gone, so any later exit poll must not wait on a stale pid. PanePool's - // restart orchestration tolerates this (waitForDaemonExit treats "unknown - // pid" as already exited). + // gone, so any later probe must not test a stale pid. PanePool's restart + // orchestration tolerates this (daemonProcessAlive treats "unknown pid" as + // already gone). m_ctlClients = -1; m_daemonPid = 0; switch (code) { case static_cast(QWebSocketProtocol::CloseCodePolicyViolated): // 1008: bad/missing token. Never retry in a tight loop — surface - // unavailable; a later ensureConnected() may attempt the spawn path. + // unavailable; a later connectAsync() may attempt the spawn path. qWarning() << "DaemonLink: daemon rejected authentication (close 1008," " reason:" << reason << ")"; @@ -349,7 +418,7 @@ void DaemonLink::onDisconnected() { break; default: // Abnormal close or transport error: daemon dead. The spawn path in - // ensureConnected() is allowed to bring one up. + // connectAsync() is allowed to bring one up. qWarning() << "DaemonLink: daemon connection lost (close" << code << ")" << reason; setStatus(Status::Unavailable); @@ -446,7 +515,7 @@ void DaemonLink::handleNotification(const QJsonObject& message) { if (op == QLatin1String("daemonDown")) { const QString reason = message.value(QStringLiteral("reason")).toString(); // Announced shutdown. Clients must NOT auto-respawn: surface it and let - // the user (or an explicit ensureConnected) decide. + // the user (or an explicit connectAsync) decide. emit daemonDown(reason); return; } @@ -499,62 +568,6 @@ void DaemonLink::applyListResult(const QJsonObject& result) { m_panes = std::move(panes); } -bool DaemonLink::ensureConnected(QString* errorOut) { - if (errorOut != nullptr) { - errorOut->clear(); - } - if (m_status == Status::Ready) { - return true; // Idempotent: cached state, no re-read of daemon.json. - } - - const auto fail = [this](QString* out, const QString& message) { - if (out != nullptr) { - *out = message; - } - setStatus(Status::Unavailable); - if (m_socket != nullptr) { - m_socket->abort(); - } - return false; - }; - - setStatus(Status::Connecting); - - // Discovery: an existing daemon.json pointing at a live pid, else spawn. - QString error; - if (!discover(&error)) { - if (!spawnDaemon(&error)) { - return fail(errorOut, error); - } - } - - // /ctl with the token in the handshake header, never in-band. - QNetworkRequest controlRequest(QUrl(QStringLiteral("ws://127.0.0.1:%1/ctl") - .arg(m_port))); - controlRequest.setRawHeader("X-Xeno-Token", m_token.toUtf8()); - m_socket->abort(); - m_socket->open(controlRequest); - - // Bounded wait for connected() -> onConnected() -> hello -> list -> Ready. - // onConnected() drives the handshake non-blocking, so this loop only parks - // the calling thread; it never re-enters the socket's own handlers. - QEventLoop handshakeLoop; - QElapsedTimer elapsed; - elapsed.start(); - while (m_status == Status::Connecting && - !elapsed.hasExpired(kHandshakeTimeoutMs)) { - QTimer::singleShot(kWaitPollIntervalMs, &handshakeLoop, - &QEventLoop::quit); - handshakeLoop.exec(); - } - if (m_status != Status::Ready) { - return fail(errorOut, QStringLiteral("daemon handshake did not complete " - "within %1 ms") - .arg(kHandshakeTimeoutMs)); - } - return true; -} - quint64 DaemonLink::request( const QJsonObject& body, std::function done) { @@ -602,9 +615,10 @@ quint64 DaemonLink::request( // REENTRANCY NOTE (read before touching): requestSync() spins a nested event // loop, so OTHER socket events are delivered while it waits — including // frames from this very link. It is therefore legal ONLY from paths this -// class's own handlers cannot re-enter: PtyClient::start() and the quit-time -// batched `list`. Never call it from onTextMessage / onConnected / -// onDisconnected, and never re-enter ensureConnected() from a frame handler. +// class's own handlers cannot re-enter: the quit-time batched `list` +// (refreshPanes), which runs outside any QML handler. Never call it from +// onTextMessage / onConnected / onDisconnected, and never from the pane-spawn +// path — PtyClient drives spawns through the non-blocking request(). bool DaemonLink::requestSync(const QJsonObject& body, QJsonObject* result, QString* errorOut, int timeoutMs) { if (errorOut != nullptr) { @@ -760,28 +774,11 @@ quint64 DaemonLink::requestShutdown() { return request(shutdown, {}); } -bool DaemonLink::waitForDaemonExit(int timeoutMs) const { - // Same bounded-poll shape as spawnDaemon's discovery loop: spin a local - // event loop so GUI-thread timers/sockets stay live while we probe. - // kill(pid, 0) asks only "does the process exist" — no signal delivered. - // An unknown pid (never hello'd, or reset by onDisconnected) counts as - // exited: there is nothing to wait for. - const auto pid = static_cast(m_daemonPid); - if (pid <= 0) { - return true; - } - QElapsedTimer elapsed; - elapsed.start(); - while (!elapsed.hasExpired(timeoutMs)) { - if (kill(pid, 0) != 0 && errno == ESRCH) { - return true; - } - QEventLoop wait; - QTimer::singleShot(kSpawnPollIntervalMs, &wait, &QEventLoop::quit); - wait.exec(); - } - // Still alive at the deadline; the caller proceeds anyway. - return kill(pid, 0) != 0 && errno == ESRCH; +bool DaemonLink::daemonProcessAlive() const { + // kill(pid, 0) asks only "does the process exist"; it delivers no signal. + // pidAlive() also counts EPERM as alive (the process exists, owned by + // someone else). An unknown pid reads as gone: there is nothing to wait for. + return pidAlive(m_daemonPid); } int DaemonLink::alivePaneCount() const { diff --git a/app/support/DaemonLink.h b/app/support/DaemonLink.h index 2e148aa..282a7aa 100644 --- a/app/support/DaemonLink.h +++ b/app/support/DaemonLink.h @@ -21,6 +21,7 @@ #include +#include #include #include #include @@ -28,6 +29,7 @@ #include +class QTimer; class QWebSocket; class DaemonLink : public QObject { @@ -59,15 +61,16 @@ class DaemonLink : public QObject { explicit DaemonLink(QObject* parent = nullptr); ~DaemonLink() override; - // Discover-or-spawn the daemon, open /ctl, send `hello`, fetch the initial - // `list`. BLOCKING with a bounded timeout, because the first pane's - // PtyClient::start() needs an answer before it can return success or failure - // to TerminalQuickItem. Idempotent: a second call returns the cached state - // without re-reading the discovery file. + // Discover-or-spawn the daemon, open /ctl, and complete the hello/list + // handshake. NON-BLOCKING: every wait is a timer, and readiness arrives as + // statusChanged(Ready | Unavailable). Idempotent, and a no-op while a + // connect is already in flight or the link is Ready. // - // NEVER call from inside a socket handler: it spins a local event loop, and - // re-entering the same socket's read path is how you wedge a link. - bool ensureConnected(QString* errorOut = nullptr); + // This is the pane-spawn path's entry point and it must stay free of + // nested event loops. Pane creation runs inside QML signal handlers, and a + // loop there delivers QML deferred deletes (destroy()) while a handler is + // still on the stack, which Qt aborts on. + void connectAsync(); [[nodiscard]] Status status() const { return m_status; } [[nodiscard]] bool isReady() const { return m_status == Status::Ready; } @@ -142,13 +145,11 @@ class DaemonLink : public QObject { // daemon explicitly). quint64 requestShutdown(); - // Bounded poll of the cached daemon pid with the kill(pid, 0) liveness - // probe (same shape as spawnDaemon's poll loop), so the caller can tell a - // genuinely dead daemon from one still tearing down. Returns true when the - // pid is gone (or was never known — nothing to wait for), false on - // timeout. Blocks the calling thread's event loop; PanePool calls it from - // the GUI thread's restart orchestration only. - [[nodiscard]] bool waitForDaemonExit(int timeoutMs) const; + // Non-blocking liveness probe of the cached daemon pid (kill(pid, 0)). + // An unknown pid (never hello'd, or reset on disconnect) reads as gone: + // there is nothing left to wait for. PanePool's restart orchestration polls + // this on a timer instead of blocking the GUI thread. + [[nodiscard]] bool daemonProcessAlive() const; signals: void statusChanged(DaemonLink::Status status); @@ -168,7 +169,17 @@ class DaemonLink : public QObject { void setStatus(Status status); bool discover(QString* errorOut); - bool spawnDaemon(QString* errorOut); + // Resolve + launch the daemon process. Returns as soon as the process is + // started: the discovery poll that follows lives in connectAsync(). Static: + // it touches no link state, only the process launch. + [[nodiscard]] static bool startDaemon(QString* errorOut); + // Open /ctl and arm the handshake watchdog. Discovery data must be set. + void openControlSocket(); + // Report a connect failure: Unavailable + socket abort + warning. + void failConnect(const QString& message); + // Spawn-poll tick: re-run discovery until daemon.json points at a live + // daemon, or the spawn deadline expires. + void pollSpawn(); void onConnected(); void onDisconnected(); void onTextMessage(const QString& message); @@ -201,4 +212,11 @@ class DaemonLink : public QObject { QJsonObject m_syncResult; QString m_syncError; bool m_syncOk = false; + + // Connect watchdogs (see connectAsync). Both are single-shot and setStatus() + // stops them as soon as the status leaves Connecting. + QTimer* m_spawnPollTimer = nullptr; + QTimer* m_handshakeTimer = nullptr; + // Spawn deadline clock, restarted by each connectAsync() attempt. + QElapsedTimer m_spawnElapsed; }; diff --git a/app/terminal/PtyClient.cpp b/app/terminal/PtyClient.cpp index cca53a3..7f17f8f 100644 --- a/app/terminal/PtyClient.cpp +++ b/app/terminal/PtyClient.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -102,6 +103,10 @@ void PtyClient::setDaemon(DaemonLink* link, const QString& paneId) { Qt::UniqueConnection); connect(m_link, &DaemonLink::paneCwdChanged, this, &PtyClient::onPaneCwdChanged, Qt::UniqueConnection); + // Readiness gates both the pending spawn (ensurePane) and the pending + // data connection: start() may be called before the daemon is up. + connect(m_link, &DaemonLink::statusChanged, this, + &PtyClient::onLinkStatusChanged, Qt::UniqueConnection); } } @@ -130,44 +135,59 @@ bool PtyClient::start(const QString& shell, const QStringList& args, return false; } - // 1. Discover-or-spawn the daemon and open /ctl. Blocking with a bounded - // timeout. There is deliberately NO local-pty fallback: two code paths, - // two linkages, double the test surface, for a case that should be a - // visible error. - if (!m_link->ensureConnected(errorOut)) { - if (errorOut != nullptr && errorOut->isEmpty()) { - *errorOut = QStringLiteral("daemon unavailable"); - } - return false; + // The daemon round trip is ASYNCHRONOUS. start() runs inside the pane + // creation path, which itself runs inside QML signal handlers; a nested + // event loop there would deliver QML deferred deletes (destroy()) while a + // handler is still on the stack, which Qt aborts on. So the spec is stored + // and the reattach-or-spawn decision happens in ensurePane(), here if the + // link is already Ready, otherwise from onLinkStatusChanged. + m_shell = shell; + m_args = args; + m_workingDirectory = workingDirectory; + m_env = env; + m_startRequested = true; + m_paneEnsured = false; // a respawn re-runs the reattach-or-spawn decision + ensurePane(); + return true; +} + +void PtyClient::ensurePane() { + if (!m_startRequested || m_paneEnsured || m_link == nullptr) { + return; } - // 2. Reattach-vs-spawn. Pane ids are persisted in session.json - // (AppModel.cpp:2434) and restored verbatim (SplitTree.cpp:312-316), so - // a relaunched app reconstructs the SAME ids the daemon still holds. - // Alive -> REATTACH: no spawn, the shell keeps running uninterrupted. - // Cold attach from seq 0 is the normal relaunch case — the daemon - // clamps to the log floor and the VT rebuilds from what the log still - // holds; the consumer attaches when this pane's tab becomes active. - // Absent or not alive -> SPAWN. + // Not connected yet: discovery/spawn/handshake run in the background, and + // statusChanged(Ready) re-enters here. connectAsync() is idempotent. + if (!m_link->isReady()) { + m_link->connectAsync(); + return; + } + + // Reattach-vs-spawn. Pane ids are persisted in session.json + // (AppModel.cpp:2434) and restored verbatim (SplitTree.cpp:312-316), so a + // relaunched app reconstructs the SAME ids the daemon still holds. + // Alive -> REATTACH: no spawn, the shell keeps running uninterrupted. + // Cold attach from seq 0 is the normal relaunch case — the daemon clamps to + // the log floor and the VT rebuilds from what the log still holds; the + // consumer attaches when this pane's tab becomes active. + // Absent or not alive -> SPAWN. if (m_link->paneAlive(m_paneId)) { refreshSnapshot(); - return true; + m_paneEnsured = true; + startStreamingIfWanted(); + return; } - // 3. Spawn via requestSync. This is the ONE blocking call in the UI, and it - // is legal here precisely because start() cannot be re-entered by the - // link's own socket handlers: no /pane data socket exists yet, and - // DaemonLink's contract pins start() as an allowed requestSync caller. QJsonObject spec; spec.insert(QStringLiteral("paneId"), m_paneId); - spec.insert(QStringLiteral("cwd"), workingDirectory); - spec.insert(QStringLiteral("command"), shell); - spec.insert(QStringLiteral("args"), QJsonArray::fromStringList(args)); + spec.insert(QStringLiteral("cwd"), m_workingDirectory); + spec.insert(QStringLiteral("command"), m_shell); + spec.insert(QStringLiteral("args"), QJsonArray::fromStringList(m_args)); // env crosses VERBATIM. The UI-built env (TERM, COLORTERM=truecolor, // TERM_PROGRAM, NO_COLOR=, SHLVL=1) is interpreted daemon-side by Pty's own // semantics, where an empty value REMOVES the inherited variable — that is // how NO_COLOR= clears the opt-out. Never pre-process or filter here. - spec.insert(QStringLiteral("env"), QJsonArray::fromStringList(env)); + spec.insert(QStringLiteral("env"), QJsonArray::fromStringList(m_env)); // Forwarded for the DAEMON to write after spawning (ownership rule); the // client never writes them itself. spec.insert(QStringLiteral("initialCommands"), @@ -182,16 +202,60 @@ bool PtyClient::start(const QString& shell, const QStringList& args, request.insert(QStringLiteral("op"), QStringLiteral("spawn")); request.insert(QStringLiteral("spec"), spec); - QJsonObject result; - QString err; - if (!m_link->requestSync(request, &result, &err)) { - if (errorOut != nullptr) { - *errorOut = err.isEmpty() ? QStringLiteral("daemon spawn failed") : err; - } - return false; + // The reply lands on the GUI thread. DaemonLink folds a successful spawn + // into its pane cache before invoking this callback. + // + // The callback can outlive this client (a pane deleted while its spawn is + // in flight), so it captures a QPointer, and it kills the shell it created + // in that case: otherwise the daemon would hold a live shell under a pane + // id nothing tracks. + QPointer self(this); + QPointer link(m_link); + const QString paneId = m_paneId; + const quint64 id = m_link->request( + request, + [self, link, paneId](const QJsonObject& /*result*/, + const QString& error) { + if (self.isNull()) { + if (error.isEmpty() && link != nullptr) { + QJsonObject kill; + kill.insert(QStringLiteral("op"), QStringLiteral("kill")); + kill.insert(QStringLiteral("paneId"), paneId); + link->request(kill, {}); + } + return; + } + if (!error.isEmpty()) { + emit self->spawnFailed(error); + return; + } + self->m_paneEnsured = true; + self->refreshSnapshot(); + self->startStreamingIfWanted(); + }); + if (id == 0) { + // The socket went away between the readiness check and the send. Leave + // m_paneEnsured false so a later Ready retries the whole decision. + emit spawnFailed( + QStringLiteral("daemon spawn request could not be sent")); } - refreshSnapshot(); - return true; +} + +void PtyClient::onLinkStatusChanged(DaemonLink::Status status) { + if (status != DaemonLink::Status::Ready) { + return; + } + // A Ready link satisfies both pending desires: the shell (ensurePane) and + // the data connection (startStreamingIfWanted). Both are idempotent. + ensurePane(); + startStreamingIfWanted(); +} + +void PtyClient::startStreamingIfWanted() { + if (!m_wantsStreaming || m_attached || m_socket != nullptr) { + return; + } + attach(resumeSeq()); } void PtyClient::write(const QByteArray& data) { @@ -329,7 +393,9 @@ void PtyClient::setStreaming(bool on) { m_wantsStreaming = true; m_reconnectAttemptsLeft = kReconnectMaxAttempts; m_reconnectDelay = kReconnectBaseDelay; - attach(resumeSeq()); + // attach() no-ops while the link is not Ready (a cold daemon still + // connecting), so the desire is satisfied by onLinkStatusChanged instead. + startStreamingIfWanted(); return; } if (!m_attached && m_socket == nullptr) { diff --git a/app/terminal/PtyClient.h b/app/terminal/PtyClient.h index 127e930..bb11080 100644 --- a/app/terminal/PtyClient.h +++ b/app/terminal/PtyClient.h @@ -30,7 +30,11 @@ #include #include -class DaemonLink; +// Complete type required: onLinkStatusChanged takes DaemonLink::Status, and a +// forward declaration cannot name a nested enum. DaemonLink.h is QtCore/QtNetwork +// only, so this pulls in nothing Quick-side. +#include "DaemonLink.h" + class QTimer; class QWebSocket; @@ -65,15 +69,17 @@ class PtyClient final : public QObject { const QString& initialInput, const QString& title, bool restart, const QStringList& restartArgv); - // Ask the daemon to spawn this pane, or to reattach when the daemon already - // holds a live shell under paneId (the app-relaunch case). Returns false and - // fills `errorOut` when the daemon is unavailable — there is deliberately no - // local-pty fallback. - // // `args` and `env` cross the wire in the spawn spec verbatim; Pty's own env // semantics (empty value REMOVES the inherited variable) are applied // daemon-side. Does NOT write initialCommands: the daemon does, so writing // them here too would run every initial command twice. + // + // Start (or reattach to) this pane's shell. ASYNCHRONOUS: the daemon round + // trip completes on the GUI thread's event loop, and the reattach-or-spawn + // decision runs when the link reports Ready. A false return means only that + // the call could not be accepted at all (no link injected); a spawn that + // fails later surfaces on spawnFailed(). Calling start() again re-runs the + // decision (the respawn path). bool start(const QString& shell, const QStringList& args, const QString& workingDirectory, const QStringList& env, QString* errorOut = nullptr); @@ -148,6 +154,11 @@ class PtyClient final : public QObject { void setStreaming(bool on); signals: + // A requested shell spawn failed. The daemon round trip is asynchronous + // (see start()), so this is how a spawn failure reaches the UI: there is no + // return value left to carry it. Never emitted for the immediate + // configuration errors start() still reports (no link injected). + void spawnFailed(const QString& error); // BYTES arrived and had their 8-byte stamp stripped. Unlike Pty this is a // DIRECT call on the GUI thread (the socket lives there), not a queued // cross-thread emit, so the receiver runs inside the socket's read handler. @@ -180,6 +191,17 @@ class PtyClient final : public QObject { void onPaneExited(const QString& paneId, int code, quint64 seq); void onPaneCwdChanged(const QString& paneId, const QString& cwd); void refreshSnapshot() const; + // Reattach-or-spawn for this pane, once the link is Ready. Idempotent per + // start() call: m_paneEnsured gates the daemon round trip. When the link is + // not Ready yet this asks for a connect and returns; statusChanged(Ready) + // comes back through onLinkStatusChanged. + void ensurePane(); + void onLinkStatusChanged(DaemonLink::Status status); + // attach(resumeSeq()) when the caller wants this pane streaming. attach() + // no-ops while the link is not Ready, so this is safe to call from every + // path that could satisfy the desire (setStreaming, spawn completion, link + // readiness). + void startStreamingIfWanted(); // The seq the next attach should request. m_hasConsumedAny disambiguates the // first attach from a resume: m_lastSeq holds the last CONSUMED seq, so // resuming means m_lastSeq + 1 — but on a first attach that would be 1 and @@ -239,6 +261,19 @@ class PtyClient final : public QObject { QString m_title; bool m_restart = false; QStringList m_restartArgv; + // The pending spawn spec, stored by start() because the daemon round trip is + // asynchronous: the reattach-or-spawn decision runs later, when the link + // reports Ready, and needs the same arguments the caller passed. + QString m_shell; + QStringList m_args; + QString m_workingDirectory; + QStringList m_env; + // start() was called: this pane wants a shell (reattached or spawned). + bool m_startRequested = false; + // The reattach-or-spawn decision for the current start() completed + // successfully. Reset by every start() (a respawn re-runs the decision) and + // by nothing else. + bool m_paneEnsured = false; // Snapshot cache pulled from DaemonLink, so the synchronous accessors never // round trip. Refreshed by list/cwd notifications. mutable qint64 m_cachedPid = 0; diff --git a/app/terminal/TerminalQuickItem.cpp b/app/terminal/TerminalQuickItem.cpp index ccf665c..716e544 100644 --- a/app/terminal/TerminalQuickItem.cpp +++ b/app/terminal/TerminalQuickItem.cpp @@ -398,7 +398,9 @@ void TerminalQuickItem::ensureTerminal() { } // Spawn the shell in a PTY (factored into spawnShell so respawn() can - // re-run just this tail against a live terminal). + // re-run just this tail against a live terminal). The daemon round trip is + // asynchronous, so a false return here means only "not accepted"; a spawn + // failure arrives later on PtyClient::spawnFailed (logged in setDaemon). QString error; if (!spawnShell(&error)) { qWarning() << "xenomorphic: pty start failed:" << error; @@ -450,6 +452,13 @@ void TerminalQuickItem::respawn() { void TerminalQuickItem::setDaemon(DaemonLink* link, const QString& paneId) { m_pty->setDaemon(link, paneId); + // The spawn round trip is asynchronous, so a failure arrives here rather + // than as a false return from start(). Log it with the same message the + // synchronous path used, so a failed pane is still diagnosable. + connect(m_pty, &PtyClient::spawnFailed, this, + [](const QString& error) { + qWarning() << "xenomorphic: pty start failed:" << error; + }); } void TerminalQuickItem::setStreaming(bool on) { m_pty->setStreaming(on); } -- knotmirror 0.0.1