From d7661f0964d6d159465f6ba85f23cdd206274e03 Mon Sep 17 00:00:00 2001
From: Jonas Kvinge <jonas@jkvinge.net>
Date: Sun, 10 May 2020 16:54:14 +0200
Subject: [PATCH] Fix possible crash in album cover fetcher

---
 src/covermanager/albumcoverfetcher.cpp       | 16 ++++++++++++++--
 src/covermanager/albumcoverfetcher.h         |  2 +-
 src/covermanager/albumcoverfetchersearch.cpp | 10 +++++++++-
 src/covermanager/albumcoverfetchersearch.h   |  1 +
 4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/src/covermanager/albumcoverfetcher.cpp b/src/covermanager/albumcoverfetcher.cpp
index 29731f83a..ec99cfc06 100644
--- a/src/covermanager/albumcoverfetcher.cpp
+++ b/src/covermanager/albumcoverfetcher.cpp
@@ -40,8 +40,20 @@ AlbumCoverFetcher::AlbumCoverFetcher(CoverProviders *cover_providers, QObject *p
       network_(network ? network : new NetworkAccessManager(this)),
       next_id_(0),
       request_starter_(new QTimer(this)) {
+
   request_starter_->setInterval(1000);
   connect(request_starter_, SIGNAL(timeout()), SLOT(StartRequests()));
+
+}
+
+AlbumCoverFetcher::~AlbumCoverFetcher() {
+
+  for (AlbumCoverFetcherSearch *search : active_requests_.values()) {
+    search->disconnect();
+    search->deleteLater();
+  }
+  active_requests_.clear();
+
 }
 
 quint64 AlbumCoverFetcher::FetchAlbumCover(const QString &artist, const QString &album, const QString &title, bool fetchall) {
@@ -125,8 +137,8 @@ void AlbumCoverFetcher::StartRequests() {
 
 void AlbumCoverFetcher::SingleSearchFinished(const quint64 request_id, const CoverSearchResults results) {
 
+  if (!active_requests_.contains(request_id)) return;
   AlbumCoverFetcherSearch *search = active_requests_.take(request_id);
-  if (!search) return;
 
   search->deleteLater();
   emit SearchFinished(request_id, results, search->statistics());
@@ -135,8 +147,8 @@ void AlbumCoverFetcher::SingleSearchFinished(const quint64 request_id, const Cov
 
 void AlbumCoverFetcher::SingleCoverFetched(const quint64 request_id, const QUrl &cover_url, const QImage &image) {
 
+  if (!active_requests_.contains(request_id)) return;
   AlbumCoverFetcherSearch *search = active_requests_.take(request_id);
-  if (!search) return;
 
   search->deleteLater();
   emit AlbumCoverFetched(request_id, cover_url, image, search->statistics());
diff --git a/src/covermanager/albumcoverfetcher.h b/src/covermanager/albumcoverfetcher.h
index 12509218b..c35c1d87f 100644
--- a/src/covermanager/albumcoverfetcher.h
+++ b/src/covermanager/albumcoverfetcher.h
@@ -89,7 +89,7 @@ class AlbumCoverFetcher : public QObject {
 
  public:
   explicit AlbumCoverFetcher(CoverProviders *cover_providers, QObject *parent = nullptr, QNetworkAccessManager *network = 0);
-  virtual ~AlbumCoverFetcher() {}
+  ~AlbumCoverFetcher();
 
   static const int kMaxConcurrentRequests;
 
diff --git a/src/covermanager/albumcoverfetchersearch.cpp b/src/covermanager/albumcoverfetchersearch.cpp
index c7158dc31..afd70cb2e 100644
--- a/src/covermanager/albumcoverfetchersearch.cpp
+++ b/src/covermanager/albumcoverfetchersearch.cpp
@@ -61,6 +61,11 @@ AlbumCoverFetcherSearch::AlbumCoverFetcherSearch(
 
 }
 
+AlbumCoverFetcherSearch::~AlbumCoverFetcherSearch() {
+  pending_requests_.clear();
+  Cancel();
+}
+
 void AlbumCoverFetcherSearch::TerminateSearch() {
 
   for (quint64 id : pending_requests_.keys()) {
@@ -205,6 +210,7 @@ void AlbumCoverFetcherSearch::FetchMoreImages() {
 
 void AlbumCoverFetcherSearch::ProviderCoverFetchFinished(QNetworkReply *reply) {
 
+  disconnect(reply, &QNetworkReply::finished, this, nullptr);
   reply->deleteLater();
 
   if (!pending_image_loads_.contains(reply)) return;
@@ -280,7 +286,7 @@ void AlbumCoverFetcherSearch::SendBestImage() {
     cover_url = best_image.first.image_url;
     image = best_image.second;
 
-    qLog(Info) << "Using " << best_image.first.image_url << "from" << best_image.first.provider << "with score" << best_image.first.score;
+    qLog(Info) << "Using" << best_image.first.image_url << "from" << best_image.first.provider << "with score" << best_image.first.score;
 
     statistics_.chosen_images_by_provider_[best_image.first.provider]++;
     statistics_.chosen_images_++;
@@ -304,7 +310,9 @@ void AlbumCoverFetcherSearch::Cancel() {
   }
   else if (!pending_image_loads_.isEmpty()) {
     for (QNetworkReply *reply : pending_image_loads_.keys()) {
+      disconnect(reply, &QNetworkReply::finished, this, nullptr);
       reply->abort();
+      reply->deleteLater();
     }
     pending_image_loads_.clear();
   }
diff --git a/src/covermanager/albumcoverfetchersearch.h b/src/covermanager/albumcoverfetchersearch.h
index c6d9a059c..8091228e2 100644
--- a/src/covermanager/albumcoverfetchersearch.h
+++ b/src/covermanager/albumcoverfetchersearch.h
@@ -49,6 +49,7 @@ class AlbumCoverFetcherSearch : public QObject {
 
  public:
   explicit AlbumCoverFetcherSearch(const CoverSearchRequest &request, QNetworkAccessManager *network, QObject *parent);
+  ~AlbumCoverFetcherSearch();
 
   void Start(CoverProviders *cover_providers);