From 96727e3487bdd74221a45892439507d4129328fe Mon Sep 17 00:00:00 2001 From: Takeshi Nakatani Date: Mon, 15 Jul 2024 06:40:05 +0000 Subject: [PATCH] Removed CurlHandlerPool and simplified it --- src/Makefile.am | 1 - src/curl.cpp | 43 +++------- src/curl.h | 9 +- src/curl_handlerpool.cpp | 114 -------------------------- src/curl_handlerpool.h | 74 ----------------- src/curl_multi.cpp | 2 +- src/fdcache_fdinfo.cpp | 8 +- src/fdcache_fdinfo.h | 3 - test/threadsanitizer_suppressions.txt | 6 ++ 9 files changed, 22 insertions(+), 238 deletions(-) delete mode 100644 src/curl_handlerpool.cpp delete mode 100644 src/curl_handlerpool.h diff --git a/src/Makefile.am b/src/Makefile.am index 45fcad6b21..b2fdf12a17 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -36,7 +36,6 @@ s3fs_SOURCES = \ metaheader.cpp \ mpu_util.cpp \ curl.cpp \ - curl_handlerpool.cpp \ curl_multi.cpp \ curl_util.cpp \ s3objlist.cpp \ diff --git a/src/curl.cpp b/src/curl.cpp index f71a5b9541..88b4ce6beb 100644 --- a/src/curl.cpp +++ b/src/curl.cpp @@ -38,7 +38,6 @@ #include "curl_multi.h" #include "curl_util.h" #include "s3fs_auth.h" -#include "curl_handlerpool.h" #include "s3fs_cred.h" #include "s3fs_util.h" #include "string_util.h" @@ -99,8 +98,6 @@ constexpr char S3fsCurl::S3FS_SSL_PRIVKEY_PASSWORD[]; std::mutex S3fsCurl::curl_handles_lock; S3fsCurl::callback_locks_t S3fsCurl::callback_locks; bool S3fsCurl::is_initglobal_done = false; -std::unique_ptr S3fsCurl::sCurlPool; -int S3fsCurl::sCurlPoolSize = 32; CURLSH* S3fsCurl::hCurlShare = nullptr; bool S3fsCurl::is_cert_check = true; // default bool S3fsCurl::is_dns_cache = true; // default @@ -162,14 +159,6 @@ bool S3fsCurl::InitS3fsCurl() if(!S3fsCurl::InitCryptMutex()){ return false; } - // [NOTE] - // sCurlPoolSize must be over parallel(or multireq) count. - // - sCurlPoolSize = std::max({sCurlPoolSize, GetMaxParallelCount(), GetMaxMultiRequest()}); - sCurlPool.reset(new CurlHandlerPool(sCurlPoolSize)); - if (!sCurlPool->Init()) { - return false; - } return true; } @@ -180,10 +169,6 @@ bool S3fsCurl::DestroyS3fsCurl() if(!S3fsCurl::DestroyCryptMutex()){ result = false; } - if(!sCurlPool->Destroy()){ - result = false; - } - sCurlPool.reset(); if(!S3fsCurl::DestroyShareCurl()){ result = false; } @@ -1976,7 +1961,7 @@ bool S3fsCurl::ResetHandle() { bool run_once = curl_warnings_once.exchange(true); - sCurlPool->ResetHandler(hCurl.get()); + curl_easy_reset(hCurl.get()); if(CURLE_OK != curl_easy_setopt(hCurl, CURLOPT_NOSIGNAL, 1)){ return false; @@ -2099,12 +2084,12 @@ bool S3fsCurl::ResetHandle() return true; } -bool S3fsCurl::CreateCurlHandle(bool only_pool, bool remake) +bool S3fsCurl::CreateCurlHandle(bool remake) { const std::lock_guard lock(S3fsCurl::curl_handles_lock); if(hCurl && remake){ - if(!DestroyCurlHandleHasLock(false, true)){ + if(!DestroyCurlHandleHasLock(true)){ S3FS_PRN_ERR("could not destroy handle."); return false; } @@ -2112,16 +2097,10 @@ bool S3fsCurl::CreateCurlHandle(bool only_pool, bool remake) } if(!hCurl){ - if(nullptr == (hCurl = sCurlPool->GetHandler(only_pool))){ - if(!only_pool){ - S3FS_PRN_ERR("Failed to create handle."); - return false; - }else{ - // [NOTE] - // Further initialization processing is left to lazy processing to be executed later. - // (Currently we do not use only_pool=true, but this code is remained for the future) - return true; - } + hCurl.reset(curl_easy_init()); + if(!hCurl){ + S3FS_PRN_ERR("Failed to create handle."); + return false; } } ResetHandle(); @@ -2129,13 +2108,13 @@ bool S3fsCurl::CreateCurlHandle(bool only_pool, bool remake) return true; } -bool S3fsCurl::DestroyCurlHandle(bool restore_pool, bool clear_internal_data) +bool S3fsCurl::DestroyCurlHandle(bool clear_internal_data) { const std::lock_guard lock(S3fsCurl::curl_handles_lock); - return DestroyCurlHandleHasLock(restore_pool, clear_internal_data); + return DestroyCurlHandleHasLock(clear_internal_data); } -bool S3fsCurl::DestroyCurlHandleHasLock(bool restore_pool, bool clear_internal_data) +bool S3fsCurl::DestroyCurlHandleHasLock(bool clear_internal_data) { // [NOTE] // If type is REQTYPE::IAMCRED or REQTYPE::IAMROLE, do not clear type. @@ -2152,7 +2131,7 @@ bool S3fsCurl::DestroyCurlHandleHasLock(bool restore_pool, bool clear_internal_d if(hCurl){ S3fsCurl::curl_progress.erase(hCurl.get()); - sCurlPool->ReturnHandler(std::move(hCurl), restore_pool); + hCurl.reset(); }else{ return false; } diff --git a/src/curl.h b/src/curl.h index 27b546780b..8f6857a43d 100644 --- a/src/curl.h +++ b/src/curl.h @@ -84,7 +84,6 @@ typedef std::unique_ptr CurlUniquePtr; //---------------------------------------------- // class S3fsCurl //---------------------------------------------- -class CurlHandlerPool; class S3fsCred; class S3fsCurl; @@ -131,8 +130,6 @@ class S3fsCurl std::mutex ssl_session; } callback_locks; static bool is_initglobal_done; - static std::unique_ptr sCurlPool; - static int sCurlPoolSize; static CURLSH* hCurlShare; static bool is_cert_check; static bool is_dns_cache; @@ -363,9 +360,9 @@ class S3fsCurl static bool SetIPResolveType(const char* value); // methods - bool CreateCurlHandle(bool only_pool = false, bool remake = false); - bool DestroyCurlHandle(bool restore_pool = true, bool clear_internal_data = true); - bool DestroyCurlHandleHasLock(bool restore_pool = true, bool clear_internal_data = true) REQUIRES(S3fsCurl::curl_handles_lock); + bool CreateCurlHandle(bool remake = false); + bool DestroyCurlHandle(bool clear_internal_data = true); + bool DestroyCurlHandleHasLock(bool clear_internal_data = true) REQUIRES(S3fsCurl::curl_handles_lock); bool GetIAMCredentials(const char* cred_url, const char* iam_v2_token, const char* ibm_secret_access_key, std::string& response); bool GetIAMRoleFromMetaData(const char* cred_url, const char* iam_v2_token, std::string& token); diff --git a/src/curl_handlerpool.cpp b/src/curl_handlerpool.cpp deleted file mode 100644 index d8c36e34c1..0000000000 --- a/src/curl_handlerpool.cpp +++ /dev/null @@ -1,114 +0,0 @@ -/* - * s3fs - FUSE-based file system backed by Amazon S3 - * - * Copyright(C) 2007 Randy Rizun - * - * 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ - -#include -#include -#include - -#include "s3fs_logger.h" -#include "curl_handlerpool.h" - -//------------------------------------------------------------------- -// Class CurlHandlerPool -//------------------------------------------------------------------- -bool CurlHandlerPool::Init() -{ - for(int cnt = 0; cnt < mMaxHandlers; ++cnt){ - CurlUniquePtr hCurl(curl_easy_init(), &curl_easy_cleanup); - if(!hCurl){ - S3FS_PRN_ERR("Init curl handlers pool failed"); - Destroy(); - return false; - } - const std::lock_guard lock(mLock); - mPool.push_back(std::move(hCurl)); - } - return true; -} - -bool CurlHandlerPool::Destroy() -{ - const std::lock_guard lock(mLock); - - while(!mPool.empty()){ - mPool.pop_back(); - } - return true; -} - -CurlUniquePtr CurlHandlerPool::GetHandler(bool only_pool) -{ - const std::lock_guard lock(mLock); - - CurlUniquePtr hCurl(nullptr, curl_easy_cleanup); - - if(!mPool.empty()){ - hCurl = std::move(mPool.back()); - mPool.pop_back(); - S3FS_PRN_DBG("Get handler from pool: rest = %d", static_cast(mPool.size())); - } - if(only_pool){ - return hCurl; - } - if(!hCurl){ - S3FS_PRN_INFO("Pool empty: force to create new handler"); - hCurl.reset(curl_easy_init()); - } - return hCurl; -} - -void CurlHandlerPool::ReturnHandler(CurlUniquePtr&& hCurl, bool restore_pool) -{ - if(!hCurl){ - return; - } - const std::lock_guard lock(mLock); - - if(restore_pool){ - S3FS_PRN_DBG("Return handler to pool"); - mPool.push_back(std::move(hCurl)); - - while(mMaxHandlers < static_cast(mPool.size())){ - S3FS_PRN_INFO("Pool full: destroy the oldest handler"); - mPool.pop_front(); - } - }else{ - S3FS_PRN_INFO("Pool full: destroy the handler"); - } -} - -void CurlHandlerPool::ResetHandler(CURL* hCurl) -{ - if(!hCurl){ - return; - } - const std::lock_guard lock(mLock); - - curl_easy_reset(hCurl); -} - -/* -* Local variables: -* tab-width: 4 -* c-basic-offset: 4 -* End: -* vim600: expandtab sw=4 ts=4 fdm=marker -* vim<600: expandtab sw=4 ts=4 -*/ diff --git a/src/curl_handlerpool.h b/src/curl_handlerpool.h deleted file mode 100644 index ac124472c6..0000000000 --- a/src/curl_handlerpool.h +++ /dev/null @@ -1,74 +0,0 @@ -/* - * s3fs - FUSE-based file system backed by Amazon S3 - * - * Copyright(C) 2007 Randy Rizun - * - * 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ - -#ifndef S3FS_CURL_HANDLERPOOL_H_ -#define S3FS_CURL_HANDLERPOOL_H_ - -#include -#include -#include -#include -#include - -#include "common.h" - -//---------------------------------------------- -// Typedefs -//---------------------------------------------- -typedef std::unique_ptr CurlUniquePtr; - -//---------------------------------------------- -// class CurlHandlerPool -//---------------------------------------------- -class CurlHandlerPool -{ - public: - explicit CurlHandlerPool(int maxHandlers) : mMaxHandlers(maxHandlers) - { - assert(maxHandlers > 0); - } - CurlHandlerPool(const CurlHandlerPool&) = delete; - CurlHandlerPool(CurlHandlerPool&&) = delete; - CurlHandlerPool& operator=(const CurlHandlerPool&) = delete; - CurlHandlerPool& operator=(CurlHandlerPool&&) = delete; - - bool Init(); - bool Destroy(); - - CurlUniquePtr GetHandler(bool only_pool); - void ReturnHandler(CurlUniquePtr&& hCurl, bool restore_pool); - void ResetHandler(CURL* hCurl); - - private: - int mMaxHandlers; - std::mutex mLock; - std::list mPool GUARDED_BY(mLock); -}; - -#endif // S3FS_CURL_HANDLERPOOL_H_ - -/* -* Local variables: -* tab-width: 4 -* c-basic-offset: 4 -* End: -* vim600: expandtab sw=4 ts=4 fdm=marker -* vim<600: expandtab sw=4 ts=4 -*/ diff --git a/src/curl_multi.cpp b/src/curl_multi.cpp index cf60724ddc..1747c7eb6f 100644 --- a/src/curl_multi.cpp +++ b/src/curl_multi.cpp @@ -351,7 +351,7 @@ void S3fsMultiCurl::RequestPerformWrapper(S3fsCurl* s3fscurl, std::promise if(!result){ result = s3fscurl->RequestPerform(); - s3fscurl->DestroyCurlHandle(true, false); + s3fscurl->DestroyCurlHandle(false); } const std::lock_guard lock(*s3fscurl->completed_tids_lock); diff --git a/src/fdcache_fdinfo.cpp b/src/fdcache_fdinfo.cpp index a1973377c2..596a1f0ba4 100644 --- a/src/fdcache_fdinfo.cpp +++ b/src/fdcache_fdinfo.cpp @@ -38,12 +38,6 @@ #include "string_util.h" #include "threadpoolman.h" -//------------------------------------------------ -// PseudoFdInfo class variables -//------------------------------------------------ -int PseudoFdInfo::max_threads = -1; -int PseudoFdInfo::opt_max_threads = -1; - //------------------------------------------------ // PseudoFdInfo class methods //------------------------------------------------ @@ -95,7 +89,7 @@ void* PseudoFdInfo::MultipartUploadThreadWorker(void* arg) }else{ S3FS_PRN_ERR("failed uploading with error(%d) [path=%s][start=%lld][size=%lld][part=%d]", result, pthparam->path.c_str(), static_cast(pthparam->start), static_cast(pthparam->size), pthparam->part_num); } - s3fscurl->DestroyCurlHandle(true, false); + s3fscurl->DestroyCurlHandle(false); // set result const std::lock_guard lock(pthparam->ppseudofdinfo->upload_list_lock); diff --git a/src/fdcache_fdinfo.h b/src/fdcache_fdinfo.h index 45eb375a26..505d77fc81 100644 --- a/src/fdcache_fdinfo.h +++ b/src/fdcache_fdinfo.h @@ -57,9 +57,6 @@ struct pseudofdinfo_thparam class PseudoFdInfo { private: - static int max_threads; - static int opt_max_threads; // for option value - int pseudo_fd; int physical_fd; int flags; // flags at open diff --git a/test/threadsanitizer_suppressions.txt b/test/threadsanitizer_suppressions.txt index 2e7cd8347a..b54923f99c 100644 --- a/test/threadsanitizer_suppressions.txt +++ b/test/threadsanitizer_suppressions.txt @@ -1 +1,7 @@ race:OPENSSL_sk_free + +# In S3fsMultiCurl called from s3fs_readdir +# S3fsMultiCurl will be removed in a series of fixes, so ignore the error until then. +race:SSL_SESSION_free +race:ASN1_tag2bit +race:SSL_SESSION_new