From 0f93fa109ccf3fbceef7b295622e33cfba4370ba Mon Sep 17 00:00:00 2001 From: Jari Sundell Date: Thu, 12 Dec 2024 22:59:48 +0900 Subject: [PATCH] Fixed curl stack shutdown with active downloads. (#1338) Fixed curl stack shutdown with active downloads. --- .clang-format | 2 +- .clang-tidy | 2 +- src/core/curl_get.cc | 3 ++ src/core/curl_socket.cc | 41 ++------------ src/core/curl_stack.cc | 107 ++++++++++++++----------------------- src/core/curl_stack.h | 87 ++++++++++-------------------- src/core/download_store.cc | 36 ------------- src/core/download_store.h | 36 ------------- src/core/manager.cc | 47 +++------------- 9 files changed, 82 insertions(+), 279 deletions(-) diff --git a/.clang-format b/.clang-format index 2f0b1678..6a41c804 100644 --- a/.clang-format +++ b/.clang-format @@ -24,7 +24,7 @@ AlignConsecutiveDeclarations: AlignConsecutiveMacros: Enabled: true AlignConsecutiveAssignments: - Enabled: true + Enabled: true IncludeCategories: - Regex: "^(config|globals)\\.h" diff --git a/.clang-tidy b/.clang-tidy index 8f8ffabe..015b26da 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -11,6 +11,6 @@ CheckOptions: - key: readability-identifier-naming.PrivateMemberPrefix value: m_ - key: readability-identifier-naming.PrivateMemberCase - value: camelBack + value: lower_case - key: readability-identifier-naming.ClassConstantCase value: lower_case diff --git a/src/core/curl_get.cc b/src/core/curl_get.cc index 25aef829..a99fc716 100644 --- a/src/core/curl_get.cc +++ b/src/core/curl_get.cc @@ -67,6 +67,9 @@ CurlGet::start() { if (m_stream == NULL) throw torrent::internal_error("Tried to call CurlGet::start without a valid output stream."); + if (!m_stack->is_running()) + return; + m_handle = curl_easy_init(); if (m_handle == NULL) diff --git a/src/core/curl_socket.cc b/src/core/curl_socket.cc index 03b04e5f..ae4c4d39 100644 --- a/src/core/curl_socket.cc +++ b/src/core/curl_socket.cc @@ -1,39 +1,3 @@ -// rTorrent - BitTorrent client -// Copyright (C) 2005-2008, Jari Sundell -// -// This program is free software; you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation; either version 2 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program; if not, write to the Free Software -// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA -// -// In addition, as a special exception, the copyright holders give -// permission to link the code of portions of this program with the -// OpenSSL library under certain conditions as described in each -// individual source file, and distribute linked combinations -// including the two. -// -// You must obey the GNU General Public License in all respects for -// all of the code used other than OpenSSL. If you modify file(s) -// with this exception, you may extend this exception to your version -// of the file(s), but you are not obligated to do so. If you do not -// wish to do so, delete this exception statement from your version. -// If you delete this exception statement from all source files in the -// program, then also delete it here. -// -// Contact: Jari Sundell -// -// Skomakerveien 33 -// 3185 Skoppum, NORWAY - #include "config.h" #include @@ -55,6 +19,9 @@ CurlSocket::receive_socket(void* easy_handle, curl_socket_t fd, int what, void* CurlStack* stack = (CurlStack*)userp; CurlSocket* socket = (CurlSocket*)socketp; + if (!stack->is_running()) + return 0; + if (what == CURL_POLL_REMOVE) { // We also probably need the special code here as we're not // guaranteed that the fd will be closed, afaik. @@ -75,7 +42,7 @@ CurlSocket::receive_socket(void* easy_handle, curl_socket_t fd, int what, void* // No interface for libcurl to signal when it's interested in error events. // Assume that hence it must always be interested in them. torrent::main_thread()->poll()->insert_error(socket); - } + } if (what == CURL_POLL_NONE || what == CURL_POLL_OUT) torrent::main_thread()->poll()->remove_read(socket); diff --git a/src/core/curl_stack.cc b/src/core/curl_stack.cc index 5668d281..0bf1a543 100644 --- a/src/core/curl_stack.cc +++ b/src/core/curl_stack.cc @@ -1,39 +1,3 @@ -// rTorrent - BitTorrent client -// Copyright (C) 2005-2011, Jari Sundell -// -// This program is free software; you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation; either version 2 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program; if not, write to the Free Software -// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA -// -// In addition, as a special exception, the copyright holders give -// permission to link the code of portions of this program with the -// OpenSSL library under certain conditions as described in each -// individual source file, and distribute linked combinations -// including the two. -// -// You must obey the GNU General Public License in all respects for -// all of the code used other than OpenSSL. If you modify file(s) -// with this exception, you may extend this exception to your version -// of the file(s), but you are not obligated to do so. If you do not -// wish to do so, delete this exception statement from your version. -// If you delete this exception statement from all source files in the -// program, then also delete it here. -// -// Contact: Jari Sundell -// -// Skomakerveien 33 -// 3185 Skoppum, NORWAY - #include "config.h" #include @@ -47,15 +11,9 @@ namespace core { -CurlStack::CurlStack() : - m_handle((void*)curl_multi_init()), - m_active(0), - m_maxActive(32), - m_ssl_verify_host(true), - m_ssl_verify_peer(true), - m_dns_timeout(60) { - - m_taskTimeout.slot() = std::bind(&CurlStack::receive_timeout, this); +CurlStack::CurlStack() { + m_handle = (void*)curl_multi_init(); + m_task_timeout.slot() = std::bind(&CurlStack::receive_timeout, this); #if (LIBCURL_VERSION_NUM >= 0x071000) curl_multi_setopt((CURLM*)m_handle, CURLMOPT_TIMERDATA, this); @@ -66,11 +24,21 @@ CurlStack::CurlStack() : } CurlStack::~CurlStack() { + shutdown(); +} + +void +CurlStack::shutdown() { + if (!m_running) + return; + + m_running = false; + while (!empty()) front()->close(); curl_multi_cleanup((CURLM*)m_handle); - priority_queue_erase(&taskScheduler, &m_taskTimeout); + priority_queue_erase(&taskScheduler, &m_task_timeout); } CurlGet* @@ -80,6 +48,9 @@ CurlStack::new_object() { CurlSocket* CurlStack::new_socket(int fd) { + if (!m_running) + throw torrent::internal_error("CurlStack::new_socket() called when not running."); + CurlSocket* socket = new CurlSocket(fd, this); curl_multi_assign((CURLM*)m_handle, fd, socket); return socket; @@ -115,7 +86,7 @@ CurlStack::receive_action(CurlSocket* socket, int events) { ; // Do nothing. if (empty()) - priority_queue_erase(&taskScheduler, &m_taskTimeout); + priority_queue_erase(&taskScheduler, &m_task_timeout); } } while (code == CURLM_CALL_MULTI_PERFORM); @@ -133,11 +104,11 @@ CurlStack::process_done_handle() { throw torrent::internal_error("CurlStack::receive_action() msg->msg != CURLMSG_DONE."); if (msg->data.result == CURLE_COULDNT_RESOLVE_HOST) { - iterator itr = std::find_if(begin(), end(), rak::equal(msg->easy_handle, std::mem_fun(&CurlGet::handle))); - + iterator itr = std::find_if(begin(), end(), [&msg](CurlGet* get) { return get->handle() == msg->easy_handle; }); + if (itr == end()) throw torrent::internal_error("Could not find CurlGet when calling CurlStack::receive_action."); - + if (!(*itr)->is_using_ipv6()) { (*itr)->retry_ipv6(); @@ -155,7 +126,7 @@ CurlStack::process_done_handle() { void CurlStack::transfer_done(void* handle, const char* msg) { - iterator itr = std::find_if(begin(), end(), rak::equal(handle, std::mem_fun(&CurlGet::handle))); + iterator itr = std::find_if(begin(), end(), [&handle](CurlGet* get) { return get->handle() == handle; }); if (itr == end()) throw torrent::internal_error("Could not find CurlGet with the right easy_handle."); @@ -171,30 +142,30 @@ CurlStack::receive_timeout() { receive_action(NULL, 0); // Sometimes libcurl forgets to reset the timeout. Try to poll the value in that case, or use 10 seconds. - if (!empty() && !m_taskTimeout.is_queued()) { + if (!empty() && !m_task_timeout.is_queued()) { long timeout; curl_multi_timeout((CURLM*)m_handle, &timeout); - priority_queue_insert(&taskScheduler, &m_taskTimeout, + priority_queue_insert(&taskScheduler, &m_task_timeout, cachedTime + rak::timer::from_milliseconds(std::max(timeout, 10000))); } } void CurlStack::add_get(CurlGet* get) { - if (!m_userAgent.empty()) - curl_easy_setopt(get->handle(), CURLOPT_USERAGENT, m_userAgent.c_str()); + if (!m_user_agent.empty()) + curl_easy_setopt(get->handle(), CURLOPT_USERAGENT, m_user_agent.c_str()); - if (!m_httpProxy.empty()) - curl_easy_setopt(get->handle(), CURLOPT_PROXY, m_httpProxy.c_str()); + if (!m_http_proxy.empty()) + curl_easy_setopt(get->handle(), CURLOPT_PROXY, m_http_proxy.c_str()); - if (!m_bindAddress.empty()) - curl_easy_setopt(get->handle(), CURLOPT_INTERFACE, m_bindAddress.c_str()); + if (!m_bind_address.empty()) + curl_easy_setopt(get->handle(), CURLOPT_INTERFACE, m_bind_address.c_str()); - if (!m_httpCaPath.empty()) - curl_easy_setopt(get->handle(), CURLOPT_CAPATH, m_httpCaPath.c_str()); + if (!m_http_ca_path.empty()) + curl_easy_setopt(get->handle(), CURLOPT_CAPATH, m_http_ca_path.c_str()); - if (!m_httpCaCert.empty()) - curl_easy_setopt(get->handle(), CURLOPT_CAINFO, m_httpCaCert.c_str()); + if (!m_http_ca_cert.empty()) + curl_easy_setopt(get->handle(), CURLOPT_CAINFO, m_http_ca_cert.c_str()); curl_easy_setopt(get->handle(), CURLOPT_SSL_VERIFYHOST, (long)(m_ssl_verify_host ? 2 : 0)); curl_easy_setopt(get->handle(), CURLOPT_SSL_VERIFYPEER, (long)(m_ssl_verify_peer ? 1 : 0)); @@ -202,12 +173,12 @@ CurlStack::add_get(CurlGet* get) { base_type::push_back(get); - if (m_active >= m_maxActive) + if (m_active >= m_max_active) return; m_active++; get->set_active(true); - + if (curl_multi_add_handle((CURLM*)m_handle, get->handle()) > 0) throw torrent::internal_error("Error calling curl_multi_add_handle."); @@ -234,8 +205,8 @@ CurlStack::remove_get(CurlGet* get) { if (curl_multi_remove_handle((CURLM*)m_handle, get->handle()) > 0) throw torrent::internal_error("Error calling curl_multi_remove_handle."); - if (m_active == m_maxActive && - (itr = std::find_if(begin(), end(), std::not1(std::mem_fun(&CurlGet::is_active)))) != end()) { + if (m_active == m_max_active && + (itr = std::find_if(begin(), end(), [](CurlGet* get) { return !get->is_active(); })) != end()) { (*itr)->set_active(true); if (curl_multi_add_handle((CURLM*)m_handle, (*itr)->handle()) > 0) @@ -262,7 +233,7 @@ int CurlStack::set_timeout(void* handle, long timeout_ms, void* userp) { CurlStack* stack = (CurlStack*)userp; - priority_queue_update(&taskScheduler, &stack->m_taskTimeout, cachedTime + rak::timer::from_milliseconds(timeout_ms)); + priority_queue_update(&taskScheduler, &stack->m_task_timeout, cachedTime + rak::timer::from_milliseconds(timeout_ms)); return 0; } diff --git a/src/core/curl_stack.h b/src/core/curl_stack.h index ebcfbcf7..71a3abc0 100644 --- a/src/core/curl_stack.h +++ b/src/core/curl_stack.h @@ -1,39 +1,3 @@ -// rTorrent - BitTorrent client -// Copyright (C) 2005-2011, Jari Sundell -// -// This program is free software; you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation; either version 2 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program; if not, write to the Free Software -// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA -// -// In addition, as a special exception, the copyright holders give -// permission to link the code of portions of this program with the -// OpenSSL library under certain conditions as described in each -// individual source file, and distribute linked combinations -// including the two. -// -// You must obey the GNU General Public License in all respects for -// all of the code used other than OpenSSL. If you modify file(s) -// with this exception, you may extend this exception to your version -// of the file(s), but you are not obligated to do so. If you do not -// wish to do so, delete this exception statement from your version. -// If you delete this exception statement from all source files in the -// program, then also delete it here. -// -// Contact: Jari Sundell -// -// Skomakerveien 33 -// 3185 Skoppum, NORWAY - #ifndef RTORRENT_CORE_CURL_STACK_H #define RTORRENT_CORE_CURL_STACK_H @@ -81,24 +45,27 @@ class CurlStack : std::deque { CurlStack(); ~CurlStack(); + void shutdown(); + bool is_running() const { return m_running; } + CurlGet* new_object(); CurlSocket* new_socket(int fd); unsigned int active() const { return m_active; } - unsigned int max_active() const { return m_maxActive; } - void set_max_active(unsigned int a) { m_maxActive = a; } + unsigned int max_active() const { return m_max_active; } + void set_max_active(unsigned int a) { m_max_active = a; } - const std::string& user_agent() const { return m_userAgent; } - const std::string& http_proxy() const { return m_httpProxy; } - const std::string& bind_address() const { return m_bindAddress; } - const std::string& http_capath() const { return m_httpCaPath; } - const std::string& http_cacert() const { return m_httpCaCert; } + const std::string& user_agent() const { return m_user_agent; } + const std::string& http_proxy() const { return m_http_proxy; } + const std::string& bind_address() const { return m_bind_address; } + const std::string& http_capath() const { return m_http_ca_path; } + const std::string& http_cacert() const { return m_http_ca_cert; } - void set_user_agent(const std::string& s) { m_userAgent = s; } - void set_http_proxy(const std::string& s) { m_httpProxy = s; } - void set_bind_address(const std::string& s) { m_bindAddress = s; } - void set_http_capath(const std::string& s) { m_httpCaPath = s; } - void set_http_cacert(const std::string& s) { m_httpCaCert = s; } + void set_user_agent(const std::string& s) { m_user_agent = s; } + void set_http_proxy(const std::string& s) { m_http_proxy = s; } + void set_bind_address(const std::string& s) { m_bind_address = s; } + void set_http_capath(const std::string& s) { m_http_ca_path = s; } + void set_http_cacert(const std::string& s) { m_http_ca_cert = s; } bool ssl_verify_host() const { return m_ssl_verify_host; } bool ssl_verify_peer() const { return m_ssl_verify_peer; } @@ -131,20 +98,22 @@ class CurlStack : std::deque { void* m_handle; - unsigned int m_active; - unsigned int m_maxActive; + bool m_running{true}; - rak::priority_item m_taskTimeout; + unsigned int m_active{0}; + unsigned int m_max_active{32}; - std::string m_userAgent; - std::string m_httpProxy; - std::string m_bindAddress; - std::string m_httpCaPath; - std::string m_httpCaCert; + rak::priority_item m_task_timeout; - bool m_ssl_verify_host; - bool m_ssl_verify_peer; - long m_dns_timeout; + std::string m_user_agent; + std::string m_http_proxy; + std::string m_bind_address; + std::string m_http_ca_path; + std::string m_http_ca_cert; + + bool m_ssl_verify_host{true}; + bool m_ssl_verify_peer{true}; + long m_dns_timeout{60}; }; } diff --git a/src/core/download_store.cc b/src/core/download_store.cc index 5ab640dc..b3a3f7ed 100644 --- a/src/core/download_store.cc +++ b/src/core/download_store.cc @@ -1,39 +1,3 @@ -// rTorrent - BitTorrent client -// Copyright (C) 2005-2011, Jari Sundell -// -// This program is free software; you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation; either version 2 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program; if not, write to the Free Software -// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA -// -// In addition, as a special exception, the copyright holders give -// permission to link the code of portions of this program with the -// OpenSSL library under certain conditions as described in each -// individual source file, and distribute linked combinations -// including the two. -// -// You must obey the GNU General Public License in all respects for -// all of the code used other than OpenSSL. If you modify file(s) -// with this exception, you may extend this exception to your version -// of the file(s), but you are not obligated to do so. If you do not -// wish to do so, delete this exception statement from your version. -// If you delete this exception statement from all source files in the -// program, then also delete it here. -// -// Contact: Jari Sundell -// -// Skomakerveien 33 -// 3185 Skoppum, NORWAY - // DownloadStore handles the saving and listing of session torrents. #include "config.h" diff --git a/src/core/download_store.h b/src/core/download_store.h index dd345c89..2acdd406 100644 --- a/src/core/download_store.h +++ b/src/core/download_store.h @@ -1,39 +1,3 @@ -// rTorrent - BitTorrent client -// Copyright (C) 2005-2011, Jari Sundell -// -// This program is free software; you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation; either version 2 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program; if not, write to the Free Software -// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA -// -// In addition, as a special exception, the copyright holders give -// permission to link the code of portions of this program with the -// OpenSSL library under certain conditions as described in each -// individual source file, and distribute linked combinations -// including the two. -// -// You must obey the GNU General Public License in all respects for -// all of the code used other than OpenSSL. If you modify file(s) -// with this exception, you may extend this exception to your version -// of the file(s), but you are not obligated to do so. If you do not -// wish to do so, delete this exception statement from your version. -// If you delete this exception statement from all source files in the -// program, then also delete it here. -// -// Contact: Jari Sundell -// -// Skomakerveien 33 -// 3185 Skoppum, NORWAY - #ifndef RTORRENT_CORE_DOWNLOAD_STORE_H #define RTORRENT_CORE_DOWNLOAD_STORE_H diff --git a/src/core/manager.cc b/src/core/manager.cc index e5444498..f7b6721b 100644 --- a/src/core/manager.cc +++ b/src/core/manager.cc @@ -1,39 +1,3 @@ -// rTorrent - BitTorrent client -// Copyright (C) 2005-2011, Jari Sundell -// -// This program is free software; you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation; either version 2 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program; if not, write to the Free Software -// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA -// -// In addition, as a special exception, the copyright holders give -// permission to link the code of portions of this program with the -// OpenSSL library under certain conditions as described in each -// individual source file, and distribute linked combinations -// including the two. -// -// You must obey the GNU General Public License in all respects for -// all of the code used other than OpenSSL. If you modify file(s) -// with this exception, you may extend this exception to your version -// of the file(s), but you are not obligated to do so. If you do not -// wish to do so, delete this exception statement from your version. -// If you delete this exception statement from all source files in the -// program, then also delete it here. -// -// Contact: Jari Sundell -// -// Skomakerveien 33 -// 3185 Skoppum, NORWAY - #include "config.h" #include @@ -186,6 +150,8 @@ Manager::initialize_second() { void Manager::cleanup() { + m_httpStack->shutdown(); + // Need to disconnect log signals? Not really since we won't receive // any more. @@ -198,15 +164,14 @@ Manager::cleanup() { delete m_httpStack; CurlStack::global_cleanup(); - } void Manager::shutdown(bool force) { if (!force) - std::for_each(m_downloadList->begin(), m_downloadList->end(), std::bind1st(std::mem_fun(&DownloadList::pause_default), m_downloadList)); + std::for_each(m_downloadList->begin(), m_downloadList->end(), [this](Download* d) { m_downloadList->pause_default(d); }); else - std::for_each(m_downloadList->begin(), m_downloadList->end(), std::bind1st(std::mem_fun(&DownloadList::close_quick), m_downloadList)); + std::for_each(m_downloadList->begin(), m_downloadList->end(), [this](Download* d) { m_downloadList->close_quick(d); }); } void @@ -222,7 +187,7 @@ Manager::listen_open() { if (portRange.is_string()) { if (std::sscanf(portRange.as_string().c_str(), "%i-%i", &portFirst, &portLast) != 2) throw torrent::input_error("Invalid port_range argument."); - + // } else if (portRange.is_list()) { } else { @@ -414,7 +379,7 @@ path_expand(std::vector* paths, const std::string& pattern) { rak::split_iterator_t first = rak::split_iterator(pattern, '/'); rak::split_iterator_t last = rak::split_iterator(pattern); - + if (first == last) return;