Improve Download Manager subsystem

PR #20257.

src/base/net/downloadhandlerimpl.cpp

@@ -1,6 +1,6 @@
 /*
  * Bittorrent Client using Qt and libtorrent.
- * Copyright (C) 2015, 2018  Vladimir Golovnev <glassez@yandex.ru>
+ * Copyright (C) 2015-2024  Vladimir Golovnev <glassez@yandex.ru>
  * Copyright (C) 2006  Christophe Dumez <chris@qbittorrent.org>
  *
  * This program is free software; you can redistribute it and/or
@@ -75,10 +75,17 @@ Net::DownloadHandlerImpl::DownloadHandlerImpl(DownloadManager *manager
 
 void Net::DownloadHandlerImpl::cancel()
 {
-    if (m_reply)
+    if (m_isFinished)
+        return;
+
+    if (m_reply && m_reply->isRunning())
     {
         m_reply->abort();
     }
+    else if (m_redirectionHandler)
+    {
+        m_redirectionHandler->cancel();
+    }
     else
     {
         setError(errorCodeToString(QNetworkReply::OperationCanceledError));
@@ -90,6 +97,7 @@ void Net::DownloadHandlerImpl::assignNetworkReply(QNetworkReply *reply)
 {
     Q_ASSERT(reply);
     Q_ASSERT(!m_reply);
+    Q_ASSERT(!m_isFinished);
 
     m_reply = reply;
     m_reply->setParent(this);
@@ -98,6 +106,11 @@ void Net::DownloadHandlerImpl::assignNetworkReply(QNetworkReply *reply)
     connect(m_reply, &QNetworkReply::finished, this, &DownloadHandlerImpl::processFinishedDownload);
 }
 
+QNetworkReply *Net::DownloadHandlerImpl::assignedNetworkReply() const
+{
+    return m_reply;
+}
+
 // Returns original url
 QString Net::DownloadHandlerImpl::url() const
 {
@@ -230,10 +243,10 @@ void Net::DownloadHandlerImpl::handleRedirection(const QUrl &newUrl)
         return;
     }
 
-    auto *redirected = static_cast<DownloadHandlerImpl *>(
+    m_redirectionHandler = static_cast<DownloadHandlerImpl *>(
             m_manager->download(DownloadRequest(m_downloadRequest).url(newUrlString), useProxy()));
-    redirected->m_redirectionCount = m_redirectionCount + 1;
+    m_redirectionHandler->m_redirectionCount = m_redirectionCount + 1;
-    connect(redirected, &DownloadHandlerImpl::finished, this, [this](const DownloadResult &result)
+    connect(m_redirectionHandler, &DownloadHandlerImpl::finished, this, [this](const DownloadResult &result)
     {
         m_result = result;
         m_result.url = url();
@@ -249,6 +262,7 @@ void Net::DownloadHandlerImpl::setError(const QString &error)
 
 void Net::DownloadHandlerImpl::finish()
 {
+    m_isFinished = true;
     emit finished(m_result);
 }
 

src/base/net/downloadhandlerimpl.h

@@ -1,6 +1,6 @@
 /*
  * Bittorrent Client using Qt and libtorrent.
- * Copyright (C) 2015, 2018  Vladimir Golovnev <glassez@yandex.ru>
+ * Copyright (C) 2015-2024  Vladimir Golovnev <glassez@yandex.ru>
  * Copyright (C) 2006  Christophe Dumez <chris@qbittorrent.org>
  *
  * This program is free software; you can redistribute it and/or
@@ -53,6 +53,7 @@ namespace Net
         bool useProxy() const;
 
         void assignNetworkReply(QNetworkReply *reply);
+        QNetworkReply *assignedNetworkReply() const;
 
     private:
         void processFinishedDownload();
@@ -65,9 +66,11 @@ namespace Net
 
         DownloadManager *m_manager = nullptr;
         QNetworkReply *m_reply = nullptr;
+        DownloadHandlerImpl *m_redirectionHandler = nullptr;
         const DownloadRequest m_downloadRequest;
         const bool m_useProxy = false;
         short m_redirectionCount = 0;
         DownloadResult m_result;
+        bool m_isFinished = false;
     };
 }

src/base/net/downloadmanager.cpp

@@ -1,6 +1,6 @@
 /*
  * Bittorrent Client using Qt and libtorrent.
- * Copyright (C) 2015, 2018  Vladimir Golovnev <glassez@yandex.ru>
+ * Copyright (C) 2015-2024  Vladimir Golovnev <glassez@yandex.ru>
  * Copyright (C) 2006  Christophe Dumez <chris@qbittorrent.org>
  *
  * This program is free software; you can redistribute it and/or
@@ -40,6 +40,7 @@
 #include <QNetworkReply>
 #include <QNetworkRequest>
 #include <QSslError>
+#include <QTimer>
 #include <QUrl>
 
 #include "base/global.h"
@@ -156,25 +157,31 @@ Net::DownloadManager *Net::DownloadManager::instance()
 Net::DownloadHandler *Net::DownloadManager::download(const DownloadRequest &downloadRequest, const bool useProxy)
 {
     // Process download request
-    const ServiceID id = ServiceID::fromURL(downloadRequest.url());
+    const auto serviceID = ServiceID::fromURL(downloadRequest.url());
-    const bool isSequentialService = m_sequentialServices.contains(id);
+    const bool isSequentialService = m_sequentialServices.contains(serviceID);
 
     auto *downloadHandler = new DownloadHandlerImpl(this, downloadRequest, useProxy);
-    connect(downloadHandler, &DownloadHandler::finished, downloadHandler, &QObject::deleteLater);
+    connect(downloadHandler, &DownloadHandler::finished, this, [this, serviceID, downloadHandler]
-    connect(downloadHandler, &QObject::destroyed, this, [this, id, downloadHandler]()
     {
-        m_waitingJobs[id].removeOne(downloadHandler);
+        if (!downloadHandler->assignedNetworkReply())
+        {
+            // DownloadHandler was finished (canceled) before QNetworkReply was assigned,
+            // so it's still in the queue. Just remove it from there.
+            m_waitingJobs[serviceID].removeOne(downloadHandler);
+        }
+
+        downloadHandler->deleteLater();
     });
 
-    if (isSequentialService && m_busyServices.contains(id))
+    if (isSequentialService && m_busyServices.contains(serviceID))
     {
-        m_waitingJobs[id].enqueue(downloadHandler);
+        m_waitingJobs[serviceID].enqueue(downloadHandler);
     }
     else
     {
         qDebug("Downloading %s...", qUtf8Printable(downloadRequest.url()));
         if (isSequentialService)
-            m_busyServices.insert(id);
+            m_busyServices.insert(serviceID);
         processRequest(downloadHandler);
     }
 
@@ -259,21 +266,19 @@ void Net::DownloadManager::applyProxySettings()
         m_proxy.setCapabilities(m_proxy.capabilities() & ~QNetworkProxy::HostNameLookupCapability);
 }
 
-void Net::DownloadManager::handleDownloadFinished(DownloadHandlerImpl *finishedHandler)
+void Net::DownloadManager::processWaitingJobs(const ServiceID &serviceID)
 {
-    const ServiceID id = ServiceID::fromURL(finishedHandler->url());
+    const auto waitingJobsIter = m_waitingJobs.find(serviceID);
-    const auto waitingJobsIter = m_waitingJobs.find(id);
     if ((waitingJobsIter == m_waitingJobs.end()) || waitingJobsIter.value().isEmpty())
     {
         // No more waiting jobs for given ServiceID
-        m_busyServices.remove(id);
+        m_busyServices.remove(serviceID);
         return;
     }
 
     auto *handler = waitingJobsIter.value().dequeue();
     qDebug("Downloading %s...", qUtf8Printable(handler->url()));
     processRequest(handler);
-    handler->disconnect(this);
 }
 
 void Net::DownloadManager::processRequest(DownloadHandlerImpl *downloadHandler)
@@ -302,9 +307,9 @@ void Net::DownloadManager::processRequest(DownloadHandlerImpl *downloadHandler)
     request.setTransferTimeout();
 
     QNetworkReply *reply = m_networkManager->get(request);
-    connect(reply, &QNetworkReply::finished, this, [this, downloadHandler]
+    connect(reply, &QNetworkReply::finished, this, [this, serviceID = ServiceID::fromURL(downloadHandler->url())]
     {
-        handleDownloadFinished(downloadHandler);
+        QTimer::singleShot(0, this, [this, serviceID] { processWaitingJobs(serviceID); });
     });
     downloadHandler->assignNetworkReply(reply);
 }

src/base/net/downloadmanager.h

@@ -1,6 +1,6 @@
 /*
  * Bittorrent Client using Qt and libtorrent.
- * Copyright (C) 2015-2023  Vladimir Golovnev <glassez@yandex.ru>
+ * Copyright (C) 2015-2024  Vladimir Golovnev <glassez@yandex.ru>
  * Copyright (C) 2006  Christophe Dumez <chris@qbittorrent.org>
  *
  * This program is free software; you can redistribute it and/or
@@ -153,7 +153,7 @@ namespace Net
         explicit DownloadManager(QObject *parent = nullptr);
 
         void applyProxySettings();
-        void handleDownloadFinished(DownloadHandlerImpl *finishedHandler);
+        void processWaitingJobs(const ServiceID &serviceID);
         void processRequest(DownloadHandlerImpl *downloadHandler);
 
         static DownloadManager *m_instance;