From 506f2a295cd1b0b3affb963a6050ae4fa9869c42 Mon Sep 17 00:00:00 2001 From: Martchus Date: Sun, 5 May 2024 12:48:01 +0200 Subject: [PATCH] Delete QNetworkReply correctly when destroying SyncthingFileModel When disconnecting the callback we need to destroy the QNetworkReply manually (as this is no longer done by the usual handler). This will also close any network connections (if still open). --- syncthingconnector/syncthingconnection.h | 11 +++++-- .../syncthingconnection_requests.cpp | 30 ++++++++++------- syncthingmodel/syncthingfilemodel.cpp | 33 +++++++++++-------- syncthingmodel/syncthingfilemodel.h | 2 +- 4 files changed, 46 insertions(+), 30 deletions(-) diff --git a/syncthingconnector/syncthingconnection.h b/syncthingconnector/syncthingconnection.h index 28f7d6f..f833d3c 100644 --- a/syncthingconnector/syncthingconnection.h +++ b/syncthingconnector/syncthingconnection.h @@ -137,6 +137,11 @@ public: SyncthingConnectionLoggingFlags loggingFlags = SyncthingConnectionLoggingFlags::FromEnvironment, QObject *parent = nullptr); ~SyncthingConnection() override; + struct QueryResult { + QNetworkReply *reply = nullptr; + QMetaObject::Connection connection; + }; + // getter/setter for const QString &syncthingUrl() const; void setSyncthingUrl(const QString &url); @@ -275,10 +280,10 @@ public Q_SLOTS: public: // methods to GET or POST information from/to Syncthing (non-slots) - QMetaObject::Connection browse(const QString &dirId, const QString &prefix, int level, + QueryResult browse(const QString &dirId, const QString &prefix, int level, std::function> &&, QString &&)> &&callback); - QMetaObject::Connection ignores(const QString &dirId, std::function &&callback); - QMetaObject::Connection setIgnores(const QString &dirId, const SyncthingIgnores &ignores, std::function &&callback); + QueryResult ignores(const QString &dirId, std::function &&callback); + QueryResult setIgnores(const QString &dirId, const SyncthingIgnores &ignores, std::function &&callback); Q_SIGNALS: void newConfig(const QJsonObject &rawConfig); diff --git a/syncthingconnector/syncthingconnection_requests.cpp b/syncthingconnector/syncthingconnection_requests.cpp index 3589594..cd2e9ce 100644 --- a/syncthingconnector/syncthingconnection_requests.cpp +++ b/syncthingconnector/syncthingconnection_requests.cpp @@ -1590,7 +1590,7 @@ void SyncthingConnection::readRevert() * to consume results of a specific request. Errors are still reported via the error() signal so there's no extra error handling * required. Note that in case of an error \a callback is invoked with a non-empty string containing the error message. */ -QMetaObject::Connection SyncthingConnection::browse(const QString &dirId, const QString &prefix, int levels, +SyncthingConnection::QueryResult SyncthingConnection::browse(const QString &dirId, const QString &prefix, int levels, std::function> &&, QString &&)> &&callback) { auto query = QUrlQuery(); @@ -1601,9 +1601,11 @@ QMetaObject::Connection SyncthingConnection::browse(const QString &dirId, const if (levels > 0) { query.addQueryItem(QStringLiteral("levels"), QString::number(levels)); } - return QObject::connect( - requestData(QStringLiteral("db/browse"), query), &QNetworkReply::finished, this, - [this, id = dirId, l = levels, cb = std::move(callback)]() mutable { readBrowse(id, l, std::move(cb)); }, Qt::QueuedConnection); + auto *const reply = requestData(QStringLiteral("db/browse"), query); + return { reply, + QObject::connect( + reply, &QNetworkReply::finished, this, + [this, id = dirId, l = levels, cb = std::move(callback)]() mutable { readBrowse(id, l, std::move(cb)); }, Qt::QueuedConnection) }; } /*! @@ -1614,13 +1616,15 @@ QMetaObject::Connection SyncthingConnection::browse(const QString &dirId, const * to consume results of a specific request. Errors are still reported via the error() signal so there's no extra error handling * required. Note that in case of an error \a callback is invoked with a non-empty string containing the error message. */ -QMetaObject::Connection SyncthingConnection::ignores(const QString &dirId, std::function &&callback) +SyncthingConnection::QueryResult SyncthingConnection::ignores(const QString &dirId, std::function &&callback) { auto query = QUrlQuery(); query.addQueryItem(QStringLiteral("folder"), formatQueryItem(dirId)); - return QObject::connect( - requestData(QStringLiteral("db/ignores"), query), &QNetworkReply::finished, this, - [this, id = dirId, cb = std::move(callback)]() mutable { readIgnores(id, std::move(cb)); }, Qt::QueuedConnection); + auto *const reply = requestData(QStringLiteral("db/ignores"), query); + return { reply, + QObject::connect( + reply, &QNetworkReply::finished, this, [this, id = dirId, cb = std::move(callback)]() mutable { readIgnores(id, std::move(cb)); }, + Qt::QueuedConnection) }; } /*! @@ -1631,7 +1635,7 @@ QMetaObject::Connection SyncthingConnection::ignores(const QString &dirId, std:: * to consume results of a specific request. Errors are still reported via the error() signal so there's no extra error handling * required. Note that in case of an error \a callback is invoked with a non-empty string containing the error message. */ -QMetaObject::Connection SyncthingConnection::setIgnores( +SyncthingConnection::QueryResult SyncthingConnection::setIgnores( const QString &dirId, const SyncthingIgnores &ignores, std::function &&callback) { auto query = QUrlQuery(); @@ -1644,9 +1648,11 @@ QMetaObject::Connection SyncthingConnection::setIgnores( jsonObj.insert(QLatin1String("ignore"), ignoreArray); auto jsonDoc = QJsonDocument(); jsonDoc.setObject(jsonObj); - return QObject::connect( - postData(QStringLiteral("db/ignores"), query, jsonDoc.toJson(QJsonDocument::Compact)), &QNetworkReply::finished, this, - [this, id = dirId, cb = std::move(callback)]() mutable { readSetIgnores(id, std::move(cb)); }, Qt::QueuedConnection); + auto *const reply = postData(QStringLiteral("db/ignores"), query, jsonDoc.toJson(QJsonDocument::Compact)); + return { reply, + QObject::connect( + reply, &QNetworkReply::finished, this, [this, id = dirId, cb = std::move(callback)]() mutable { readSetIgnores(id, std::move(cb)); }, + Qt::QueuedConnection) }; } /// \cond diff --git a/syncthingmodel/syncthingfilemodel.cpp b/syncthingmodel/syncthingfilemodel.cpp index fb65349..a04c5b6 100644 --- a/syncthingmodel/syncthingfilemodel.cpp +++ b/syncthingmodel/syncthingfilemodel.cpp @@ -10,6 +10,7 @@ #include #include +#include #include using namespace std; @@ -46,25 +47,28 @@ SyncthingFileModel::SyncthingFileModel(SyncthingConnection &connection, const Sy m_root->size = dir.globalStats.bytes; m_root->type = SyncthingItemType::Directory; m_fetchQueue.append(QString()); - m_connection.browse(m_dirId, QString(), 1, [this](std::vector> &&items, QString &&errorMessage) { - Q_UNUSED(errorMessage) + m_pendingRequest + = m_connection.browse(m_dirId, QString(), 1, [this](std::vector> &&items, QString &&errorMessage) { + m_pendingRequest.reply = nullptr; + Q_UNUSED(errorMessage) - m_fetchQueue.removeAll(QString()); - if (items.empty()) { - return; - } - const auto last = items.size() - 1; - beginInsertRows(index(0, 0), 0, last < std::numeric_limits::max() ? static_cast(last) : std::numeric_limits::max()); - populatePath(QString(), items); - m_root->children = std::move(items); - m_root->childrenPopulated = true; - endInsertRows(); - }); + m_fetchQueue.removeAll(QString()); + if (items.empty()) { + return; + } + const auto last = items.size() - 1; + beginInsertRows(index(0, 0), 0, last < std::numeric_limits::max() ? static_cast(last) : std::numeric_limits::max()); + populatePath(QString(), items); + m_root->children = std::move(items); + m_root->childrenPopulated = true; + endInsertRows(); + }); } SyncthingFileModel::~SyncthingFileModel() { - QObject::disconnect(m_pendingRequest); + QObject::disconnect(m_pendingRequest.connection); + delete m_pendingRequest.reply; } QHash SyncthingFileModel::roleNames() const @@ -384,6 +388,7 @@ void SyncthingFileModel::processFetchQueue() const auto &path = m_fetchQueue.front(); m_pendingRequest = m_connection.browse( m_dirId, path, 1, [this, p = path](std::vector> &&items, QString &&errorMessage) mutable { + m_pendingRequest.reply = nullptr; Q_UNUSED(errorMessage) m_fetchQueue.removeAll(p); diff --git a/syncthingmodel/syncthingfilemodel.h b/syncthingmodel/syncthingfilemodel.h index c4bd2d1..81e2afd 100644 --- a/syncthingmodel/syncthingfilemodel.h +++ b/syncthingmodel/syncthingfilemodel.h @@ -55,7 +55,7 @@ private: QString m_dirId; QString m_localPath; QStringList m_fetchQueue; - QMetaObject::Connection m_pendingRequest; + SyncthingConnection::QueryResult m_pendingRequest; std::unique_ptr m_root; };