Skip to content

Commit

Permalink
Removed CurlHandlerPool and simplified it
Browse files Browse the repository at this point in the history
  • Loading branch information
ggtakec committed Nov 3, 2024
1 parent ef6c213 commit 96727e3
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 238 deletions.
1 change: 0 additions & 1 deletion src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ s3fs_SOURCES = \
metaheader.cpp \
mpu_util.cpp \
curl.cpp \
curl_handlerpool.cpp \
curl_multi.cpp \
curl_util.cpp \
s3objlist.cpp \
Expand Down
43 changes: 11 additions & 32 deletions src/curl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<CurlHandlerPool> S3fsCurl::sCurlPool;
int S3fsCurl::sCurlPoolSize = 32;
CURLSH* S3fsCurl::hCurlShare = nullptr;
bool S3fsCurl::is_cert_check = true; // default
bool S3fsCurl::is_dns_cache = true; // default
Expand Down Expand Up @@ -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;
}

Expand All @@ -180,10 +169,6 @@ bool S3fsCurl::DestroyS3fsCurl()
if(!S3fsCurl::DestroyCryptMutex()){
result = false;
}
if(!sCurlPool->Destroy()){
result = false;
}
sCurlPool.reset();
if(!S3fsCurl::DestroyShareCurl()){
result = false;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2099,43 +2084,37 @@ bool S3fsCurl::ResetHandle()
return true;
}

bool S3fsCurl::CreateCurlHandle(bool only_pool, bool remake)
bool S3fsCurl::CreateCurlHandle(bool remake)
{
const std::lock_guard<std::mutex> lock(S3fsCurl::curl_handles_lock);

if(hCurl && remake){
if(!DestroyCurlHandleHasLock(false, true)){
if(!DestroyCurlHandleHasLock(true)){
S3FS_PRN_ERR("could not destroy handle.");
return false;
}
S3FS_PRN_INFO3("already has handle, so destroyed it or restored it to pool.");
}

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();

return true;
}

bool S3fsCurl::DestroyCurlHandle(bool restore_pool, bool clear_internal_data)
bool S3fsCurl::DestroyCurlHandle(bool clear_internal_data)
{
const std::lock_guard<std::mutex> 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.
Expand All @@ -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;
}
Expand Down
9 changes: 3 additions & 6 deletions src/curl.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ typedef std::unique_ptr<CURL, decltype(&curl_easy_cleanup)> CurlUniquePtr;
//----------------------------------------------
// class S3fsCurl
//----------------------------------------------
class CurlHandlerPool;
class S3fsCred;
class S3fsCurl;

Expand Down Expand Up @@ -131,8 +130,6 @@ class S3fsCurl
std::mutex ssl_session;
} callback_locks;
static bool is_initglobal_done;
static std::unique_ptr<CurlHandlerPool> sCurlPool;
static int sCurlPoolSize;
static CURLSH* hCurlShare;
static bool is_cert_check;
static bool is_dns_cache;
Expand Down Expand Up @@ -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);
Expand Down
114 changes: 0 additions & 114 deletions src/curl_handlerpool.cpp

This file was deleted.

74 changes: 0 additions & 74 deletions src/curl_handlerpool.h

This file was deleted.

2 changes: 1 addition & 1 deletion src/curl_multi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ void S3fsMultiCurl::RequestPerformWrapper(S3fsCurl* s3fscurl, std::promise<int>

if(!result){
result = s3fscurl->RequestPerform();
s3fscurl->DestroyCurlHandle(true, false);
s3fscurl->DestroyCurlHandle(false);
}

const std::lock_guard<std::mutex> lock(*s3fscurl->completed_tids_lock);
Expand Down
8 changes: 1 addition & 7 deletions src/fdcache_fdinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
//------------------------------------------------
Expand Down Expand Up @@ -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<long long>(pthparam->start), static_cast<long long>(pthparam->size), pthparam->part_num);
}
s3fscurl->DestroyCurlHandle(true, false);
s3fscurl->DestroyCurlHandle(false);

// set result
const std::lock_guard<std::mutex> lock(pthparam->ppseudofdinfo->upload_list_lock);
Expand Down
3 changes: 0 additions & 3 deletions src/fdcache_fdinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions test/threadsanitizer_suppressions.txt
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 96727e3

Please sign in to comment.