Files
vdm/gui/src/rpc/RpcConnection.cpp
T
samiandClaude Sonnet 5 c6f864ea30 gui: fix RpcClient double-free on stop() and the 1000-row list cap
Both found live while building gui/tests/dod's DoD harness, not from reading
the code — see that commit for how.

RpcClient::stop() left conn_ dangling after joining the worker thread: the
thread's own finish() flushes the DeferredDelete stop()'s
connect(&thread_, &QThread::finished, conn_, &QObject::deleteLater) already
posted, so conn_ is gone by the time stop() returns, but nothing cleared the
pointer. Any caller that calls stop() and later lets the client destruct
(the harness's own client.stop() at shutdown; also plain, correct API usage)
hit a double-free in the destructor's leftover `delete conn_`. Caught by
ASan on the very first run that actually exercised the stop-then-destroy
path.

requestInitialList() also called download.list with a hardcoded
`{"limit": 1000}`, silently capping the table at 1000 rows no matter how
many the daemon actually has — download.list.schema.json's own description
says "the GUI pages", not "the GUI takes it all in one call". The
scroll-60fps DoD gate refused to run against mockd --tasks 10000 rather
than "pass" against a 1000-row table, which is what surfaced it.
requestInitialList() now pages (5000 per call, the schema's own max) until
`total` is satisfied, then resets the model once with everything.

Separately: RpcConnection's session.subscribe list never included
event.settings.changed or event.grabber.progress, even though RpcClient has
carried signals for both since the Options/Grabber work — session.subscribe
"replaces the previous selection" and "nothing is delivered until this is
called", so both events were being silently dropped by any real daemon that
enforces the subscription (mockd does; verified live with a second
subscribed client actually receiving event.settings.changed after this
fix, round-tripped through a real veloxd's settings.set). GrabberWizard's
5 s poll fallback is exactly why this went unnoticed until now — it covered
for the missing push the whole time.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01NSCdCWFXBTSBBK3MzWtJiC
2026-09-12 21:27:28 +04:00

208 lines
6.7 KiB
C++

#include "rpc/RpcConnection.hpp"
#include <QJsonArray>
#include <QJsonDocument>
#include <QLocalSocket>
#include <QTimer>
#include "rpc/Protocol.hpp"
#include "velox_proto.hpp"
namespace velox::gui::rpc {
namespace {
constexpr int kStateDisconnected = int(ConnectionState::Disconnected);
constexpr int kStateConnecting = int(ConnectionState::Connecting);
constexpr int kStateHandshaking = int(ConnectionState::Handshaking);
constexpr int kStateConnected = int(ConnectionState::Connected);
constexpr int kStateReconnecting = int(ConnectionState::Reconnecting);
QJsonObject makeErrorEnvelope(int code, const QString &message) {
QJsonObject err{{"code", code}, {"message", message}};
return QJsonObject{{"jsonrpc", "2.0"}, {"error", err}};
}
QString contractProtocolVersion() {
const auto v = velox::proto::kProtocolVersion;
return QString::fromUtf8(v.data(), static_cast<qsizetype>(v.size()));
}
} // namespace
RpcConnection::RpcConnection(QString socketPath, QObject *parent)
: QObject(parent), socketPath_(std::move(socketPath)) {
socket_ = new QLocalSocket(this);
connect(socket_, &QLocalSocket::connected, this, &RpcConnection::onConnected);
connect(socket_, &QLocalSocket::disconnected, this, &RpcConnection::onDisconnected);
connect(socket_, &QLocalSocket::errorOccurred, this, &RpcConnection::onErrorOccurred);
connect(socket_, &QLocalSocket::readyRead, this, &RpcConnection::onReadyRead);
reconnectTimer_ = new QTimer(this);
reconnectTimer_->setSingleShot(true);
connect(reconnectTimer_, &QTimer::timeout, this, &RpcConnection::attemptReconnect);
}
RpcConnection::~RpcConnection() = default;
void RpcConnection::start() {
if (wantConnected_) {
return;
}
wantConnected_ = true;
backoffMs_ = kMinBackoffMs;
setState(kStateConnecting);
socket_->connectToServer(socketPath_);
}
void RpcConnection::stop() {
wantConnected_ = false;
reconnectTimer_->stop();
socket_->abort();
rxBuffer_.clear();
// Fail any in-flight callbacks so RpcClient does not leak them.
const auto tags = pendingTags_.values();
pendingTags_.clear();
for (const quint64 tag : tags) {
emit callCompleted(tag, makeErrorEnvelope(-1, QStringLiteral("connection stopped")));
}
setState(kStateDisconnected);
}
void RpcConnection::sendCall(quint64 tag, const QString &methodName, const QJsonObject &params) {
if (state_ != kStateConnected) {
emit callCompleted(tag, makeErrorEnvelope(-1, QStringLiteral("not connected")));
return;
}
const qint64 id = nextRpcId_++;
pendingTags_.insert(id, tag);
sendRaw(id, methodName, params);
}
void RpcConnection::onConnected() {
rxBuffer_.clear();
beginHandshake();
}
void RpcConnection::beginHandshake() {
setState(kStateHandshaking);
sendRaw(kHelloId, QString::fromLatin1(method::kSessionHello),
QJsonObject{{"clientType", "gui"},
{"clientName", QStringLiteral("velox-gui 0.1.0")},
{"protocolVersion", contractProtocolVersion()}});
}
// A dropped connection surfaces as disconnected(); a *failed* connect attempt surfaces as
// errorOccurred() with no disconnected() to follow. Both funnel here, and the isActive()
// guard means the two firing together for one failure only arms one timer.
void RpcConnection::scheduleReconnect() {
if (!wantConnected_) {
setState(kStateDisconnected);
return;
}
if (reconnectTimer_->isActive()) {
return;
}
setState(kStateReconnecting);
reconnectTimer_->start(backoffMs_);
backoffMs_ = qMin(backoffMs_ * 2, kMaxBackoffMs);
}
void RpcConnection::onDisconnected() {
scheduleReconnect();
}
void RpcConnection::onErrorOccurred() {
scheduleReconnect();
}
void RpcConnection::attemptReconnect() {
if (!wantConnected_) {
return;
}
setState(kStateConnecting);
socket_->abort();
socket_->connectToServer(socketPath_);
}
void RpcConnection::onReadyRead() {
rxBuffer_ += socket_->readAll();
int nl = rxBuffer_.indexOf('\n');
while (nl != -1) {
const QByteArray line = rxBuffer_.left(nl).trimmed();
rxBuffer_.remove(0, nl + 1);
nl = rxBuffer_.indexOf('\n');
if (line.isEmpty()) {
continue;
}
QJsonParseError perr{};
const QJsonDocument doc = QJsonDocument::fromJson(line, &perr);
if (perr.error != QJsonParseError::NoError || !doc.isObject()) {
continue; // a malformed frame is dropped, never thrown
}
dispatchFrame(doc.object());
}
}
void RpcConnection::dispatchFrame(const QJsonObject &frame) {
const QJsonValue idVal = frame.value("id");
const bool isResponse = frame.contains("result") || frame.contains("error");
if (idVal.isDouble() && isResponse) {
const qint64 id = static_cast<qint64>(idVal.toDouble());
if (id == kHelloId) {
if (frame.contains("error")) {
socket_->abort(); // version mismatch or refused — bounce and retry
return;
}
sendRaw(
kSubscribeId, QString::fromLatin1(method::kSessionSubscribe),
QJsonObject{
{"events", QJsonArray{event::kTaskAdded, event::kTaskRemoved, event::kTaskState,
event::kTaskProgress, event::kSpeedGlobal, event::kNotify,
event::kSettingsChanged, event::kGrabberProgress}}});
return;
}
if (id == kSubscribeId) {
if (frame.contains("error")) {
socket_->abort();
return;
}
backoffMs_ = kMinBackoffMs;
setState(kStateConnected);
return;
}
const auto it = pendingTags_.constFind(id);
if (it != pendingTags_.cend()) {
const quint64 tag = it.value();
pendingTags_.erase(it);
emit callCompleted(tag, frame);
}
return;
}
const QString methodName = frame.value("method").toString();
if (!methodName.isEmpty()) {
emit notificationReceived(methodName, frame.value("params").toObject());
}
}
void RpcConnection::sendRaw(qint64 rpcId, const QString &methodName, const QJsonObject &params) {
const QJsonObject req{{"jsonrpc", "2.0"},
{"id", static_cast<double>(rpcId)},
{"method", methodName},
{"params", params}};
QByteArray line = QJsonDocument(req).toJson(QJsonDocument::Compact);
line += '\n';
socket_->write(line);
}
void RpcConnection::setState(int state) {
if (state_ == state) {
return;
}
state_ = state;
emit stateChanged(state);
}
} // namespace velox::gui::rpc