From 82f91755a5da4a8848d8216e5a4dbbbf1f6c1939 Mon Sep 17 00:00:00 2001
From: Levi Neely <lneely@pascom.net>
Date: Thu, 8 Oct 2026 15:20:21 +0200
Subject: [PATCH] plumb: fix crash from deleting a running reader thread on
 shutdown
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Kate crashed on a worker thread inside olliepalette.so near
QArrayData::deallocate. Root cause: ~Plumber did stop(); wait(2000);
delete m_reader; — but the reader can block longer than 2s (NineP connect
waitForConnected(5000), or readExactly's unbounded waitForReadyRead(-1) mid
message). When wait() timed out we deleted a still-running QThread, freeing
its QString/QByteArray members under run() → use-after-free / double free on
the reader thread.

Fixes:
  - ~Plumber never deletes a running thread. stop(); wait(3000); delete only
    if it finished, else hand ownership to the thread via finished->deleteLater
    so it frees only after run() returns.
  - NineP::readExactly waits with a bounded 2s timeout instead of forever, so a
    truncated/stalled message fails cleanly and the reader stays promptly
    stoppable (never force-deleted while blocked).

test_plumb_live still 4/4 (reader start/recv/clean shutdown); 18/18 ctest.
---
 src/plumb/ninep.cpp   |  8 +++++++-
 src/plumb/plumber.cpp | 19 ++++++++++++++-----
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/plumb/ninep.cpp b/src/plumb/ninep.cpp
index f718fd5..abbbcda 100644
--- a/src/plumb/ninep.cpp
+++ b/src/plumb/ninep.cpp
@@ -129,10 +129,16 @@ void NineP::close()
 
 bool NineP::readExactly(char *buf, int n)
 {
+    // Bounded per-chunk wait: a well-formed plumb message arrives whole, so the
+    // bytes are already buffered after the first-byte wait. The timeout only
+    // guards against a truncated/stalled message, in which case we fail rather
+    // than block forever — critical so the reader thread stays promptly
+    // stoppable and is never force-deleted while blocked here.
+    static constexpr int ReadChunkTimeoutMs = 2000;
     int got = 0;
     while (got < n) {
         if (m_sock->bytesAvailable() == 0) {
-            if (!m_sock->waitForReadyRead(-1)) {
+            if (!m_sock->waitForReadyRead(ReadChunkTimeoutMs)) {
                 m_error = QStringLiteral("socket read failed: %1").arg(m_sock->errorString());
                 return false;
             }
diff --git a/src/plumb/plumber.cpp b/src/plumb/plumber.cpp
index b366dc3..c843d43 100644
--- a/src/plumb/plumber.cpp
+++ b/src/plumb/plumber.cpp
@@ -160,12 +160,21 @@ Plumber::Plumber(QObject *parent)
 
 Plumber::~Plumber()
 {
-    if (m_reader) {
-        m_reader->stop();
-        m_reader->wait(2000);
-        delete m_reader;
-        m_reader = nullptr;
+    if (!m_reader) {
+        return;
     }
+    m_reader->stop();
+    // Never delete a still-running QThread: that frees its QString/QByteArray
+    // members out from under run(), crashing on the reader thread
+    // (QArrayData::deallocate). Give it a brief grace to exit cooperatively;
+    // if it has not (e.g. still inside a blocking connect/read), hand ownership
+    // to the thread itself via deleteLater so it frees only after run() returns.
+    if (m_reader->wait(3000)) {
+        delete m_reader;
+    } else {
+        connect(m_reader, &QThread::finished, m_reader, &QObject::deleteLater);
+    }
+    m_reader = nullptr;
 }
 
 bool Plumber::send(const QString &data, const QString &wdir)