Wrap CURL* in a std::unique_ptr (#2495)

This is safer and clarifies the ownership of pointers.

Co-authored-by: Takeshi Nakatani <ggtakec@gmail.com>
This commit is contained in:
Andrew Gaul 2024-07-15 15:15:03 +09:00 committed by GitHub
parent 60b871e0ae
commit 77ffe7d634
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 37 additions and 36 deletions

View File

@ -79,6 +79,16 @@ static constexpr char SPECIAL_DARWIN_MIME_FILE[] = "/etc/apache2/mime.typ
#define S3FS_CURLOPT_XFERINFOFUNCTION CURLOPT_PROGRESSFUNCTION #define S3FS_CURLOPT_XFERINFOFUNCTION CURLOPT_PROGRESSFUNCTION
#endif #endif
// Wrappers to pass std::unique_ptr to raw pointer functions. Undefine curl_easy_setopt to work around curl variadic argument macro.
#undef curl_easy_setopt
template<typename Arg> CURLcode curl_easy_setopt(const CurlUniquePtr& handle, CURLoption option, Arg arg) {
return curl_easy_setopt(handle.get(), option, arg);
}
template<typename Arg> CURLcode curl_easy_getinfo(const CurlUniquePtr& handle, CURLINFO info, Arg arg) {
return curl_easy_getinfo(handle.get(), info, arg);
}
//------------------------------------------------------------------- //-------------------------------------------------------------------
// Class S3fsCurl // Class S3fsCurl
//------------------------------------------------------------------- //-------------------------------------------------------------------
@ -1842,7 +1852,7 @@ bool S3fsCurl::PreHeadRequestSetCurlOpts(S3fsCurl* s3fscurl)
return true; return true;
} }
bool S3fsCurl::AddUserAgent(CURL* hCurl) bool S3fsCurl::AddUserAgent(const CurlUniquePtr& hCurl)
{ {
if(!hCurl){ if(!hCurl){
return false; return false;
@ -1939,7 +1949,7 @@ int S3fsCurl::RawCurlDebugFunc(const CURL* hcurl, curl_infotype type, char* data
// Methods for S3fsCurl // Methods for S3fsCurl
//------------------------------------------------------------------- //-------------------------------------------------------------------
S3fsCurl::S3fsCurl(bool ahbe) : S3fsCurl::S3fsCurl(bool ahbe) :
hCurl(nullptr), type(REQTYPE::UNSET), requestHeaders(nullptr), type(REQTYPE::UNSET), requestHeaders(nullptr),
LastResponseCode(S3FSCURL_RESPONSECODE_NOTSET), postdata(nullptr), postdata_remaining(0), is_use_ahbe(ahbe), LastResponseCode(S3FSCURL_RESPONSECODE_NOTSET), postdata(nullptr), postdata_remaining(0), is_use_ahbe(ahbe),
retry_count(0), b_infile(nullptr), b_postdata(nullptr), b_postdata_remaining(0), b_partdata_startpos(0), b_partdata_size(0), retry_count(0), b_infile(nullptr), b_postdata(nullptr), b_postdata_remaining(0), b_partdata_startpos(0), b_partdata_size(0),
b_ssekey_pos(-1), b_ssetype(sse_type_t::SSE_DISABLE), b_ssekey_pos(-1), b_ssetype(sse_type_t::SSE_DISABLE),
@ -1965,7 +1975,7 @@ bool S3fsCurl::ResetHandle()
curl_warnings_once = true; curl_warnings_once = true;
} }
sCurlPool->ResetHandler(hCurl); sCurlPool->ResetHandler(hCurl.get());
if(CURLE_OK != curl_easy_setopt(hCurl, CURLOPT_NOSIGNAL, 1)){ if(CURLE_OK != curl_easy_setopt(hCurl, CURLOPT_NOSIGNAL, 1)){
return false; return false;
@ -1982,7 +1992,7 @@ bool S3fsCurl::ResetHandle()
if(CURLE_OK != curl_easy_setopt(hCurl, S3FS_CURLOPT_XFERINFOFUNCTION, S3fsCurl::CurlProgress)){ if(CURLE_OK != curl_easy_setopt(hCurl, S3FS_CURLOPT_XFERINFOFUNCTION, S3fsCurl::CurlProgress)){
return false; return false;
} }
if(CURLE_OK != curl_easy_setopt(hCurl, CURLOPT_PROGRESSDATA, hCurl)){ if(CURLE_OK != curl_easy_setopt(hCurl, CURLOPT_PROGRESSDATA, hCurl.get())){
return false; return false;
} }
// curl_easy_setopt(hCurl, CURLOPT_FORBID_REUSE, 1); // curl_easy_setopt(hCurl, CURLOPT_FORBID_REUSE, 1);
@ -2083,7 +2093,7 @@ bool S3fsCurl::ResetHandle()
} }
} }
S3fsCurl::curl_progress[hCurl] = {time(nullptr), -1, -1}; S3fsCurl::curl_progress[hCurl.get()] = {time(nullptr), -1, -1};
return true; return true;
} }
@ -2140,9 +2150,8 @@ bool S3fsCurl::DestroyCurlHandleHasLock(bool restore_pool, bool clear_internal_d
} }
if(hCurl){ if(hCurl){
S3fsCurl::curl_progress.erase(hCurl); S3fsCurl::curl_progress.erase(hCurl.get());
sCurlPool->ReturnHandler(hCurl, restore_pool); sCurlPool->ReturnHandler(std::move(hCurl), restore_pool);
hCurl = nullptr;
}else{ }else{
return false; return false;
} }
@ -2540,7 +2549,7 @@ int S3fsCurl::RequestPerform(bool dontAddAuthHeaders /*=false*/)
{ {
if(S3fsLog::IsS3fsLogDbg()){ if(S3fsLog::IsS3fsLogDbg()){
char* ptr_url = nullptr; char* ptr_url = nullptr;
curl_easy_getinfo(hCurl, CURLINFO_EFFECTIVE_URL , &ptr_url); curl_easy_getinfo(hCurl, CURLINFO_EFFECTIVE_URL, &ptr_url);
S3FS_PRN_DBG("connecting to URL %s", SAFESTRPTR(ptr_url)); S3FS_PRN_DBG("connecting to URL %s", SAFESTRPTR(ptr_url));
} }
@ -2563,7 +2572,7 @@ int S3fsCurl::RequestPerform(bool dontAddAuthHeaders /*=false*/)
} }
// Requests // Requests
curlCode = curl_easy_perform(hCurl); curlCode = curl_easy_perform(hCurl.get());
// Check result // Check result
switch(curlCode){ switch(curlCode){
@ -2691,7 +2700,7 @@ int S3fsCurl::RequestPerform(bool dontAddAuthHeaders /*=false*/)
sleep(4); sleep(4);
{ {
const std::lock_guard<std::mutex> lock(S3fsCurl::curl_handles_lock); const std::lock_guard<std::mutex> lock(S3fsCurl::curl_handles_lock);
S3fsCurl::curl_progress[hCurl] = {time(nullptr), -1, -1}; S3fsCurl::curl_progress[hCurl.get()] = {time(nullptr), -1, -1};
} }
break; break;

View File

@ -76,6 +76,7 @@ struct curlprogress {
double dl_progress; double dl_progress;
double ul_progress; double ul_progress;
}; };
typedef std::unique_ptr<CURL, decltype(&curl_easy_cleanup)> CurlUniquePtr;
//---------------------------------------------- //----------------------------------------------
// class S3fsCurl // class S3fsCurl
@ -173,7 +174,7 @@ class S3fsCurl
static long ipresolve_type; // this value is a libcurl symbol. static long ipresolve_type; // this value is a libcurl symbol.
// variables // variables
CURL* hCurl; CurlUniquePtr hCurl = {nullptr, curl_easy_cleanup};
REQTYPE type; // type of request REQTYPE type; // type of request
std::string path; // target object path std::string path; // target object path
std::string base_path; // base path (for multi curl head request) std::string base_path; // base path (for multi curl head request)
@ -256,7 +257,7 @@ class S3fsCurl
static bool LoadEnvSseCKeys(); static bool LoadEnvSseCKeys();
static bool LoadEnvSseKmsid(); static bool LoadEnvSseKmsid();
static bool PushbackSseKeys(const std::string& onekey); static bool PushbackSseKeys(const std::string& onekey);
static bool AddUserAgent(CURL* hCurl); static bool AddUserAgent(const CurlUniquePtr& hCurl);
static int CurlDebugFunc(const CURL* hcurl, curl_infotype type, char* data, size_t size, void* userptr); static int CurlDebugFunc(const CURL* hcurl, curl_infotype type, char* data, size_t size, void* userptr);
static int CurlDebugBodyInFunc(const CURL* hcurl, curl_infotype type, char* data, size_t size, void* userptr); static int CurlDebugBodyInFunc(const CURL* hcurl, curl_infotype type, char* data, size_t size, void* userptr);
@ -391,7 +392,6 @@ class S3fsCurl
int MultipartRenameRequest(const char* from, const char* to, headers_t& meta, off_t size); int MultipartRenameRequest(const char* from, const char* to, headers_t& meta, off_t size);
// methods(variables) // methods(variables)
CURL* GetCurlHandle() const { return hCurl; }
const std::string& GetPath() const { return path; } const std::string& GetPath() const { return path; }
const std::string& GetBasePath() const { return base_path; } const std::string& GetBasePath() const { return base_path; }
const std::string& GetSpecialSavedPath() const { return saved_path; } const std::string& GetSpecialSavedPath() const { return saved_path; }

View File

@ -29,13 +29,13 @@
bool CurlHandlerPool::Init() bool CurlHandlerPool::Init()
{ {
for(int cnt = 0; cnt < mMaxHandlers; ++cnt){ for(int cnt = 0; cnt < mMaxHandlers; ++cnt){
CURL* hCurl = curl_easy_init(); CurlUniquePtr hCurl(curl_easy_init(), &curl_easy_cleanup);
if(!hCurl){ if(!hCurl){
S3FS_PRN_ERR("Init curl handlers pool failed"); S3FS_PRN_ERR("Init curl handlers pool failed");
Destroy(); Destroy();
return false; return false;
} }
mPool.push_back(hCurl); mPool.push_back(std::move(hCurl));
} }
return true; return true;
} }
@ -45,23 +45,19 @@ bool CurlHandlerPool::Destroy()
const std::lock_guard<std::mutex> lock(mLock); const std::lock_guard<std::mutex> lock(mLock);
while(!mPool.empty()){ while(!mPool.empty()){
CURL* hCurl = mPool.back();
mPool.pop_back(); mPool.pop_back();
if(hCurl){
curl_easy_cleanup(hCurl);
}
} }
return true; return true;
} }
CURL* CurlHandlerPool::GetHandler(bool only_pool) CurlUniquePtr CurlHandlerPool::GetHandler(bool only_pool)
{ {
const std::lock_guard<std::mutex> lock(mLock); const std::lock_guard<std::mutex> lock(mLock);
CURL* hCurl = nullptr; CurlUniquePtr hCurl(nullptr, curl_easy_cleanup);
if(!mPool.empty()){ if(!mPool.empty()){
hCurl = mPool.back(); hCurl = std::move(mPool.back());
mPool.pop_back(); mPool.pop_back();
S3FS_PRN_DBG("Get handler from pool: rest = %d", static_cast<int>(mPool.size())); S3FS_PRN_DBG("Get handler from pool: rest = %d", static_cast<int>(mPool.size()));
} }
@ -70,12 +66,12 @@ CURL* CurlHandlerPool::GetHandler(bool only_pool)
} }
if(!hCurl){ if(!hCurl){
S3FS_PRN_INFO("Pool empty: force to create new handler"); S3FS_PRN_INFO("Pool empty: force to create new handler");
hCurl = curl_easy_init(); hCurl.reset(curl_easy_init());
} }
return hCurl; return hCurl;
} }
void CurlHandlerPool::ReturnHandler(CURL* hCurl, bool restore_pool) void CurlHandlerPool::ReturnHandler(CurlUniquePtr&& hCurl, bool restore_pool)
{ {
if(!hCurl){ if(!hCurl){
return; return;
@ -84,19 +80,14 @@ void CurlHandlerPool::ReturnHandler(CURL* hCurl, bool restore_pool)
if(restore_pool){ if(restore_pool){
S3FS_PRN_DBG("Return handler to pool"); S3FS_PRN_DBG("Return handler to pool");
mPool.push_back(hCurl); mPool.push_back(std::move(hCurl));
while(mMaxHandlers < static_cast<int>(mPool.size())){ while(mMaxHandlers < static_cast<int>(mPool.size())){
CURL* hOldCurl = mPool.front();
mPool.pop_front();
if(hOldCurl){
S3FS_PRN_INFO("Pool full: destroy the oldest handler"); S3FS_PRN_INFO("Pool full: destroy the oldest handler");
curl_easy_cleanup(hOldCurl); mPool.pop_front();
}
} }
}else{ }else{
S3FS_PRN_INFO("Pool full: destroy the handler"); S3FS_PRN_INFO("Pool full: destroy the handler");
curl_easy_cleanup(hCurl);
} }
} }

View File

@ -24,12 +24,13 @@
#include <cassert> #include <cassert>
#include <curl/curl.h> #include <curl/curl.h>
#include <list> #include <list>
#include <memory>
#include <mutex> #include <mutex>
//---------------------------------------------- //----------------------------------------------
// Typedefs // Typedefs
//---------------------------------------------- //----------------------------------------------
typedef std::list<CURL*> hcurllist_t; typedef std::unique_ptr<CURL, decltype(&curl_easy_cleanup)> CurlUniquePtr;
//---------------------------------------------- //----------------------------------------------
// class CurlHandlerPool // class CurlHandlerPool
@ -49,14 +50,14 @@ class CurlHandlerPool
bool Init(); bool Init();
bool Destroy(); bool Destroy();
CURL* GetHandler(bool only_pool); CurlUniquePtr GetHandler(bool only_pool);
void ReturnHandler(CURL* hCurl, bool restore_pool); void ReturnHandler(CurlUniquePtr&& hCurl, bool restore_pool);
void ResetHandler(CURL* hCurl); void ResetHandler(CURL* hCurl);
private: private:
int mMaxHandlers; int mMaxHandlers;
std::mutex mLock; std::mutex mLock;
hcurllist_t mPool; std::list<CurlUniquePtr> mPool;
}; };
#endif // S3FS_CURL_HANDLERPOOL_H_ #endif // S3FS_CURL_HANDLERPOOL_H_