From c6c7d0a6f2ba9f1cac478704b6b3658a1c3a85c3 Mon Sep 17 00:00:00 2001 From: eidheim Date: Wed, 19 Feb 2020 09:33:36 +0100 Subject: [PATCH] Related to !248: completed synchronous request() fixes --- asio_compatibility.hpp | 6 ++ client_http.hpp | 132 +++++++++++++++++++++++++---------------- http_examples.cpp | 40 +++++++------ https_examples.cpp | 40 +++++++------ tests/io_test.cpp | 66 ++++++++++----------- 5 files changed, 160 insertions(+), 124 deletions(-) diff --git a/asio_compatibility.hpp b/asio_compatibility.hpp index ca332c2..1e6648d 100644 --- a/asio_compatibility.hpp +++ b/asio_compatibility.hpp @@ -50,6 +50,9 @@ namespace SimpleWeb { void async_resolve(asio::ip::tcp::resolver &resolver, const std::pair &host_port, handler_type &&handler) { resolver.async_resolve(host_port.first, host_port.second, std::forward(handler)); } + asio::executor_work_guard make_work_guard(io_context &context) { + return asio::make_work_guard(context); + } #else using io_context = asio::io_service; using resolver_results = asio::ip::tcp::resolver::iterator; @@ -73,6 +76,9 @@ namespace SimpleWeb { void async_resolve(asio::ip::tcp::resolver &resolver, const std::pair &host_port, handler_type &&handler) { resolver.async_resolve(asio::ip::tcp::resolver::query(host_port.first, host_port.second), std::forward(handler)); } + io_context::work make_work_guard(io_context &context) { + return io_context::work(context); + } #endif } // namespace SimpleWeb diff --git a/client_http.hpp b/client_http.hpp index 9946dfb..afd1f64 100644 --- a/client_http.hpp +++ b/client_http.hpp @@ -3,8 +3,8 @@ #include "asio_compatibility.hpp" #include "mutex.hpp" -#include #include "utility.hpp" +#include #include #include #include @@ -210,25 +210,45 @@ namespace SimpleWeb { Config config; /// If you want to reuse an already created asio::io_service, store its pointer here before calling a request function. + /// Do not set when using synchronous request functions. std::shared_ptr io_service; - /// Convenience function to perform synchronous request. The io_service is run within this function. + /// Convenience function to perform synchronous request. The io_service is started in this function. + /// Should not be combined with asynchronous request functions. /// If you reuse the io_service for other tasks, use the asynchronous request functions instead. /// When requesting Server-Sent Events: will throw on error::eof, please use asynchronous request functions instead. std::shared_ptr request(const std::string &method, const std::string &path = {"/"}, string_view content = {}, const CaseInsensitiveMultimap &header = CaseInsensitiveMultimap()) { - start_internal_io_service_handler_thread(); - std::promise> response_promise; - std::future> response_future = response_promise.get_future(); + { + LockGuard lock(synchronous_request_mutex); + if(!synchronous_request_called) { + if(io_service) // Throw if io_service already set + throw make_error_code::make_error_code(errc::operation_not_permitted); + io_service = std::make_shared(); + internal_io_service = true; + auto io_service_ = io_service; + std::thread thread([io_service_] { + auto work = make_work_guard(*io_service_); + io_service_->run(); + }); + thread.detach(); + synchronous_request_called = true; + } + } + std::shared_ptr response; - error_code ec; - request(method, path, content, header, [&response_promise, response, ec](std::shared_ptr response_, const error_code &ec_) mutable { - if(ec_ && !ec) - ec = ec_; + std::promise> response_promise; + bool stop_future_handlers = false; + request(method, path, content, header, [&response, &response_promise, &stop_future_handlers](std::shared_ptr response_, error_code ec) { + if(stop_future_handlers) + return; + if(!response) response = response_; else if(!ec) { - if(response_->streambuf.size() + response->streambuf.size() > response->streambuf.max_size()) + if(response_->streambuf.size() + response->streambuf.size() > response->streambuf.max_size()) { ec = make_error_code::make_error_code(errc::message_size); + response->close(); + } else { // Move partial response_ content to response: auto &source = response_->streambuf; @@ -237,34 +257,54 @@ namespace SimpleWeb { source.consume(source.size()); } } - if(response_->content.end) { - if(!ec) - response_promise.set_value(response); - else - response_promise.set_exception(std::make_exception_ptr(system_error(ec))); + + if(ec) { + response_promise.set_exception(std::make_exception_ptr(system_error(ec))); + stop_future_handlers = true; } + else if(response_->content.end) + response_promise.set_value(response); }); - return response_future.get(); + return response_promise.get_future().get(); } - /// Convenience function to perform synchronous request. The io_service is run within this function. + /// Convenience function to perform synchronous request. The io_service is started in this function. + /// Should not be combined with asynchronous request functions. /// If you reuse the io_service for other tasks, use the asynchronous request functions instead. /// When requesting Server-Sent Events: will throw on error::eof, please use asynchronous request functions instead. std::shared_ptr request(const std::string &method, const std::string &path, std::istream &content, const CaseInsensitiveMultimap &header = CaseInsensitiveMultimap()) { - start_internal_io_service_handler_thread(); - std::promise> response_promise; - std::future> response_future = response_promise.get_future(); + { + LockGuard lock(synchronous_request_mutex); + if(!synchronous_request_called) { + if(io_service) // Throw if io_service already set + throw make_error_code::make_error_code(errc::operation_not_permitted); + io_service = std::make_shared(); + internal_io_service = true; + auto io_service_ = io_service; + std::thread thread([io_service_] { + auto work = make_work_guard(*io_service_); + io_service_->run(); + }); + thread.detach(); + synchronous_request_called = 1; + } + } + std::shared_ptr response; - error_code ec; - request(method, path, content, header, [&response_promise, response, ec](std::shared_ptr response_, const error_code &ec_) mutable { - if(ec_ && !ec) - ec = ec_; + std::promise> response_promise; + bool stop_future_handlers = false; + request(method, path, content, header, [&response, &response_promise, &stop_future_handlers](std::shared_ptr response_, error_code ec) { + if(stop_future_handlers) + return; + if(!response) response = response_; else if(!ec) { - if(response_->streambuf.size() + response->streambuf.size() > response->streambuf.max_size()) + if(response_->streambuf.size() + response->streambuf.size() > response->streambuf.max_size()) { ec = make_error_code::make_error_code(errc::message_size); + response->close(); + } else { // Move partial response_ content to response: auto &source = response_->streambuf; @@ -273,15 +313,16 @@ namespace SimpleWeb { source.consume(source.size()); } } - if(response_->content.end) { - if(!ec) - response_promise.set_value(response); - else - response_promise.set_exception(std::make_exception_ptr(system_error(ec))); + + if(ec) { + response_promise.set_exception(std::make_exception_ptr(system_error(ec))); + stop_future_handlers = true; } + else if(response_->content.end) + response_promise.set_value(response); }); - return response_future.get(); + return response_promise.get_future().get(); } /// Asynchronous request where running Client's io_service is required. @@ -434,21 +475,17 @@ namespace SimpleWeb { (*it)->close(); it = connections.erase(it); } - if(internal_io_service_handler_thread) { - io_service->stop(); - internal_io_service_handler_thread->join(); - internal_io_service_handler_thread.reset(); - io_service.reset(); - } } virtual ~ClientBase() noexcept { handler_runner->stop(); stop(); + if(internal_io_service) + io_service->stop(); } protected: - std::unique_ptr internal_io_service_handler_thread GUARDED_BY(connections_mutex); + bool internal_io_service = false; std::string host; unsigned short port; @@ -461,31 +498,22 @@ namespace SimpleWeb { std::shared_ptr handler_runner; + Mutex synchronous_request_mutex; + bool synchronous_request_called GUARDED_BY(synchronous_request_mutex) = false; + ClientBase(const std::string &host_port, unsigned short default_port) noexcept : default_port(default_port), handler_runner(new ScopeRunner()) { auto parsed_host_port = parse_host_port(host_port, default_port); host = parsed_host_port.first; port = parsed_host_port.second; } - void start_internal_io_service_handler_thread() noexcept { - LockGuard lock(connections_mutex); - - if(!io_service) { - io_service = std::make_shared(); - internal_io_service_handler_thread = std::unique_ptr(new std::thread([this]{ - // Ensure that the io_service does not stop - asio::io_service::work dummy_work(*io_service); - io_service->run(); - })); - } - } - std::shared_ptr get_connection() noexcept { std::shared_ptr connection; LockGuard lock(connections_mutex); if(!io_service) { - io_service = std::make_shared(); + io_service = std::make_shared(); + internal_io_service = true; } for(auto it = connections.begin(); it != connections.end(); ++it) { diff --git a/http_examples.cpp b/http_examples.cpp index fc867c8..f23b710 100644 --- a/http_examples.cpp +++ b/http_examples.cpp @@ -217,31 +217,35 @@ int main() { cout << "Server listening on port " << server_port.get_future().get() << endl << endl; // Client examples - HttpClient client("localhost:8080"); - string json_string = "{\"firstName\": \"John\",\"lastName\": \"Smith\",\"age\": 25}"; // Synchronous request examples - try { - cout << "Example GET request to http://localhost:8080/match/123" << endl; - auto r1 = client.request("GET", "/match/123"); - cout << "Response content: " << r1->content.rdbuf() << endl << endl; // Alternatively, use the convenience function r1->content.string() + { + HttpClient client("localhost:8080"); + try { + cout << "Example GET request to http://localhost:8080/match/123" << endl; + auto r1 = client.request("GET", "/match/123"); + cout << "Response content: " << r1->content.rdbuf() << endl << endl; // Alternatively, use the convenience function r1->content.string() - cout << "Example POST request to http://localhost:8080/string" << endl; - auto r2 = client.request("POST", "/string", json_string); - cout << "Response content: " << r2->content.rdbuf() << endl << endl; - } - catch(const SimpleWeb::system_error &e) { - cerr << "Client request error: " << e.what() << endl; + cout << "Example POST request to http://localhost:8080/string" << endl; + auto r2 = client.request("POST", "/string", json_string); + cout << "Response content: " << r2->content.rdbuf() << endl << endl; + } + catch(const SimpleWeb::system_error &e) { + cerr << "Client request error: " << e.what() << endl; + } } // Asynchronous request example - cout << "Example POST request to http://localhost:8080/json" << endl; - client.request("POST", "/json", json_string, [](shared_ptr response, const SimpleWeb::error_code &ec) { - if(!ec) - cout << "Response content: " << response->content.rdbuf() << endl; - }); - client.io_service->run(); + { + HttpClient client("localhost:8080"); + cout << "Example POST request to http://localhost:8080/json" << endl; + client.request("POST", "/json", json_string, [](shared_ptr response, const SimpleWeb::error_code &ec) { + if(!ec) + cout << "Response content: " << response->content.rdbuf() << endl; + }); + client.io_service->run(); + } server_thread.join(); } diff --git a/https_examples.cpp b/https_examples.cpp index 2200e69..fa64bba 100644 --- a/https_examples.cpp +++ b/https_examples.cpp @@ -215,31 +215,35 @@ int main() { cout << "Server listening on port " << server_port.get_future().get() << endl << endl; // Client examples - HttpsClient client("localhost:8080", false); // Second create() parameter set to false: no certificate verification - string json_string = "{\"firstName\": \"John\",\"lastName\": \"Smith\",\"age\": 25}"; // Synchronous request examples - try { - cout << "Example GET request to https://localhost:8080/match/123" << endl; - auto r1 = client.request("GET", "/match/123"); - cout << "Response content: " << r1->content.rdbuf() << endl << endl; // Alternatively, use the convenience function r1->content.string() + { + HttpsClient client("localhost:8080", false); + try { + cout << "Example GET request to http://localhost:8080/match/123" << endl; + auto r1 = client.request("GET", "/match/123"); + cout << "Response content: " << r1->content.rdbuf() << endl << endl; // Alternatively, use the convenience function r1->content.string() - cout << "Example POST request to https://localhost:8080/string" << endl; - auto r2 = client.request("POST", "/string", json_string); - cout << "Response content: " << r2->content.rdbuf() << endl << endl; - } - catch(const SimpleWeb::system_error &e) { - cerr << "Client request error: " << e.what() << endl; + cout << "Example POST request to http://localhost:8080/string" << endl; + auto r2 = client.request("POST", "/string", json_string); + cout << "Response content: " << r2->content.rdbuf() << endl << endl; + } + catch(const SimpleWeb::system_error &e) { + cerr << "Client request error: " << e.what() << endl; + } } // Asynchronous request example - cout << "Example POST request to https://localhost:8080/json" << endl; - client.request("POST", "/json", json_string, [](shared_ptr response, const SimpleWeb::error_code &ec) { - if(!ec) - cout << "Response content: " << response->content.rdbuf() << endl; - }); - client.io_service->run(); + { + HttpsClient client("localhost:8080", false); + cout << "Example POST request to http://localhost:8080/json" << endl; + client.request("POST", "/json", json_string, [](shared_ptr response, const SimpleWeb::error_code &ec) { + if(!ec) + cout << "Response content: " << response->content.rdbuf() << endl; + }); + client.io_service->run(); + } server_thread.join(); } diff --git a/tests/io_test.cpp b/tests/io_test.cpp index d8492dd..160daf8 100644 --- a/tests/io_test.cpp +++ b/tests/io_test.cpp @@ -389,27 +389,25 @@ int main() { // Test large responses { - // Don't mix synchronous and asynchronous on the same client - HttpClient sync_request_client("localhost:8080"); - HttpClient async_request_client("localhost:8080"); - sync_request_client.config.max_response_streambuf_size = 400; - async_request_client.config.max_response_streambuf_size = 400; - async_request_client.io_service = std::make_shared(); { + HttpClient client("localhost:8080"); + client.config.max_response_streambuf_size = 400; bool thrown = false; try { - auto r = sync_request_client.request("GET", "/long-response"); + auto r = client.request("GET", "/long-response"); } catch(...) { thrown = true; } ASSERT(thrown); } + HttpClient client("localhost:8080"); + client.config.max_response_streambuf_size = 400; { size_t calls = 0; bool end = false; std::string content; - async_request_client.request("GET", "/long-response", [&calls, &content, &end](shared_ptr response, const SimpleWeb::error_code &ec) { + client.request("GET", "/long-response", [&calls, &content, &end](shared_ptr response, const SimpleWeb::error_code &ec) { ASSERT(!ec); content += response->content.string(); calls++; @@ -417,8 +415,7 @@ int main() { ASSERT(response->content.end == false); end = response->content.end; }); - SimpleWeb::restart(*async_request_client.io_service); - async_request_client.io_service->run(); + client.io_service->run(); ASSERT(content == long_response); ASSERT(calls > 2); ASSERT(end == true); @@ -426,15 +423,15 @@ int main() { { size_t calls = 0; std::string content; - async_request_client.request("GET", "/long-response", [&calls, &content](shared_ptr response, const SimpleWeb::error_code &ec) { + client.request("GET", "/long-response", [&calls, &content](shared_ptr response, const SimpleWeb::error_code &ec) { if(calls == 0) ASSERT(!ec); content += response->content.string(); calls++; response->close(); }); - SimpleWeb::restart(*async_request_client.io_service); - async_request_client.io_service->run(); + SimpleWeb::restart(*client.io_service); + client.io_service->run(); ASSERT(!content.empty()); ASSERT(calls >= 2); } @@ -442,31 +439,28 @@ int main() { // Test client timeout { - // Don't mix synchronous and asynchronous on the same client - HttpClient sync_request_client("localhost:8080"); - HttpClient async_request_client("localhost:8080"); - sync_request_client.config.timeout = 2; - async_request_client.config.timeout = 2; - { - bool thrown = false; - try { - auto r = sync_request_client.request("GET", "/work"); - } - catch(...) { - thrown = true; - } - ASSERT(thrown); + HttpClient client("localhost:8080"); + client.config.timeout = 2; + bool thrown = false; + try { + auto r = client.request("GET", "/work"); } - { - bool call = false; - async_request_client.request("GET", "/work", [&call](shared_ptr /*response*/, const SimpleWeb::error_code &ec) { - ASSERT(ec); - call = true; - }); - SimpleWeb::restart(*async_request_client.io_service); - async_request_client.io_service->run(); - ASSERT(call); + catch(...) { + thrown = true; } + ASSERT(thrown); + } + { + HttpClient client("localhost:8080"); + client.config.timeout = 2; + bool call = false; + client.request("GET", "/work", [&call](shared_ptr /*response*/, const SimpleWeb::error_code &ec) { + ASSERT(ec); + call = true; + }); + SimpleWeb::restart(*client.io_service); + client.io_service->run(); + ASSERT(call); } // Test asynchronous requests