diff --git a/src/common/NetworkPrivate.cpp b/src/common/NetworkPrivate.cpp index 6f78a58e1..9709ba98b 100644 --- a/src/common/NetworkPrivate.cpp +++ b/src/common/NetworkPrivate.cpp @@ -125,6 +125,11 @@ void loadUncached(const std::shared_ptr &data) } auto handleReply = [data, reply]() mutable { + if (data->hasCaller_ && !data->caller_.get()) + { + return; + } + // TODO(pajlada): A reply was received, kill the timeout timer if (reply->error() != QNetworkReply::NetworkError::NoError) { @@ -162,7 +167,6 @@ void loadUncached(const std::shared_ptr &data) if (data->executeConcurrently || isGuiThread()) { handleReply(); - delete worker; } else @@ -206,11 +210,22 @@ void loadCached(const std::shared_ptr &data) // XXX: If outcome is Failure, we should invalidate the cache file // somehow/somewhere /*auto outcome =*/ + if (data->hasCaller_ && !data->caller_.get()) + { + return; + } data->onSuccess_(result); } else { - postToThread([data, result]() { data->onSuccess_(result); }); + postToThread([data, result]() { + if (data->hasCaller_ && !data->caller_.get()) + { + return; + } + + data->onSuccess_(result); + }); } } } // namespace chatterino diff --git a/src/common/NetworkPrivate.hpp b/src/common/NetworkPrivate.hpp index dca19ff4c..a9c315167 100644 --- a/src/common/NetworkPrivate.hpp +++ b/src/common/NetworkPrivate.hpp @@ -1,6 +1,7 @@ #pragma once #include "common/NetworkCommon.hpp" +#include "util/QObjectRef.hpp" #include #include @@ -32,7 +33,8 @@ struct NetworkData { ~NetworkData(); QNetworkRequest request_; - const QObject *caller_ = nullptr; + bool hasCaller_{}; + QObjectRef caller_; bool useQuickLoadCache_{}; bool executeConcurrently{}; diff --git a/src/common/NetworkRequest.cpp b/src/common/NetworkRequest.cpp index adab1e6be..901635252 100644 --- a/src/common/NetworkRequest.cpp +++ b/src/common/NetworkRequest.cpp @@ -50,7 +50,11 @@ NetworkRequest NetworkRequest::type(NetworkRequestType newRequestType) && NetworkRequest NetworkRequest::caller(const QObject *caller) && { - this->data->caller_ = caller; + // Caller must be in gui thread + assert(caller->thread() == qApp->thread()); + + this->data->caller_ = const_cast(caller); + this->data->hasCaller_ = true; return std::move(*this); } @@ -144,6 +148,9 @@ void NetworkRequest::execute() this->data->useQuickLoadCache_ = false; } + // Can not have a caller and be concurrent at the same time. + assert(!(this->data->caller_ && this->data->executeConcurrently)); + load(std::move(this->data)); } diff --git a/src/common/NetworkRequest.hpp b/src/common/NetworkRequest.hpp index f82be761b..501138f0e 100644 --- a/src/common/NetworkRequest.hpp +++ b/src/common/NetworkRequest.hpp @@ -44,6 +44,10 @@ public: NetworkRequest payload(const QByteArray &payload) &&; NetworkRequest cache() &&; + /// NetworkRequest makes sure that the `caller` object still exists when the + /// callbacks are executed. Cannot be used with concurrent() since we can't + /// make sure that the object doesn't get deleted while the callback is + /// running. NetworkRequest caller(const QObject *caller) &&; NetworkRequest header(const char *headerName, const char *value) &&; NetworkRequest header(const char *headerName, const QByteArray &value) &&; diff --git a/src/messages/Image.cpp b/src/messages/Image.cpp index 9a1740152..1b9c0258d 100644 --- a/src/messages/Image.cpp +++ b/src/messages/Image.cpp @@ -327,7 +327,7 @@ int Image::height() const assertInGuiThread(); if (auto pixmap = this->frames_->first()) - return pixmap->height() * this->scale_; + return int(pixmap->height() * this->scale_); else return 16; } @@ -336,7 +336,6 @@ void Image::load() { NetworkRequest(this->url().string) .concurrent() - .caller(&this->object_) .cache() .onSuccess([that = this, weak = weakOf(this)](auto result) -> Outcome { auto shared = weak.lock(); diff --git a/src/providers/twitch/PartialTwitchUser.cpp b/src/providers/twitch/PartialTwitchUser.cpp index 5c3ceeec1..d482311f6 100644 --- a/src/providers/twitch/PartialTwitchUser.cpp +++ b/src/providers/twitch/PartialTwitchUser.cpp @@ -31,11 +31,6 @@ void PartialTwitchUser::getId(std::function successCallback, { assert(!this->username_.isEmpty()); - if (caller == nullptr) - { - caller = QThread::currentThread(); - } - NetworkRequest("https://api.twitch.tv/kraken/users?login=" + this->username_) .caller(caller) diff --git a/src/providers/twitch/TwitchChannel.cpp b/src/providers/twitch/TwitchChannel.cpp index 2ec8823fb..5034caa14 100644 --- a/src/providers/twitch/TwitchChannel.cpp +++ b/src/providers/twitch/TwitchChannel.cpp @@ -607,7 +607,6 @@ void TwitchChannel::loadRecentMessages() } NetworkRequest(Env::get().recentMessagesApiUrl.arg(this->getName())) - .caller(QThread::currentThread()) .concurrent() .onSuccess([weak = weakOf(this)](auto result) -> Outcome { auto shared = weak.lock(); diff --git a/src/util/QObjectRef.hpp b/src/util/QObjectRef.hpp index 1065e8e7d..cf8ad69ee 100644 --- a/src/util/QObjectRef.hpp +++ b/src/util/QObjectRef.hpp @@ -69,7 +69,7 @@ private: this->t_ = other; } - T *t_{}; + std::atomic t_{}; QMetaObject::Connection conn_; }; } // namespace chatterino diff --git a/src/widgets/dialogs/LogsPopup.cpp b/src/widgets/dialogs/LogsPopup.cpp index 3263f0dfd..e2af4f46c 100644 --- a/src/widgets/dialogs/LogsPopup.cpp +++ b/src/widgets/dialogs/LogsPopup.cpp @@ -93,7 +93,7 @@ void LogsPopup::getLogviewerLogs(const QString &roomID) NetworkRequest(url) .caller(this) - .onError([this](int errorCode) { + .onError([this](int /*errorCode*/) { this->getOverrustleLogs(); return true; }) @@ -138,7 +138,7 @@ void LogsPopup::getOverrustleLogs() NetworkRequest(url) .caller(this) - .onError([this](int errorCode) { + .onError([this](int /*errorCode*/) { auto box = new QMessageBox( QMessageBox::Information, "Error getting logs", "No logs could be found for channel " + this->channelName_);