plumb: fix crash from deleting a running reader thread on shutdown

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.
This commit is contained in:
Levi Neely 2026-10-08 15:20:21 +02:00
parent b7320a5f23
commit 82f91755a5
2 changed files with 21 additions and 6 deletions

View File

@ -129,10 +129,16 @@ void NineP::close()
bool NineP::readExactly(char *buf, int n) 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; int got = 0;
while (got < n) { while (got < n) {
if (m_sock->bytesAvailable() == 0) { 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()); m_error = QStringLiteral("socket read failed: %1").arg(m_sock->errorString());
return false; return false;
} }

View File

@ -160,12 +160,21 @@ Plumber::Plumber(QObject *parent)
Plumber::~Plumber() Plumber::~Plumber()
{ {
if (m_reader) { if (!m_reader) {
m_reader->stop(); return;
m_reader->wait(2000);
delete m_reader;
m_reader = nullptr;
} }
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) bool Plumber::send(const QString &data, const QString &wdir)