Add connection pool option (V3) #1219
Conversation
|
@cleaton thanks for contributing! I will try to review it over the weekend. |
|
Please also add a MR here for documentation: https://github.com/libcpr/docs |
| lock->lock(); | ||
| }; | ||
|
|
||
| auto unlock_f = +[](__attribute__((unused)) CURL* handle, __attribute__((unused)) curl_lock_data data, void* userptr) { |
There was a problem hiding this comment.
All of the __attribute__((unused)) could be replaced with [[maybe_unused]]?
There was a problem hiding this comment.
I prefer just commenting them out.
|
|
||
| namespace cpr { | ||
| ConnectionPool::ConnectionPool() { | ||
| auto* curl_share = curl_share_init(); |
There was a problem hiding this comment.
| auto* curl_share = curl_share_init(); | |
| const CURLSH* curl_share = curl_share_init(); |
I'm trying to avoid auto when ever possible since I prefer explicit code.
| @@ -390,6 +391,13 @@ void Session::SetConnectTimeout(const ConnectTimeout& timeout) { | |||
| curl_easy_setopt(curl_->handle, CURLOPT_CONNECTTIMEOUT_MS, timeout.Milliseconds()); | |||
| } | |||
|
|
|||
| void Session::SetConnectionPool(const ConnectionPool& pool) { | |||
| auto * curl = curl_->handle; | |||
There was a problem hiding this comment.
| auto * curl = curl_->handle; | |
| CURL* curl = curl_->handle; |
| if (curl) { | ||
| pool.SetupHandler(curl); | ||
| } |
There was a problem hiding this comment.
You can expect curl to be valid here. If you do not trust it, use a assert(curl)
| namespace cpr { | ||
| class ConnectionPool { | ||
| public: | ||
| ConnectionPool(); | ||
| ConnectionPool(const ConnectionPool&) = default; | ||
| ConnectionPool& operator=(const ConnectionPool&) = delete; | ||
| void SetupHandler(CURL* easy_handler) const; | ||
|
|
||
| private: | ||
| std::shared_ptr<std::mutex> connection_mutex_; | ||
| std::shared_ptr<CURLSH> curl_sh_; | ||
| }; | ||
| } // namespace cpr | ||
| #endif |
There was a problem hiding this comment.
Can I get a bit of inline code documentation here? I know not all the existing code is documented well, but I try to change this with all new additions e.g. #1168
| @@ -79,6 +82,18 @@ void AbstractServer::Run() { | |||
| server_stop_cv.notify_all(); | |||
| } | |||
|
|
|||
| void AbstractServer::AddConnection(int remote_port) { | |||
| unique_connections.insert(remote_port); | |||
There was a problem hiding this comment.
Should we take care of cleaning them up once the connection is shut down?
| auto lock_f = +[](__attribute__((unused)) CURL* handle, __attribute__((unused)) curl_lock_data data, __attribute__((unused)) curl_lock_access access, void* userptr) { | ||
| std::mutex* lock = static_cast<std::mutex*>(userptr); | ||
| lock->lock(); | ||
| }; |
There was a problem hiding this comment.
| auto lock_f = +[](__attribute__((unused)) CURL* handle, __attribute__((unused)) curl_lock_data data, __attribute__((unused)) curl_lock_access access, void* userptr) { | |
| std::mutex* lock = static_cast<std::mutex*>(userptr); | |
| lock->lock(); | |
| }; | |
| auto lock_f = +[](CURL* /*handle*/, curl_lock_data /*data*/, curl_lock_access /*access*/, void* userptr) { | |
| std::mutex* lock = static_cast<std::mutex*>(userptr); | |
| lock->lock(); | |
| }; |
I prefer just commenting them out.
| lock->lock(); | ||
| }; | ||
|
|
||
| auto unlock_f = +[](__attribute__((unused)) CURL* handle, __attribute__((unused)) curl_lock_data data, void* userptr) { |
There was a problem hiding this comment.
I prefer just commenting them out.
Add connection pool option (see #488 & #330)