From 95ebb6a492c5dd3a9d7dd8f8be4c7b86d60c4905 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Tue, 7 Feb 2023 10:18:21 +0100 Subject: [PATCH 1/9] Build a new curl "handle" at everyrequest instead of reusing the same one. --- src/aria2.cpp | 47 +++++++++++++++++++++++++++-------------------- src/aria2.h | 3 --- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/aria2.cpp b/src/aria2.cpp index 78f541128..ee7f3b8ba 100644 --- a/src/aria2.cpp +++ b/src/aria2.cpp @@ -24,7 +24,7 @@ #define LOG_ARIA_ERROR() \ { \ std::cerr << "ERROR: aria2 RPC request failed. (" << res << ")." << std::endl; \ - std::cerr << (m_curlErrorBuffer[0] ? m_curlErrorBuffer.get() : curl_easy_strerror(res)) << std::endl; \ + std::cerr << (curlErrorBuffer[0] ? curlErrorBuffer : curl_easy_strerror(res)) << std::endl; \ } namespace kiwix { @@ -32,9 +32,7 @@ namespace kiwix { Aria2::Aria2(): mp_aria(nullptr), m_port(42042), - m_secret(getNewRpcSecret()), - m_curlErrorBuffer(new char[CURL_ERROR_SIZE]), - mp_curl(nullptr) + m_secret(getNewRpcSecret()) { m_downloadDir = getDataDirectory(); makeDirectory(m_downloadDir); @@ -91,26 +89,28 @@ Aria2::Aria2(): launchCmd.append(cmd).append(" "); } mp_aria = Subprocess::run(callCmd); - mp_curl = curl_easy_init(); - curl_easy_setopt(mp_curl, CURLOPT_URL, "http://localhost/rpc"); - curl_easy_setopt(mp_curl, CURLOPT_PORT, m_port); - curl_easy_setopt(mp_curl, CURLOPT_POST, 1L); - curl_easy_setopt(mp_curl, CURLOPT_ERRORBUFFER, m_curlErrorBuffer.get()); + CURL* p_curl = curl_easy_init(); + char curlErrorBuffer[CURL_ERROR_SIZE]; + + curl_easy_setopt(p_curl, CURLOPT_URL, "http://localhost/rpc"); + curl_easy_setopt(p_curl, CURLOPT_PORT, m_port); + curl_easy_setopt(p_curl, CURLOPT_POST, 1L); + curl_easy_setopt(p_curl, CURLOPT_ERRORBUFFER, curlErrorBuffer); int watchdog = 50; while(--watchdog) { sleep(10); - m_curlErrorBuffer[0] = 0; - auto res = curl_easy_perform(mp_curl); + curlErrorBuffer[0] = 0; + auto res = curl_easy_perform(p_curl); if (res == CURLE_OK) { break; } else if (watchdog == 1) { LOG_ARIA_ERROR(); } } + curl_easy_cleanup(p_curl); if (!watchdog) { - curl_easy_cleanup(mp_curl); throw std::runtime_error("Cannot connect to aria2c rpc. Aria2c launch cmd : " + launchCmd); } } @@ -118,7 +118,6 @@ Aria2::Aria2(): Aria2::~Aria2() { std::unique_lock lock(m_lock); - curl_easy_cleanup(mp_curl); } void Aria2::close() @@ -142,17 +141,25 @@ std::string Aria2::doRequest(const MethodCall& methodCall) long response_code; { std::unique_lock lock(m_lock); - curl_easy_setopt(mp_curl, CURLOPT_POSTFIELDSIZE, requestContent.size()); - curl_easy_setopt(mp_curl, CURLOPT_POSTFIELDS, requestContent.c_str()); - curl_easy_setopt(mp_curl, CURLOPT_WRITEFUNCTION, &write_callback_to_iss); - curl_easy_setopt(mp_curl, CURLOPT_WRITEDATA, &outStream); - m_curlErrorBuffer[0] = 0; - res = curl_easy_perform(mp_curl); + char curlErrorBuffer[CURL_ERROR_SIZE]; + CURL* p_curl = curl_easy_init(); + curl_easy_setopt(p_curl, CURLOPT_URL, "http://localhost/rpc"); + curl_easy_setopt(p_curl, CURLOPT_PORT, m_port); + curl_easy_setopt(p_curl, CURLOPT_POST, 1L); + curl_easy_setopt(p_curl, CURLOPT_ERRORBUFFER, curlErrorBuffer); + curl_easy_setopt(p_curl, CURLOPT_POSTFIELDSIZE, requestContent.size()); + curl_easy_setopt(p_curl, CURLOPT_POSTFIELDS, requestContent.c_str()); + curl_easy_setopt(p_curl, CURLOPT_WRITEFUNCTION, &write_callback_to_iss); + curl_easy_setopt(p_curl, CURLOPT_WRITEDATA, &outStream); + curlErrorBuffer[0] = 0; + res = curl_easy_perform(p_curl); if (res != CURLE_OK) { LOG_ARIA_ERROR(); + curl_easy_cleanup(p_curl); throw std::runtime_error("Cannot perform request"); } - curl_easy_getinfo(mp_curl, CURLINFO_RESPONSE_CODE, &response_code); + curl_easy_getinfo(p_curl, CURLINFO_RESPONSE_CODE, &response_code); + curl_easy_cleanup(p_curl); } auto responseContent = outStream.str(); diff --git a/src/aria2.h b/src/aria2.h index 47a3f826a..ee4b58eb6 100644 --- a/src/aria2.h +++ b/src/aria2.h @@ -24,10 +24,7 @@ class Aria2 int m_port; std::string m_secret; std::string m_downloadDir; - std::unique_ptr m_curlErrorBuffer; - CURL* mp_curl; std::mutex m_lock; - std::string doRequest(const MethodCall& methodCall); public: From 1aa8521e15af6d1fba5582f8adbe7aa78d1129e6 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Tue, 7 Feb 2023 10:22:07 +0100 Subject: [PATCH 2/9] Remove the lock. As we now build a new request handle for every request, we don't need a lock. libcurl itself is thread safe as long as we don't share a handle. --- src/aria2.cpp | 42 +++++++++++++++++------------------------- src/aria2.h | 4 +--- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/src/aria2.cpp b/src/aria2.cpp index ee7f3b8ba..e58b2bd67 100644 --- a/src/aria2.cpp +++ b/src/aria2.cpp @@ -115,11 +115,6 @@ Aria2::Aria2(): } } -Aria2::~Aria2() -{ - std::unique_lock lock(m_lock); -} - void Aria2::close() { saveSession(); @@ -139,28 +134,25 @@ std::string Aria2::doRequest(const MethodCall& methodCall) std::stringstream outStream; CURLcode res; long response_code; - { - std::unique_lock lock(m_lock); - char curlErrorBuffer[CURL_ERROR_SIZE]; - CURL* p_curl = curl_easy_init(); - curl_easy_setopt(p_curl, CURLOPT_URL, "http://localhost/rpc"); - curl_easy_setopt(p_curl, CURLOPT_PORT, m_port); - curl_easy_setopt(p_curl, CURLOPT_POST, 1L); - curl_easy_setopt(p_curl, CURLOPT_ERRORBUFFER, curlErrorBuffer); - curl_easy_setopt(p_curl, CURLOPT_POSTFIELDSIZE, requestContent.size()); - curl_easy_setopt(p_curl, CURLOPT_POSTFIELDS, requestContent.c_str()); - curl_easy_setopt(p_curl, CURLOPT_WRITEFUNCTION, &write_callback_to_iss); - curl_easy_setopt(p_curl, CURLOPT_WRITEDATA, &outStream); - curlErrorBuffer[0] = 0; - res = curl_easy_perform(p_curl); - if (res != CURLE_OK) { - LOG_ARIA_ERROR(); - curl_easy_cleanup(p_curl); - throw std::runtime_error("Cannot perform request"); - } - curl_easy_getinfo(p_curl, CURLINFO_RESPONSE_CODE, &response_code); + char curlErrorBuffer[CURL_ERROR_SIZE]; + CURL* p_curl = curl_easy_init(); + curl_easy_setopt(p_curl, CURLOPT_URL, "http://localhost/rpc"); + curl_easy_setopt(p_curl, CURLOPT_PORT, m_port); + curl_easy_setopt(p_curl, CURLOPT_POST, 1L); + curl_easy_setopt(p_curl, CURLOPT_ERRORBUFFER, curlErrorBuffer); + curl_easy_setopt(p_curl, CURLOPT_POSTFIELDSIZE, requestContent.size()); + curl_easy_setopt(p_curl, CURLOPT_POSTFIELDS, requestContent.c_str()); + curl_easy_setopt(p_curl, CURLOPT_WRITEFUNCTION, &write_callback_to_iss); + curl_easy_setopt(p_curl, CURLOPT_WRITEDATA, &outStream); + curlErrorBuffer[0] = 0; + res = curl_easy_perform(p_curl); + if (res != CURLE_OK) { + LOG_ARIA_ERROR(); curl_easy_cleanup(p_curl); + throw std::runtime_error("Cannot perform request"); } + curl_easy_getinfo(p_curl, CURLINFO_RESPONSE_CODE, &response_code); + curl_easy_cleanup(p_curl); auto responseContent = outStream.str(); if (response_code != 200) { diff --git a/src/aria2.h b/src/aria2.h index ee4b58eb6..f6cd633b8 100644 --- a/src/aria2.h +++ b/src/aria2.h @@ -12,7 +12,6 @@ #include "xmlrpc.h" #include -#include #include namespace kiwix { @@ -24,12 +23,11 @@ class Aria2 int m_port; std::string m_secret; std::string m_downloadDir; - std::mutex m_lock; std::string doRequest(const MethodCall& methodCall); public: Aria2(); - virtual ~Aria2(); + virtual ~Aria2() = default; void close(); std::string addUri(const std::vector& uri, const std::vector>& options = {}); From d1fe1b89ae570665f43f2a0fb97f4c38f1c06d0f Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Tue, 7 Feb 2023 10:37:27 +0100 Subject: [PATCH 3/9] Do not automatically update the status of existing Download. User may already have a pointer to the `Download` and it is not protected against concurrent access. We could update the status of new created `Download` as by definition, no one have a pointer on it. But it better to not do it neither : - For consistency - Because the first call on update status may be long on windows (because of file preallocation). It is better to not block the downloader for that. --- src/downloader.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/downloader.cpp b/src/downloader.cpp index 9bffad385..cd342b3e6 100644 --- a/src/downloader.cpp +++ b/src/downloader.cpp @@ -180,23 +180,20 @@ Download* Downloader::startDownload(const std::string& uri, const std::vectorupdateStatus(true); return m_knownDownloads.at(did).get(); } catch(std::exception& e) { for (auto gid : mp_aria->tellActive()) { if (gid == did) { m_knownDownloads[gid] = std::unique_ptr(new Download(mp_aria, gid)); - m_knownDownloads.at(gid).get()->updateStatus(true); return m_knownDownloads[gid].get(); } } for (auto gid : mp_aria->tellWaiting()) { if (gid == did) { m_knownDownloads[gid] = std::unique_ptr(new Download(mp_aria, gid)); - m_knownDownloads.at(gid).get()->updateStatus(true); return m_knownDownloads[gid].get(); } - } + } throw e; } } From 52ae5c3a5ffc34b15f4eb8049811851c8c099e05 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Tue, 7 Feb 2023 10:51:42 +0100 Subject: [PATCH 4/9] Make Downloader thread safe. --- include/downloader.h | 4 +++- src/downloader.cpp | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/include/downloader.h b/include/downloader.h index 4cd8337ad..726049ba0 100644 --- a/include/downloader.h +++ b/include/downloader.h @@ -25,6 +25,7 @@ #include #include #include +#include namespace kiwix { @@ -95,10 +96,11 @@ class Downloader Download* startDownload(const std::string& uri, const std::vector>& options = {}); Download* getDownload(const std::string& did); - size_t getNbDownload() { return m_knownDownloads.size(); } + size_t getNbDownload(); std::vector getDownloadIds(); private: + std::mutex m_lock; std::map> m_knownDownloads; std::shared_ptr mp_aria; }; diff --git a/src/downloader.cpp b/src/downloader.cpp index cd342b3e6..f6b13a21a 100644 --- a/src/downloader.cpp +++ b/src/downloader.cpp @@ -156,6 +156,7 @@ void Downloader::close() } std::vector Downloader::getDownloadIds() { + std::unique_lock lock(m_lock); std::vector ret; for(auto& p:m_knownDownloads) { ret.push_back(p.first); @@ -165,6 +166,7 @@ std::vector Downloader::getDownloadIds() { Download* Downloader::startDownload(const std::string& uri, const std::vector>& options) { + std::unique_lock lock(m_lock); for (auto& p: m_knownDownloads) { auto& d = p.second; auto& uris = d->getUris(); @@ -179,6 +181,7 @@ Download* Downloader::startDownload(const std::string& uri, const std::vector lock(m_lock); try { return m_knownDownloads.at(did).get(); } catch(std::exception& e) { @@ -198,4 +201,9 @@ Download* Downloader::getDownload(const std::string& did) } } +size_t Downloader::getNbDownload() { + std::unique_lock lock(m_lock); + return m_knownDownloads.size(); +} + } From 0e612de4d1b14153d9d24bc4e797411363c44dbf Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Tue, 7 Feb 2023 10:55:57 +0100 Subject: [PATCH 5/9] Make `Downloader` return shared_ptr instead of raw pointer. This is dangerous by nature to return raw pointer on internal data. --- include/downloader.h | 6 +++--- src/downloader.cpp | 20 ++++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/include/downloader.h b/include/downloader.h index 726049ba0..d2cebe8e0 100644 --- a/include/downloader.h +++ b/include/downloader.h @@ -93,15 +93,15 @@ class Downloader void close(); - Download* startDownload(const std::string& uri, const std::vector>& options = {}); - Download* getDownload(const std::string& did); + std::shared_ptr startDownload(const std::string& uri, const std::vector>& options = {}); + std::shared_ptr getDownload(const std::string& did); size_t getNbDownload(); std::vector getDownloadIds(); private: std::mutex m_lock; - std::map> m_knownDownloads; + std::map> m_knownDownloads; std::shared_ptr mp_aria; }; } diff --git a/src/downloader.cpp b/src/downloader.cpp index f6b13a21a..02c75f6fb 100644 --- a/src/downloader.cpp +++ b/src/downloader.cpp @@ -164,37 +164,37 @@ std::vector Downloader::getDownloadIds() { return ret; } -Download* Downloader::startDownload(const std::string& uri, const std::vector>& options) +std::shared_ptr Downloader::startDownload(const std::string& uri, const std::vector>& options) { std::unique_lock lock(m_lock); for (auto& p: m_knownDownloads) { auto& d = p.second; auto& uris = d->getUris(); if (std::find(uris.begin(), uris.end(), uri) != uris.end()) - return d.get(); + return d; } std::vector uris = {uri}; auto gid = mp_aria->addUri(uris, options); - m_knownDownloads[gid] = std::unique_ptr(new Download(mp_aria, gid)); - return m_knownDownloads[gid].get(); + m_knownDownloads[gid] = std::make_shared(mp_aria, gid); + return m_knownDownloads[gid]; } -Download* Downloader::getDownload(const std::string& did) +std::shared_ptr Downloader::getDownload(const std::string& did) { std::unique_lock lock(m_lock); try { - return m_knownDownloads.at(did).get(); + return m_knownDownloads.at(did); } catch(std::exception& e) { for (auto gid : mp_aria->tellActive()) { if (gid == did) { - m_knownDownloads[gid] = std::unique_ptr(new Download(mp_aria, gid)); - return m_knownDownloads[gid].get(); + m_knownDownloads[gid] = std::make_shared(mp_aria, gid); + return m_knownDownloads[gid]; } } for (auto gid : mp_aria->tellWaiting()) { if (gid == did) { - m_knownDownloads[gid] = std::unique_ptr(new Download(mp_aria, gid)); - return m_knownDownloads[gid].get(); + m_knownDownloads[gid] = std::make_shared(mp_aria, gid); + return m_knownDownloads[gid]; } } throw e; From 18b7b5f277a7734c024f7b36b5ad9e0584740b1c Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Tue, 7 Feb 2023 11:00:27 +0100 Subject: [PATCH 6/9] Mark constant methods as const. --- include/downloader.h | 24 ++++++++++++------------ src/downloader.cpp | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/include/downloader.h b/include/downloader.h index d2cebe8e0..e5bcff7a1 100644 --- a/include/downloader.h +++ b/include/downloader.h @@ -58,15 +58,15 @@ class Download { void pauseDownload(); void resumeDownload(); void cancelDownload(); - StatusResult getStatus() { return m_status; } - std::string getDid() { return m_did; } - std::string getFollowedBy() { return m_followedBy; } - uint64_t getTotalLength() { return m_totalLength; } - uint64_t getCompletedLength() { return m_completedLength; } - uint64_t getDownloadSpeed() { return m_downloadSpeed; } - uint64_t getVerifiedLength() { return m_verifiedLength; } - std::string getPath() { return m_path; } - std::vector& getUris() { return m_uris; } + StatusResult getStatus() const { return m_status; } + const std::string& getDid() const { return m_did; } + const std::string& getFollowedBy() const { return m_followedBy; } + uint64_t getTotalLength() const { return m_totalLength; } + uint64_t getCompletedLength() const { return m_completedLength; } + uint64_t getDownloadSpeed() const { return m_downloadSpeed; } + uint64_t getVerifiedLength() const { return m_verifiedLength; } + const std::string& getPath() const { return m_path; } + const std::vector& getUris() const { return m_uris; } protected: std::shared_ptr mp_aria; @@ -96,11 +96,11 @@ class Downloader std::shared_ptr startDownload(const std::string& uri, const std::vector>& options = {}); std::shared_ptr getDownload(const std::string& did); - size_t getNbDownload(); - std::vector getDownloadIds(); + size_t getNbDownload() const; + std::vector getDownloadIds() const; private: - std::mutex m_lock; + mutable std::mutex m_lock; std::map> m_knownDownloads; std::shared_ptr mp_aria; }; diff --git a/src/downloader.cpp b/src/downloader.cpp index 02c75f6fb..1163f69dc 100644 --- a/src/downloader.cpp +++ b/src/downloader.cpp @@ -155,7 +155,7 @@ void Downloader::close() mp_aria->close(); } -std::vector Downloader::getDownloadIds() { +std::vector Downloader::getDownloadIds() const { std::unique_lock lock(m_lock); std::vector ret; for(auto& p:m_knownDownloads) { @@ -201,7 +201,7 @@ std::shared_ptr Downloader::getDownload(const std::string& did) } } -size_t Downloader::getNbDownload() { +size_t Downloader::getNbDownload() const { std::unique_lock lock(m_lock); return m_knownDownloads.size(); } From f239f2de184f6f4adc1069635528fdbb9c6778a6 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Tue, 7 Feb 2023 11:22:36 +0100 Subject: [PATCH 7/9] Add documentation. --- include/downloader.h | 106 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/include/downloader.h b/include/downloader.h index e5bcff7a1..b27a54159 100644 --- a/include/downloader.h +++ b/include/downloader.h @@ -44,6 +44,14 @@ class AriaError : public std::runtime_error { }; +/** + * A representation of a current download. + * + * `Download` is not thread safe. User must care to not call method on a + * same download from different threads. + * However, it is safe to use different `Download`s from different threads. + */ + class Download { public: typedef enum { K_ACTIVE, K_WAITING, K_PAUSED, K_ERROR, K_COMPLETE, K_REMOVED, K_UNKNOWN } StatusResult; @@ -54,18 +62,87 @@ class Download { : mp_aria(p_aria), m_status(K_UNKNOWN), m_did(did) {}; + + /** + * Update the status of the download. + * + * This call make an aria rpc call and is blocking. + * Some download (started with a metalink) are in fact several downloads. + * - A first one to download the metadlink. + * - A second one to download the real file. + * + * By default, `Download` track only the first download. So status may appear + * as COMPLETE even if the real file downloading is still running. + * By passing true to follow, `Dowload` will detect that and will track the + * second download instead. + * + * `getFoo` methods are based on the last statusUpdate. + */ void updateStatus(bool follow=false); + + /** + * Pause the download (and call updateStatus) + */ void pauseDownload(); + + /** + * Resume the download (and call updateStatus) + */ void resumeDownload(); + + /** + * Cancel the download. + * + * A canceled downlod cannot be resume and updateStatus does nothing. + * However, you can still get information based on the last known information. + */ void cancelDownload(); + + /* + * Get the status of the download. + */ StatusResult getStatus() const { return m_status; } + + /* + * Get the id of the download. + */ const std::string& getDid() const { return m_did; } + + /* + * Get the id of the "second" download. + * + * Set only if the "first" download is a metalink and is complete. + */ const std::string& getFollowedBy() const { return m_followedBy; } + + /* + * Get the total length of the download. + */ uint64_t getTotalLength() const { return m_totalLength; } + + /* + * Get the completed length of the download. + */ uint64_t getCompletedLength() const { return m_completedLength; } + + /* + * Get the download speed of the download. + */ uint64_t getDownloadSpeed() const { return m_downloadSpeed; } + + /* + * Get the verified length of the download. + */ uint64_t getVerifiedLength() const { return m_verifiedLength; } + + /* + * Get the path (local file) of the download. + */ const std::string& getPath() const { return m_path; } + + /* + * Get the download uris of the download. + */ const std::vector& getUris() const { return m_uris; } protected: @@ -84,6 +161,9 @@ class Download { /** * A tool to download things. * + * A Downloader manages `Download` using aria2 in the background. + * `Downloader` is threadsafe. + * However, the returned `Download`s are NOT threadsafe. */ class Downloader { @@ -93,10 +173,36 @@ class Downloader void close(); + /** + * Start a new download. + * + * This method is thread safe and return a pointer to a newly created `Download`. + * User should call `update` on the returned `Download` to have an accurate status. + * + * @param uri: The uri of the thing to download. + * @param options: A series of pair to pass to aria. + * @return: The newly created Download. + */ std::shared_ptr startDownload(const std::string& uri, const std::vector>& options = {}); + + /** + * Get a download corrsponding to a download id (did) + * User should call `update` on the returned `Download` to have an accurate status. + * + * @param did: The download id to search for. + * @return: The Download corresponding to did. + * @throw: Throw std::out_of_range if did is not found. + */ std::shared_ptr getDownload(const std::string& did); + /** + * Get the number of downloads currently managed. + */ size_t getNbDownload() const; + + /** + * Get the ids of the managed downloads. + */ std::vector getDownloadIds() const; private: From 2c3b7409aa5c8ff5b5b60263fd8117b70d992be2 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Tue, 7 Feb 2023 11:25:34 +0100 Subject: [PATCH 8/9] Remove the default value of follow parameter in `updateStatus`. `false` is a pretty bad default value as most user want to track the real download. By removing the default value, we force user to make a choice. We could have change the default value to true but it would have been a silent API change and we don't want that. --- include/downloader.h | 11 ++++++----- src/downloader.cpp | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/include/downloader.h b/include/downloader.h index b27a54159..f385823c9 100644 --- a/include/downloader.h +++ b/include/downloader.h @@ -71,14 +71,15 @@ class Download { * - A first one to download the metadlink. * - A second one to download the real file. * - * By default, `Download` track only the first download. So status may appear - * as COMPLETE even if the real file downloading is still running. - * By passing true to follow, `Dowload` will detect that and will track the - * second download instead. + * If `follow` is true, updateStatus tries to detect that and tracks + * the second download when the first one is finished. + * By passing false to `follow`, `Download` will only track the first download. * * `getFoo` methods are based on the last statusUpdate. + * + * @param follow: Do we have to follow following downloads. */ - void updateStatus(bool follow=false); + void updateStatus(bool follow); /** * Pause the download (and call updateStatus) diff --git a/src/downloader.cpp b/src/downloader.cpp index 1163f69dc..2035cecf3 100644 --- a/src/downloader.cpp +++ b/src/downloader.cpp @@ -130,7 +130,7 @@ Downloader::Downloader() : try { for (auto gid : mp_aria->tellActive()) { m_knownDownloads[gid] = std::unique_ptr(new Download(mp_aria, gid)); - m_knownDownloads[gid]->updateStatus(); + m_knownDownloads[gid]->updateStatus(false); } } catch (std::exception& e) { std::cerr << "aria2 tellActive failed : " << e.what() << std::endl; @@ -138,7 +138,7 @@ Downloader::Downloader() : try { for (auto gid : mp_aria->tellWaiting()) { m_knownDownloads[gid] = std::unique_ptr(new Download(mp_aria, gid)); - m_knownDownloads[gid]->updateStatus(); + m_knownDownloads[gid]->updateStatus(false); } } catch (std::exception& e) { std::cerr << "aria2 tellWaiting failed : " << e.what() << std::endl; From 1ba588272c91e8ec59847311e1cb083be6a3a316 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 8 Feb 2023 12:06:51 +0100 Subject: [PATCH 9/9] Get `Waiting` downloads before `Active` ones. `Waiting` can become `Active` while we are getting the downloads. We may have rare case where we miss a download if we get `Active` before `Waiting`. --- src/downloader.cpp | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/downloader.cpp b/src/downloader.cpp index 2035cecf3..d874ad899 100644 --- a/src/downloader.cpp +++ b/src/downloader.cpp @@ -127,14 +127,6 @@ void Download::cancelDownload() Downloader::Downloader() : mp_aria(new Aria2()) { - try { - for (auto gid : mp_aria->tellActive()) { - m_knownDownloads[gid] = std::unique_ptr(new Download(mp_aria, gid)); - m_knownDownloads[gid]->updateStatus(false); - } - } catch (std::exception& e) { - std::cerr << "aria2 tellActive failed : " << e.what() << std::endl; - } try { for (auto gid : mp_aria->tellWaiting()) { m_knownDownloads[gid] = std::unique_ptr(new Download(mp_aria, gid)); @@ -143,6 +135,16 @@ Downloader::Downloader() : } catch (std::exception& e) { std::cerr << "aria2 tellWaiting failed : " << e.what() << std::endl; } + try { + for (auto gid : mp_aria->tellActive()) { + if( m_knownDownloads.find(gid) == m_knownDownloads.end()) { + m_knownDownloads[gid] = std::unique_ptr(new Download(mp_aria, gid)); + m_knownDownloads[gid]->updateStatus(false); + } + } + } catch (std::exception& e) { + std::cerr << "aria2 tellActive failed : " << e.what() << std::endl; + } } /* Destructor */ @@ -185,13 +187,13 @@ std::shared_ptr Downloader::getDownload(const std::string& did) try { return m_knownDownloads.at(did); } catch(std::exception& e) { - for (auto gid : mp_aria->tellActive()) { + for (auto gid : mp_aria->tellWaiting()) { if (gid == did) { m_knownDownloads[gid] = std::make_shared(mp_aria, gid); return m_knownDownloads[gid]; } } - for (auto gid : mp_aria->tellWaiting()) { + for (auto gid : mp_aria->tellActive()) { if (gid == did) { m_knownDownloads[gid] = std::make_shared(mp_aria, gid); return m_knownDownloads[gid];