From 28eeef7d65b1df271c2e612744a0b351331a7cf1 Mon Sep 17 00:00:00 2001 From: eidheim Date: Fri, 7 Jul 2017 17:16:53 +0200 Subject: [PATCH] Synchronous client request calls is now safe to use concurrently --- client_http.hpp | 31 +++++++++++++++++++++++++++---- http_examples.cpp | 1 - https_examples.cpp | 1 - tests/io_test.cpp | 30 +++++++++++++++++++++++++++++- 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/client_http.hpp b/client_http.hpp index 96a17f4..afa669b 100644 --- a/client_http.hpp +++ b/client_http.hpp @@ -132,7 +132,8 @@ namespace SimpleWeb { virtual ~ClientBase() {} /// Convenience function to perform synchronous request. The io_service is run within this function. - /// If reusing the io_service for other tasks, please use the asynchronous request functions instead. + /// If reusing the io_service for other tasks, use the asynchronous request functions instead. + /// Do not use concurrently with the asynchronous request functions. std::shared_ptr request(const std::string &method, const std::string &path = std::string("/"), string_view content = "", const CaseInsensitiveMultimap &header = CaseInsensitiveMultimap()) { std::shared_ptr response; @@ -142,14 +143,23 @@ namespace SimpleWeb { throw system_error(ec); }); - io_service->reset(); + { + std::unique_lock lock(concurrent_synchronous_requests_mutex); + ++concurrent_synchronous_requests; + } io_service->run(); + { + std::unique_lock lock(concurrent_synchronous_requests_mutex); + if(--concurrent_synchronous_requests == 0) + io_service->reset(); + } return response; } /// Convenience function to perform synchronous request. The io_service is run within this function. - /// If reusing the io_service for other tasks, please use the asynchronous request functions instead. + /// If reusing the io_service for other tasks, use the asynchronous request functions instead. + /// Do not use concurrently with the asynchronous request functions. std::shared_ptr request(const std::string &method, const std::string &path, std::istream &content, const CaseInsensitiveMultimap &header = CaseInsensitiveMultimap()) { std::shared_ptr response; @@ -159,13 +169,22 @@ namespace SimpleWeb { throw system_error(ec); }); - io_service->reset(); + { + std::unique_lock lock(concurrent_synchronous_requests_mutex); + ++concurrent_synchronous_requests; + } io_service->run(); + { + std::unique_lock lock(concurrent_synchronous_requests_mutex); + if(--concurrent_synchronous_requests == 0) + io_service->reset(); + } return response; } /// Asynchronous request where setting and/or running Client's io_service is required. + /// Do not use concurrently with the synchronous request functions. void request(const std::string &method, const std::string &path, string_view content, const CaseInsensitiveMultimap &header, std::function, const error_code &)> &&request_callback_) { auto session = std::make_shared(io_service, get_connection(), create_request_header(method, path, header)); @@ -208,6 +227,7 @@ namespace SimpleWeb { } /// Asynchronous request where setting and/or running Client's io_service is required. + /// Do not use concurrently with the synchronous request functions. void request(const std::string &method, const std::string &path, string_view content, std::function, const error_code &)> &&request_callback) { request(method, path, content, CaseInsensitiveMultimap(), std::move(request_callback)); @@ -283,6 +303,9 @@ namespace SimpleWeb { std::shared_ptr>> connections; std::shared_ptr connections_mutex; + size_t concurrent_synchronous_requests = 0; + std::mutex concurrent_synchronous_requests_mutex; + ClientBase(const std::string &host_port, unsigned short default_port) : io_service(new asio::io_service()), connections(new std::vector>()), connections_mutex(new std::mutex()) { auto parsed_host_port = parse_host_port(host_port, default_port); diff --git a/http_examples.cpp b/http_examples.cpp index 3c98748..f6a1a10 100644 --- a/http_examples.cpp +++ b/http_examples.cpp @@ -221,7 +221,6 @@ int main() { if(!ec) cout << response->content.rdbuf() << endl; }); - client.io_service->reset(); // needed because the io_service has been run already in the synchronous examples client.io_service->run(); server_thread.join(); diff --git a/https_examples.cpp b/https_examples.cpp index a93b423..465e7b1 100644 --- a/https_examples.cpp +++ b/https_examples.cpp @@ -220,7 +220,6 @@ int main() { if(!ec) cout << response->content.rdbuf() << endl; }); - client.io_service->reset(); // needed because the io_service has been run already in the synchronous examples client.io_service->run(); server_thread.join(); diff --git a/tests/io_test.cpp b/tests/io_test.cpp index d0d75ef..c7bf3a0 100644 --- a/tests/io_test.cpp +++ b/tests/io_test.cpp @@ -179,7 +179,7 @@ int main() { assert(call); { - vector calls(100); + vector calls(100, 0); vector threads; for(size_t c = 0; c < 100; ++c) { calls[c] = 0; @@ -204,6 +204,34 @@ int main() { } } + /// Test concurrent synchronous request calls + { + HttpClient client("localhost:8080"); + { + vector calls(100, 0); + vector threads; + for(size_t c = 0; c < 100; ++c) { + calls[c] = 0; + threads.emplace_back([c, &client, &calls] { + try { + auto r = client.request("GET", "/match/123"); + assert(SimpleWeb::status_code(r->status_code) == SimpleWeb::StatusCode::success_ok); + assert(r->content.string() == "123"); + calls[c] = 1; + } + catch(...) { + assert(false); + } + }); + } + for(auto &thread : threads) + thread.join(); + assert(client.connections->size() == 1); + for(auto call : calls) + assert(call); + } + } + { HttpClient client("localhost:8080"); assert(client.connections->size() == 0);