add host based connection pool map#156
add host based connection pool map#156kavyako merged 8 commits intomicrosoft:developmentfrom hanzhumsft:host_based_connection_pool
Conversation
|
Hi @hanzhumsft, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
| const int m_timeout_secs; | ||
| const bool m_start_with_ssl; | ||
| const std::function<void(boost::asio::ssl::context&)>& m_ssl_context_callback; | ||
| std::function<void(boost::asio::ssl::context&)> m_ssl_context_callback; |
There was a problem hiding this comment.
Why is this change needed?
There was a problem hiding this comment.
This is to prevent a dangling reference, because the lifetime of asio_connection_pool can be longer than the function object m_ssl_context_callback references. That function object comes from http_client_config when creating a http_client object.
|
Running tests on Linux: **** outside_tests:outside_cnn_dot_com FAILED **** **** outside_tests:outside_google_dot_com FAILED **** **** outside_tests:multiple_https_requests FAILED **** **** outside_tests:reading_google_stream FAILED **** Finished running all 777 tests. All 4 cases failed without my change (1e0b5c5). |
| return std::make_shared<asio_connection_pool>(crossplat::threadpool::shared_instance().service(), | ||
| base_uri().scheme() == "https" && !_http_client_communicator::client_config().proxy().is_specified(), | ||
| std::chrono::seconds(30), // Unused sockets are kept in pool for 30 seconds. | ||
| this->client_config().get_ssl_context_callback()); |
There was a problem hiding this comment.
[minor] This branch of the if already has guaranteed there is no ssl_context_callback, so passing in nullptr here is probably more clear.
| { | ||
| m_ssl_context = utility::details::make_unique<boost::asio::ssl::context>(boost::asio::ssl::context::sslv23); | ||
| ssl_context_callback(*m_ssl_context); | ||
| if (ssl_context_callback) |
There was a problem hiding this comment.
It is possible that the ssl_context_callback does not have a valid target.
| if (!ec) | ||
| { | ||
| std::lock_guard<std::mutex> lg(m_connection_pool_map_mutex); | ||
| auto &pool = m_connection_pool_map[pool_key]; |
There was a problem hiding this comment.
This will allocate a new pool shared_ptr if there is not a map entry. It would probably be better to use .find(pool_key) to avoid allocating a new map entry just to erase it.
|
Instead of having a "pool of pools" and a timer for each |
This change is for the http_client implementation on Linux.
The change can make the following code running efficiently on Linux.
Originally, the following code will create a connection, close it, create another connection, and so on ...
Now the connections can be reused if the authority is same.
On Windows, connection is always reused (based on host) and managed by WinHTTP. This change makes it easier for cross-platform projects to use the same piece of code.