From e769f1008edf72745521522c8bfb598343936db7 Mon Sep 17 00:00:00 2001 From: Igor Gaponenko Date: Tue, 23 Jan 2024 17:32:29 +0000 Subject: [PATCH] The corect implementation of the connectin pool for sync HTTP client(2) --- src/http/Client.cc | 6 +++--- src/http/ClientConnPool.cc | 22 +++++++++++----------- src/http/ClientConnPool.h | 30 +++++++++++++++++------------- 3 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/http/Client.cc b/src/http/Client.cc index 2c8caeebf..7526efa93 100644 --- a/src/http/Client.cc +++ b/src/http/Client.cc @@ -154,10 +154,10 @@ void Client::_setConnOptions() { } if (_connPool != nullptr) { _curlEasyErrorChecked("curl_easy_setopt(CURLOPT_SHARE)", - curl_easy_setopt(_hcurl, CURLOPT_SHARE, _connPool->_shareCurl)); - if (_connPool->_maxConnections > 0) { + curl_easy_setopt(_hcurl, CURLOPT_SHARE, _connPool->shareCurl())); + if (_connPool->maxConnections() > 0) { _curlEasyErrorChecked("curl_easy_setopt(CURLOPT_MAXCONNECTS)", - curl_easy_setopt(_hcurl, CURLOPT_MAXCONNECTS, _connPool->_maxConnections)); + curl_easy_setopt(_hcurl, CURLOPT_MAXCONNECTS, _connPool->maxConnections())); } } } diff --git a/src/http/ClientConnPool.cc b/src/http/ClientConnPool.cc index a43ccea12..47b659525 100644 --- a/src/http/ClientConnPool.cc +++ b/src/http/ClientConnPool.cc @@ -32,32 +32,32 @@ using namespace std; namespace lsst::qserv::http { +mutex ClientConnPool::_accessShareCurlMtx; + ClientConnPool::ClientConnPool(long maxConnections) : _maxConnections(maxConnections) { _shareCurl = curl_share_init(); assert(_shareCurl != nullptr); - _curlShareErrorChecked( - "curl_share_setopt(CURLSHOPT_LOCKFUNC)", - curl_share_setopt(_shareCurl, CURLSHOPT_LOCKFUNC, &ClientConnPool::_share_lock_cb)); - _curlShareErrorChecked( - "curl_share_setopt(CURLSHOPT_UNLOCKFUNC)", - curl_share_setopt(_shareCurl, CURLSHOPT_UNLOCKFUNC, &ClientConnPool::_share_unlock_cb)); - _curlShareErrorChecked("curl_share_setopt(CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT)", - curl_share_setopt(_shareCurl, CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT)); + _errorChecked("curl_share_setopt(CURLSHOPT_LOCKFUNC)", + curl_share_setopt(_shareCurl, CURLSHOPT_LOCKFUNC, &ClientConnPool::_share_lock_cb)); + _errorChecked("curl_share_setopt(CURLSHOPT_UNLOCKFUNC)", + curl_share_setopt(_shareCurl, CURLSHOPT_UNLOCKFUNC, &ClientConnPool::_share_unlock_cb)); + _errorChecked("curl_share_setopt(CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT)", + curl_share_setopt(_shareCurl, CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT)); } ClientConnPool::~ClientConnPool() { curl_share_cleanup(_shareCurl); } void ClientConnPool::_share_lock_cb(CURL* handle, curl_lock_data data, curl_lock_access access, void* userptr) { - _accessShareCurlMtx.lock(); + ClientConnPool::_accessShareCurlMtx.lock(); } void ClientConnPool::_share_unlock_cb(CURL* handle, curl_lock_data data, curl_lock_access access, void* userptr) { - _accessShareCurlMtx.unlock(); + ClientConnPool::_accessShareCurlMtx.unlock(); } -void ClientConnPool::_curlShareErrorChecked(string const& scope, CURLSHcode errnum) { +void ClientConnPool::_errorChecked(string const& scope, CURLSHcode errnum) { if (errnum != CURLSHE_OK) { string const errorStr = curl_share_strerror(errnum); long const httpResponseCode = 0; diff --git a/src/http/ClientConnPool.h b/src/http/ClientConnPool.h index e8593313c..bf3bb5f48 100644 --- a/src/http/ClientConnPool.h +++ b/src/http/ClientConnPool.h @@ -32,9 +32,13 @@ namespace lsst::qserv::http { /** * Class ClientConnPool is a helper class utilizing the libcurl's context - * sharing mechanism for building a configurable pool of teh TCP connections. - * The implementation is based on: - * https://curl.se/libcurl/c/libcurl-share.html + * sharing mechanism for building a configurable pool of the TCP connections. + * Note that this implementation doesn't directly manage connections. The connections + * are owned and managed by the library itself. A role of the class is to provide + * a synchronization context for acquring/releasing these connections in the multi-thread + * environment. + * + * The implementation is based on: https://curl.se/libcurl/c/libcurl-share.html */ class ClientConnPool { public: @@ -48,16 +52,15 @@ class ClientConnPool { ~ClientConnPool(); -private: - /// The class needs access to _numConnections and _shareCurl for customising - /// the TCP connection management. - friend class Client; + long maxConnections() const { return _maxConnections; } + CURLSH* shareCurl() { return _shareCurl; } +private: // These callback functions are required by libcurl to allow easy-based instances - // of the class Client join the pool. + // of the class Client acquire/release connections from the pool. - void _share_lock_cb(CURL* handle, curl_lock_data data, curl_lock_access access, void* userptr); - void _share_unlock_cb(CURL* handle, curl_lock_data data, curl_lock_access access, void* userptr); + static void _share_lock_cb(CURL* handle, curl_lock_data data, curl_lock_access access, void* userptr); + static void _share_unlock_cb(CURL* handle, curl_lock_data data, curl_lock_access access, void* userptr); /** * Check for an error condition. @@ -66,12 +69,13 @@ class ClientConnPool { * @param errnum A result reported by the CURL library function. * @throw std::runtime_error If the error-code is not CURLSHE_OK. */ - void _curlShareErrorChecked(std::string const& scope, CURLSHcode errnum); + void _errorChecked(std::string const& scope, CURLSHcode errnum); - long const _maxConnections = 0; + /// The miutex is shared by all instances of the pool. + static std::mutex _accessShareCurlMtx; + long const _maxConnections = 0; CURLSH* _shareCurl; - std::mutex _accessShareCurlMtx; }; } // namespace lsst::qserv::http