From fd06a69580dbebff02b08a7f5b68b2a9725bf1db Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 25 Jul 2018 19:03:50 -0700 Subject: [PATCH 01/14] Microperf: Use lock_guard instead of unique_lock. --- Release/src/http/client/http_client_asio.cpp | 97 ++++++++++---------- 1 file changed, 49 insertions(+), 48 deletions(-) diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index 4ba3e0851b..c7f4365b49 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -173,14 +173,15 @@ class asio_connection template void async_connect(const Iterator &begin, const Handler &handler) { - std::unique_lock lock(m_socket_lock); - if (!m_closed) - m_socket.async_connect(begin, handler); - else { - lock.unlock(); - handler(boost::asio::error::operation_aborted); - } + std::lock_guard lock(m_socket_lock); + if (!m_closed) { + m_socket.async_connect(begin, handler); + return; + } + } // unlock + + handler(boost::asio::error::operation_aborted); } template @@ -462,14 +463,14 @@ class asio_context : public request_context, public std::enable_shared_from_this ctx->m_timer.set_ctx(std::weak_ptr(ctx)); return ctx; } - + class ssl_proxy_tunnel : public std::enable_shared_from_this { public: ssl_proxy_tunnel(std::shared_ptr context, std::function)> ssl_tunnel_established) : m_ssl_tunnel_established(ssl_tunnel_established), m_context(context) {} - + void start_proxy_connect() { auto proxy = m_context->m_http_client->client_config().proxy(); @@ -504,7 +505,7 @@ class asio_context : public request_context, public std::enable_shared_from_this client->m_resolver.async_resolve(query, boost::bind(&ssl_proxy_tunnel::handle_resolve, shared_from_this(), boost::asio::placeholders::error, boost::asio::placeholders::iterator)); } - private: + private: void handle_resolve(const boost::system::error_code& ec, tcp::resolver::iterator endpoints) { if (ec) @@ -555,7 +556,7 @@ class asio_context : public request_context, public std::enable_shared_from_this m_context->report_error("Failed to send connect request to proxy.", err, httpclient_errorcode_context::writebody); } } - + void handle_status_line(const boost::system::error_code& ec) { if (!ec) @@ -567,13 +568,13 @@ class asio_context : public request_context, public std::enable_shared_from_this response_stream >> http_version; status_code status_code; response_stream >> status_code; - + if (!response_stream || http_version.substr(0, 5) != "HTTP/") { m_context->report_error("Invalid HTTP status line during proxy connection", ec, httpclient_errorcode_context::readheader); return; } - + if (status_code != 200) { m_context->report_error("Expected a 200 response from proxy, received: " + to_string(status_code), ec, httpclient_errorcode_context::readheader); @@ -595,14 +596,14 @@ class asio_context : public request_context, public std::enable_shared_from_this // Failed to write to socket because connection was already closed while it was in the pool. // close() here ensures socket is closed in a robust way and prevents the connection from being put to the pool again. m_context->m_connection->close(); - + // Create a new context and copy the request object, completion event and // cancellation registration to maintain the old state. // This also obtains a new connection from pool. auto new_ctx = m_context->create_request_context(m_context->m_http_client, m_context->m_request); new_ctx->m_request_completion = m_context->m_request_completion; new_ctx->m_cancellationRegistration = m_context->m_cancellationRegistration; - + auto client = std::static_pointer_cast(m_context->m_http_client); // Resend the request using the new context. client->send_request(new_ctx); @@ -613,15 +614,15 @@ class asio_context : public request_context, public std::enable_shared_from_this } } } - + std::function)> m_ssl_tunnel_established; std::shared_ptr m_context; - + boost::asio::streambuf m_request; boost::asio::streambuf m_response; }; - - + + enum class http_proxy_type { none, @@ -636,11 +637,11 @@ class asio_context : public request_context, public std::enable_shared_from_this request_context::report_error(make_error_code(std::errc::operation_canceled).value(), "Request canceled by user."); return; } - + http_proxy_type proxy_type = http_proxy_type::none; std::string proxy_host; int proxy_port = -1; - + // There is no support for auto-detection of proxies on non-windows platforms, it must be specified explicitly from the client code. if (m_http_client->client_config().proxy().is_specified()) { @@ -650,7 +651,7 @@ class asio_context : public request_context, public std::enable_shared_from_this proxy_port = proxy_uri.port() == -1 ? 8080 : proxy_uri.port(); proxy_host = utility::conversions::to_utf8string(proxy_uri.host()); } - + auto start_http_request_flow = [proxy_type, proxy_host, proxy_port AND_CAPTURE_MEMBER_FUNCTION_POINTERS](std::shared_ptr ctx) { if (ctx->m_request._cancellation_token().is_canceled()) @@ -658,20 +659,20 @@ class asio_context : public request_context, public std::enable_shared_from_this ctx->request_context::report_error(make_error_code(std::errc::operation_canceled).value(), "Request canceled by user."); return; } - + const auto &base_uri = ctx->m_http_client->base_uri(); const auto full_uri = uri_builder(base_uri).append(ctx->m_request.relative_uri()).to_uri(); - + // For a normal http proxy, we need to specify the full request uri, otherwise just specify the resource auto encoded_resource = proxy_type == http_proxy_type::http ? full_uri.to_string() : full_uri.resource().to_string(); - + if (encoded_resource.empty()) { encoded_resource = U("/"); } - + const auto &method = ctx->m_request.method(); - + // stop injection of headers via method // resource should be ok, since it's been encoded // and host won't resolve @@ -680,20 +681,20 @@ class asio_context : public request_context, public std::enable_shared_from_this ctx->report_exception(http_exception("The method string is invalid.")); return; } - + std::ostream request_stream(&ctx->m_body_buf); request_stream.imbue(std::locale::classic()); const auto &host = utility::conversions::to_utf8string(base_uri.host()); - + request_stream << utility::conversions::to_utf8string(method) << " " << utility::conversions::to_utf8string(encoded_resource) << " " << "HTTP/1.1" << CRLF; - + int port = base_uri.port(); - + if (base_uri.is_port_default()) { port = (ctx->m_connection->is_ssl() ? 443 : 80); } - + // Add the Host header if user has not specified it explicitly if (!ctx->m_request.headers().has(header_names::host)) { @@ -703,10 +704,10 @@ class asio_context : public request_context, public std::enable_shared_from_this } request_stream << CRLF; } - + // Extra request headers are constructed here. std::string extra_headers; - + // Add header for basic proxy authentication if (proxy_type == http_proxy_type::http && ctx->m_http_client->client_config().proxy().credentials().is_set()) { @@ -746,25 +747,25 @@ class asio_context : public request_context, public std::enable_shared_from_this extra_headers.append("Content-Length: 0\r\n"); } } - + if (proxy_type == http_proxy_type::http) { extra_headers.append( "Cache-Control: no-store, no-cache\r\n" "Pragma: no-cache\r\n"); } - + request_stream << utility::conversions::to_utf8string(::web::http::details::flatten_http_headers(ctx->m_request.headers())); request_stream << extra_headers; // Enforce HTTP connection keep alive (even for the old HTTP/1.0 protocol). request_stream << "Connection: Keep-Alive" << CRLF << CRLF; - + // Start connection timeout timer. if (!ctx->m_timer.has_started()) { ctx->m_timer.start(); } - + if (ctx->m_connection->is_reused() || proxy_type == http_proxy_type::ssl_tunnel) { // If socket is a reused connection or we're connected via an ssl-tunneling proxy, try to write the request directly. In both cases we have already established a tcp connection. @@ -774,16 +775,16 @@ class asio_context : public request_context, public std::enable_shared_from_this { // If the connection is new (unresolved and unconnected socket), then start async // call to resolve first, leading eventually to request write. - + // For normal http proxies, we want to connect directly to the proxy server. It will relay our request. auto tcp_host = proxy_type == http_proxy_type::http ? proxy_host : host; auto tcp_port = proxy_type == http_proxy_type::http ? proxy_port : port; - + tcp::resolver::query query(tcp_host, to_string(tcp_port)); auto client = std::static_pointer_cast(ctx->m_http_client); client->m_resolver.async_resolve(query, boost::bind(&asio_context::handle_resolve, ctx, boost::asio::placeholders::error, boost::asio::placeholders::iterator)); } - + // Register for notification on cancellation to abort this request. if (ctx->m_request._cancellation_token() != pplx::cancellation_token::none()) { @@ -800,7 +801,7 @@ class asio_context : public request_context, public std::enable_shared_from_this }); } }; - + if (proxy_type == http_proxy_type::ssl_tunnel) { // The ssl_tunnel_proxy keeps the context alive and then calls back once the ssl tunnel is established via 'start_http_request_flow' @@ -1079,7 +1080,7 @@ class asio_context : public request_context, public std::enable_shared_from_this // Reuse error handling. return handle_write_body(ec); } - + m_timer.reset(); const auto &progress = m_request._get_impl()->_progress_handler(); if (progress) @@ -1160,7 +1161,7 @@ class asio_context : public request_context, public std::enable_shared_from_this response_stream >> http_version; status_code status_code; response_stream >> status_code; - + std::string status_message; std::getline(response_stream, status_message); @@ -1432,7 +1433,7 @@ class asio_context : public request_context, public std::enable_shared_from_this } this_request->m_body_buf.consume(to_read + CRLF.size()); // consume crlf this_request->m_connection->async_read_until(this_request->m_body_buf, CRLF, boost::bind(&asio_context::handle_chunk_header, this_request, boost::asio::placeholders::error)); - }); + }); } } } @@ -1485,7 +1486,7 @@ class asio_context : public request_context, public std::enable_shared_from_this if(m_decompressor) { auto decompressed = m_decompressor->decompress(boost::asio::buffer_cast(m_body_buf.data()), read_size); - + if (m_decompressor->has_error()) { this_request->report_exception(std::runtime_error("Failed to decompress the response body")); @@ -1660,7 +1661,7 @@ class asio_context : public request_context, public std::enable_shared_from_this timeout_timer m_timer; boost::asio::streambuf m_body_buf; std::shared_ptr m_connection; - + std::unique_ptr m_decompressor; #if defined(__APPLE__) || (defined(ANDROID) || defined(__ANDROID__)) @@ -1684,7 +1685,7 @@ void asio_client::send_request(const std::shared_ptr &request_c { client_config().invoke_nativehandle_options(ctx->m_connection->m_ssl_stream.get()); } - else + else { client_config().invoke_nativehandle_options(&(ctx->m_connection->m_socket)); } From ccf8e165b30f0bfc3ab5ed450209ca389a6d2a57 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 25 Jul 2018 19:27:53 -0700 Subject: [PATCH 02/14] Bill hates lock_guard more. --- .../broadcast_server/broadcast_server.cpp | 16 ++++++++-------- .../src/http/listener/http_server_httpsys.cpp | 12 ++++++------ Release/src/websockets/client/ws_client_wspp.cpp | 12 ++++++------ 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/Release/libs/websocketpp/examples/broadcast_server/broadcast_server.cpp b/Release/libs/websocketpp/examples/broadcast_server/broadcast_server.cpp index e87e879eee..36087d31dd 100644 --- a/Release/libs/websocketpp/examples/broadcast_server/broadcast_server.cpp +++ b/Release/libs/websocketpp/examples/broadcast_server/broadcast_server.cpp @@ -19,7 +19,7 @@ using websocketpp::lib::bind; using websocketpp::lib::thread; using websocketpp::lib::mutex; -using websocketpp::lib::unique_lock; +using websocketpp::lib::lock_guard; using websocketpp::lib::condition_variable; /* on_open insert connection_hdl into channel @@ -71,7 +71,7 @@ class broadcast_server { } void on_open(connection_hdl hdl) { - unique_lock lock(m_action_lock); + lock_guard lock(m_action_lock); //std::cout << "on_open" << std::endl; m_actions.push(action(SUBSCRIBE,hdl)); lock.unlock(); @@ -79,7 +79,7 @@ class broadcast_server { } void on_close(connection_hdl hdl) { - unique_lock lock(m_action_lock); + lock_guard lock(m_action_lock); //std::cout << "on_close" << std::endl; m_actions.push(action(UNSUBSCRIBE,hdl)); lock.unlock(); @@ -88,7 +88,7 @@ class broadcast_server { void on_message(connection_hdl hdl, server::message_ptr msg) { // queue message up for sending by processing thread - unique_lock lock(m_action_lock); + lock_guard lock(m_action_lock); //std::cout << "on_message" << std::endl; m_actions.push(action(MESSAGE,hdl,msg)); lock.unlock(); @@ -97,7 +97,7 @@ class broadcast_server { void process_messages() { while(1) { - unique_lock lock(m_action_lock); + lock_guard lock(m_action_lock); while(m_actions.empty()) { m_action_cond.wait(lock); @@ -109,13 +109,13 @@ class broadcast_server { lock.unlock(); if (a.type == SUBSCRIBE) { - unique_lock con_lock(m_connection_lock); + lock_guard con_lock(m_connection_lock); m_connections.insert(a.hdl); } else if (a.type == UNSUBSCRIBE) { - unique_lock con_lock(m_connection_lock); + lock_guard con_lock(m_connection_lock); m_connections.erase(a.hdl); } else if (a.type == MESSAGE) { - unique_lock con_lock(m_connection_lock); + lock_guard con_lock(m_connection_lock); con_list::iterator it; for (it = m_connections.begin(); it != m_connections.end(); ++it) { diff --git a/Release/src/http/listener/http_server_httpsys.cpp b/Release/src/http/listener/http_server_httpsys.cpp index 7d3cea203c..1da43e7a97 100644 --- a/Release/src/http/listener/http_server_httpsys.cpp +++ b/Release/src/http/listener/http_server_httpsys.cpp @@ -482,7 +482,7 @@ windows_request_context::~windows_request_context() // Bug is that task_completion_event accesses internal state after setting. // Workaround is to use a lock incurring additional synchronization, if can acquire // the lock then setting of the event has completed. - std::unique_lock lock(m_responseCompletedLock); + std::lock_guard lock(m_responseCompletedLock); // Add a task-based continuation so no exceptions thrown from the task go 'unobserved'. pplx::create_task(m_response_completed).then([](pplx::task t) @@ -759,7 +759,7 @@ void windows_request_context::init_response_callbacks(ShouldWaitForBody shouldWa catch (...) { // Should never get here, if we do there's a chance that a circular reference will cause leaks, - // or worse, undefined behaviour as we don't know who owns 'this' anymore + // or worse, undefined behaviour as we don't know who owns 'this' anymore _ASSERTE(false); m_response = http::http_response(status_codes::InternalError); } @@ -899,7 +899,7 @@ void windows_request_context::transmit_body() if ( !m_sending_in_chunks && !m_transfer_encoding ) { // We are done sending data. - std::unique_lock lock(m_responseCompletedLock); + std::lock_guard lock(m_responseCompletedLock); m_response_completed.set(); return; } @@ -1024,7 +1024,7 @@ void windows_request_context::send_response_body_io_completion(DWORD error_code, /// void windows_request_context::cancel_request_io_completion(DWORD, DWORD) { - std::unique_lock lock(m_responseCompletedLock); + std::lock_guard lock(m_responseCompletedLock); m_response_completed.set_exception(m_except_ptr); } @@ -1047,7 +1047,7 @@ void windows_request_context::cancel_request(std::exception_ptr except_ptr) if(error_code != NO_ERROR && error_code != ERROR_IO_PENDING) { CancelThreadpoolIo(pServer->m_threadpool_io); - std::unique_lock lock(m_responseCompletedLock); + std::lock_guard lock(m_responseCompletedLock); m_response_completed.set_exception(except_ptr); } } @@ -1059,4 +1059,4 @@ std::unique_ptr make_http_httpsys_server() }}}} -#endif +#endif diff --git a/Release/src/websockets/client/ws_client_wspp.cpp b/Release/src/websockets/client/ws_client_wspp.cpp index 8f2658a8f5..d2ea9e8dd6 100644 --- a/Release/src/websockets/client/ws_client_wspp.cpp +++ b/Release/src/websockets/client/ws_client_wspp.cpp @@ -134,22 +134,23 @@ class wspp_callback_client : public websocket_client_callback_impl, public std:: ~wspp_callback_client() CPPREST_NOEXCEPT { _ASSERTE(m_state < DESTROYED); - std::unique_lock lock(m_wspp_client_lock); + State localState; + { + std::lock_guard lock(m_wspp_client_lock); + localState = m_state; + } // Unlock the mutex so connect/close can use it. // Now, what states could we be in? - switch (m_state) { + switch (localState) { case DESTROYED: // This should be impossible std::abort(); case CREATED: - lock.unlock(); break; case CLOSED: case CONNECTING: case CONNECTED: case CLOSING: - // Unlock the mutex so connect/close can use it. - lock.unlock(); try { // This will do nothing in the already-connected case @@ -806,4 +807,3 @@ websocket_callback_client::websocket_callback_client(websocket_client_config con }}} #endif - From f1a167871264ae0f2c3d5ff154d4e96602142b6e Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 25 Jul 2018 21:46:49 -0700 Subject: [PATCH 03/14] Ref them connections. --- Release/src/http/client/http_client_asio.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index c7f4365b49..6fef66c741 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -357,24 +357,25 @@ class asio_connection_pool : public std::enable_shared_from_this lock(self.m_lock); if (self.m_prev_epoch == self.m_epoch) { - self.m_connections.clear(); + connections.clear(); self.is_timer_running = false; return; } else { auto prev_epoch = self.m_prev_epoch; - auto erase_end = std::find_if(self.m_connections.begin(), self.m_connections.end(), + auto erase_end = std::find_if(connections.begin(), connections.end(), [prev_epoch](std::pair>& p) { return p.first > prev_epoch; }); - self.m_connections.erase(self.m_connections.begin(), erase_end); + connections.erase(connections.begin(), erase_end); start_epoch_interval(pool); } }); From 0c165922bef74b31e403ba56391fb8016728df9d Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 25 Jul 2018 21:48:49 -0700 Subject: [PATCH 04/14] Demacroize. --- Release/src/http/client/http_client_asio.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index 6fef66c741..b67dd32238 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -82,7 +82,7 @@ using utility::conversions::details::to_string; using std::to_string; #endif -#define CRLF std::string("\r\n") +const std::string CRLF("\r\n"); namespace web { namespace http { From a36e368fc6562f728fd29c3ba403bb88be9e0632 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 25 Jul 2018 21:51:31 -0700 Subject: [PATCH 05/14] Commonize port and merge some CRLFs into adjacent string literals. --- Release/src/http/client/http_client_asio.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index b67dd32238..d86965ee24 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -482,14 +482,15 @@ class asio_context : public request_context, public std::enable_shared_from_this const auto &base_uri = m_context->m_http_client->base_uri(); const auto &host = utility::conversions::to_utf8string(base_uri.host()); - const auto &port = base_uri.port(); + const int portRaw = base_uri.port(); + const int port = (portRaw != 0) ? portRaw : 443; std::ostream request_stream(&m_request); request_stream.imbue(std::locale::classic()); - request_stream << "CONNECT " << host << ":" << ((port != 0) ? port : 443) << " HTTP/1.1" << CRLF; - request_stream << "Host: " << host << ":" << ((port != 0) ? port : 443) << CRLF; - request_stream << "Proxy-Connection: Keep-Alive" << CRLF; + request_stream << "CONNECT " << host << ":" << port << " HTTP/1.1\r\n"; + request_stream << "Host: " << host << ":" << port << CRLF; + request_stream << "Proxy-Connection: Keep-Alive\r\n"; if(m_context->m_http_client->client_config().proxy().credentials().is_set()) { @@ -687,7 +688,7 @@ class asio_context : public request_context, public std::enable_shared_from_this request_stream.imbue(std::locale::classic()); const auto &host = utility::conversions::to_utf8string(base_uri.host()); - request_stream << utility::conversions::to_utf8string(method) << " " << utility::conversions::to_utf8string(encoded_resource) << " " << "HTTP/1.1" << CRLF; + request_stream << utility::conversions::to_utf8string(method) << " " << utility::conversions::to_utf8string(encoded_resource) << " " << "HTTP/1.1\r\n"; int port = base_uri.port(); @@ -759,7 +760,7 @@ class asio_context : public request_context, public std::enable_shared_from_this request_stream << utility::conversions::to_utf8string(::web::http::details::flatten_http_headers(ctx->m_request.headers())); request_stream << extra_headers; // Enforce HTTP connection keep alive (even for the old HTTP/1.0 protocol). - request_stream << "Connection: Keep-Alive" << CRLF << CRLF; + request_stream << "Connection: Keep-Alive\r\n\r\n"; // Start connection timeout timer. if (!ctx->m_timer.has_started()) @@ -1339,7 +1340,7 @@ class asio_context : public request_context, public std::enable_shared_from_this } else { - async_read_until_buffersize(octets + CRLF.size(), // + 2 for crlf + async_read_until_buffersize(octets + CRLF.size(), boost::bind(&asio_context::handle_chunk, shared_from_this(), boost::asio::placeholders::error, octets)); } } From 0ff6d4a187f53afd7425a5be94290389a7a1b2db Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Thu, 26 Jul 2018 17:47:50 -0700 Subject: [PATCH 06/14] Add missing template keyword. --- Release/include/cpprest/streams.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Release/include/cpprest/streams.h b/Release/include/cpprest/streams.h index cb7cd4d0ea..d2923d3774 100644 --- a/Release/include/cpprest/streams.h +++ b/Release/include/cpprest/streams.h @@ -1748,7 +1748,7 @@ class type_parser parse(streams::streambuf buffer) { - return base::_parse_input,std::basic_string>(buffer, _accept_char, _extract_result); + return base::template _parse_input,std::basic_string>(buffer, _accept_char, _extract_result); } private: From 719e40dba346fb56bb44c677284bcec6020388bd Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Fri, 27 Jul 2018 14:38:53 -0700 Subject: [PATCH 07/14] Avoid stringstream in constructing the proxy_str. --- Release/include/cpprest/details/basic_types.h | 2 ++ Release/src/http/client/http_client_winhttp.cpp | 12 ++++-------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/Release/include/cpprest/details/basic_types.h b/Release/include/cpprest/details/basic_types.h index eadba01684..20555b45b7 100644 --- a/Release/include/cpprest/details/basic_types.h +++ b/Release/include/cpprest/details/basic_types.h @@ -50,6 +50,7 @@ typedef uint32_t HRESULT; // Needed for PPLX typedef wchar_t char_t ; typedef std::wstring string_t; #define _XPLATSTR(x) L ## x +#define _XPLATTOSTR(x) std::to_wstring(x) typedef std::wostringstream ostringstream_t; typedef std::wofstream ofstream_t; typedef std::wostream ostream_t; @@ -67,6 +68,7 @@ typedef std::wstringstream stringstream_t; typedef char char_t; typedef std::string string_t; #define _XPLATSTR(x) x +#define _XPLATTOSTR(x) std::to_string(x) typedef std::ostringstream ostringstream_t; typedef std::ofstream ofstream_t; typedef std::ostream ostream_t; diff --git a/Release/src/http/client/http_client_winhttp.cpp b/Release/src/http/client/http_client_winhttp.cpp index 2417859956..14bb7138f6 100644 --- a/Release/src/http/client/http_client_winhttp.cpp +++ b/Release/src/http/client/http_client_winhttp.cpp @@ -489,17 +489,13 @@ class winhttp_client : public _http_client_communicator } else { + proxy_str = uri.host(); if (uri.port() > 0) { - utility::ostringstream_t ss; - ss.imbue(std::locale::classic()); - ss << uri.host() << _XPLATSTR(":") << uri.port(); - proxy_str = ss.str(); - } - else - { - proxy_str = uri.host(); + proxy_str.push_back(_XPLATSTR(':')); + proxy_str.append(_XPLATTOSTR(uri.port())); } + proxy_name = proxy_str.c_str(); } } From 34d85cf2dbc58e94f478513c9bccbf12cee25262 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 30 Jul 2018 13:18:04 -0700 Subject: [PATCH 08/14] Remove unused forward declarations of some HTTP things. --- Release/include/cpprest/details/web_utilities.h | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/Release/include/cpprest/details/web_utilities.h b/Release/include/cpprest/details/web_utilities.h index ba6416546e..f99a8aa933 100644 --- a/Release/include/cpprest/details/web_utilities.h +++ b/Release/include/cpprest/details/web_utilities.h @@ -14,17 +14,6 @@ namespace web { - -namespace http { namespace client { namespace details { -class winhttp_client; -class winrt_client; -class asio_context; -}}} -namespace websockets { namespace client { namespace details { -class winrt_callback_client; -class wspp_callback_client; -}}} - namespace details { From 3d9b3f849dc833e1cdf282c0d7b589b6ba963686 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 30 Jul 2018 13:21:24 -0700 Subject: [PATCH 09/14] Use NOMINMAX instead of undef min and max everywhere. --- Release/src/http/client/http_client_asio.cpp | 3 --- Release/src/http/client/http_client_winhttp.cpp | 4 ---- Release/src/http/client/http_client_winrt.cpp | 3 --- Release/src/http/common/http_msg.cpp | 4 ---- Release/src/http/listener/http_server_asio.cpp | 7 ++----- Release/src/http/listener/http_server_httpsys.cpp | 3 --- Release/src/json/json.cpp | 4 ---- Release/src/pch/stdafx.h | 3 ++- Release/src/websockets/client/ws_client_wspp.cpp | 4 ---- 9 files changed, 4 insertions(+), 31 deletions(-) diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index d86965ee24..ecb749d63e 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -15,9 +15,6 @@ #include "stdafx.h" -#undef min -#undef max - #include "cpprest/asyncrt_utils.h" #include "../common/internal_http_helpers.h" diff --git a/Release/src/http/client/http_client_winhttp.cpp b/Release/src/http/client/http_client_winhttp.cpp index 14bb7138f6..e23a40a120 100644 --- a/Release/src/http/client/http_client_winhttp.cpp +++ b/Release/src/http/client/http_client_winhttp.cpp @@ -21,9 +21,6 @@ #include #endif -#undef min -#undef max - namespace web { namespace http @@ -1560,4 +1557,3 @@ std::shared_ptr<_http_client_communicator> create_platform_final_pipeline_stage( } }}}} - diff --git a/Release/src/http/client/http_client_winrt.cpp b/Release/src/http/client/http_client_winrt.cpp index 4ada65a75b..697f48a337 100644 --- a/Release/src/http/client/http_client_winrt.cpp +++ b/Release/src/http/client/http_client_winrt.cpp @@ -28,9 +28,6 @@ using namespace std; using namespace Platform; using namespace Microsoft::WRL; -#undef min -#undef max - namespace web { namespace http diff --git a/Release/src/http/common/http_msg.cpp b/Release/src/http/common/http_msg.cpp index ad2c70e9d7..4ad489dc01 100644 --- a/Release/src/http/common/http_msg.cpp +++ b/Release/src/http/common/http_msg.cpp @@ -13,9 +13,6 @@ #include "stdafx.h" #include "../common/internal_http_helpers.h" -#undef min -#undef max - using namespace web; using namespace utility; using namespace concurrency; @@ -1081,4 +1078,3 @@ const http_version http_versions::HTTP_1_1 = { 1, 1 }; #undef DAT #endif }} // namespace web::http - diff --git a/Release/src/http/listener/http_server_asio.cpp b/Release/src/http/listener/http_server_asio.cpp index 78193fa3e2..1914d63357 100644 --- a/Release/src/http/listener/http_server_asio.cpp +++ b/Release/src/http/listener/http_server_asio.cpp @@ -14,9 +14,6 @@ */ #include "stdafx.h" -#undef min -#undef max - #include #include #include @@ -1033,7 +1030,7 @@ will_deref_and_erase_t asio_server_connection::cancel_sending_response_with_erro { auto * context = static_cast(response._get_server_context()); context->m_response_completed.set_exception(eptr); - + // always terminate the connection since error happens return finish_request_response(); } @@ -1044,7 +1041,7 @@ will_deref_and_erase_t asio_server_connection::handle_write_chunked_response(con { return handle_response_written(response, ec); } - + auto readbuf = response._get_impl()->instream().streambuf(); if (readbuf.is_eof()) { diff --git a/Release/src/http/listener/http_server_httpsys.cpp b/Release/src/http/listener/http_server_httpsys.cpp index 1da43e7a97..8da74d0083 100644 --- a/Release/src/http/listener/http_server_httpsys.cpp +++ b/Release/src/http/listener/http_server_httpsys.cpp @@ -22,9 +22,6 @@ #include "http_server_httpsys.h" #include "http_server_impl.h" -#undef min -#undef max - using namespace web; using namespace utility; using namespace concurrency; diff --git a/Release/src/json/json.cpp b/Release/src/json/json.cpp index 04ddbe0fe4..e5a6089aea 100644 --- a/Release/src/json/json.cpp +++ b/Release/src/json/json.cpp @@ -13,9 +13,6 @@ #include "stdafx.h" -#undef min -#undef max - using namespace web; bool json::details::g_keep_json_object_unsorted = false; @@ -519,4 +516,3 @@ const web::json::details::json_error_category_impl& web::json::details::json_err #endif return instance; } - diff --git a/Release/src/pch/stdafx.h b/Release/src/pch/stdafx.h index 1532db2f54..93ad966509 100644 --- a/Release/src/pch/stdafx.h +++ b/Release/src/pch/stdafx.h @@ -20,6 +20,7 @@ #endif #ifdef _WIN32 +#define NOMINMAX #ifdef CPPREST_TARGET_XP #include #ifndef _WIN32_WINNT @@ -137,4 +138,4 @@ #if defined(__clang__) #pragma clang diagnostic pop -#endif +#endif diff --git a/Release/src/websockets/client/ws_client_wspp.cpp b/Release/src/websockets/client/ws_client_wspp.cpp index d2ea9e8dd6..a465fe081b 100644 --- a/Release/src/websockets/client/ws_client_wspp.cpp +++ b/Release/src/websockets/client/ws_client_wspp.cpp @@ -20,10 +20,6 @@ #include "ws_client_impl.h" -// These must be undef'ed before including websocketpp because it is not Windows.h safe. -#undef min -#undef max - // Force websocketpp to use C++ std::error_code instead of Boost. #define _WEBSOCKETPP_CPP11_SYSTEM_ERROR_ #if defined(_MSC_VER) From 6a55deb62c2cbcfff7c8e653546350bc7465ee95 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 30 Jul 2018 13:38:30 -0700 Subject: [PATCH 10/14] Bunch of inheritance hygiene. --- Release/src/http/client/http_client_asio.cpp | 10 +++++----- Release/src/http/client/http_client_winhttp.cpp | 8 ++++---- Release/src/http/client/http_client_winrt.cpp | 14 +++++++------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index ecb749d63e..3a7061bc85 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -297,7 +297,7 @@ class asio_connection /// } /// /// -class asio_connection_pool : public std::enable_shared_from_this +class asio_connection_pool final : public std::enable_shared_from_this { public: asio_connection_pool() : m_pool_epoch_timer(crossplat::threadpool::shared_instance().service()) @@ -397,9 +397,9 @@ class asio_client final : public _http_client_communicator , m_start_with_ssl(base_uri().scheme() == U("https") && !this->client_config().proxy().is_specified()) {} - void send_request(const std::shared_ptr &request_ctx) override; + virtual void send_request(const std::shared_ptr &request_ctx) override; - unsigned long open() override { return 0; } + virtual unsigned long open() override { return 0; } void release_connection(std::shared_ptr& conn) { @@ -429,7 +429,7 @@ class asio_client final : public _http_client_communicator const bool m_start_with_ssl; }; -class asio_context : public request_context, public std::enable_shared_from_this +class asio_context final : public request_context, public std::enable_shared_from_this { friend class asio_client; public: @@ -462,7 +462,7 @@ class asio_context : public request_context, public std::enable_shared_from_this return ctx; } - class ssl_proxy_tunnel : public std::enable_shared_from_this + class ssl_proxy_tunnel final : public std::enable_shared_from_this { public: ssl_proxy_tunnel(std::shared_ptr context, std::function)> ssl_tunnel_established) diff --git a/Release/src/http/client/http_client_winhttp.cpp b/Release/src/http/client/http_client_winhttp.cpp index e23a40a120..ec9ffe2df0 100644 --- a/Release/src/http/client/http_client_winhttp.cpp +++ b/Release/src/http/client/http_client_winhttp.cpp @@ -180,7 +180,7 @@ enum msg_body_type }; // Additional information necessary to track a WinHTTP request. -class winhttp_request_context : public request_context +class winhttp_request_context final : public request_context { public: @@ -244,7 +244,7 @@ class winhttp_request_context : public request_context memory_holder m_body_data; std::unique_ptr decompressor; - virtual void cleanup() + void cleanup() { if(m_request_handle != nullptr) { @@ -256,7 +256,7 @@ class winhttp_request_context : public request_context protected: - virtual void finish() + virtual void finish() override { request_context::finish(); assert(m_self_reference != nullptr); @@ -342,7 +342,7 @@ struct ie_proxy_config : WINHTTP_CURRENT_USER_IE_PROXY_CONFIG }; // WinHTTP client. -class winhttp_client : public _http_client_communicator +class winhttp_client final : public _http_client_communicator { public: winhttp_client(http::uri address, http_client_config client_config) diff --git a/Release/src/http/client/http_client_winrt.cpp b/Release/src/http/client/http_client_winrt.cpp index 697f48a337..c2b4e14c37 100644 --- a/Release/src/http/client/http_client_winrt.cpp +++ b/Release/src/http/client/http_client_winrt.cpp @@ -38,7 +38,7 @@ namespace details { // Additional information necessary to track a WinRT request. -class winrt_request_context : public request_context +class winrt_request_context final : public request_context { public: @@ -60,7 +60,7 @@ class winrt_request_context : public request_context }; // Implementation of IXMLHTTPRequest2Callback. -class HttpRequestCallback : +class HttpRequestCallback final : public RuntimeClass, IXMLHTTPRequest2Callback, FtmBase> { public: @@ -187,7 +187,7 @@ class HttpRequestCallback : /// read and write operations. The I/O will be done off the UI thread, so there is no risk /// of causing the UI to become unresponsive. /// -class IRequestStream +class IRequestStream final : public Microsoft::WRL::RuntimeClass, ISequentialStream> { public: @@ -276,7 +276,7 @@ class IRequestStream /// read and write operations. The I/O will be done off the UI thread, so there is no risk /// of causing the UI to become unresponsive. /// -class IResponseStream +class IResponseStream final : public Microsoft::WRL::RuntimeClass, ISequentialStream> { public: @@ -350,7 +350,7 @@ class IResponseStream }; // WinRT client. -class winrt_client : public _http_client_communicator +class winrt_client final : public _http_client_communicator { public: winrt_client(http::uri&& address, http_client_config&& client_config) @@ -376,13 +376,13 @@ class winrt_client : public _http_client_communicator protected: // Method to open client. - unsigned long open() + virtual unsigned long open() override { return 0; } // Start sending request. - void send_request(_In_ const std::shared_ptr &request) + virtual void send_request(_In_ const std::shared_ptr &request) override { http_request &msg = request->m_request; auto winrt_context = std::static_pointer_cast(request); From 0f86d9e9973e16fd285ac47f2300ff7144d65e2c Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 30 Jul 2018 13:52:26 -0700 Subject: [PATCH 11/14] What do you mean you want examples to compile? --- .../broadcast_server/broadcast_server.cpp | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/Release/libs/websocketpp/examples/broadcast_server/broadcast_server.cpp b/Release/libs/websocketpp/examples/broadcast_server/broadcast_server.cpp index 36087d31dd..f95dce0e61 100644 --- a/Release/libs/websocketpp/examples/broadcast_server/broadcast_server.cpp +++ b/Release/libs/websocketpp/examples/broadcast_server/broadcast_server.cpp @@ -20,6 +20,7 @@ using websocketpp::lib::bind; using websocketpp::lib::thread; using websocketpp::lib::mutex; using websocketpp::lib::lock_guard; +using websocketpp::lib::unique_lock; using websocketpp::lib::condition_variable; /* on_open insert connection_hdl into channel @@ -71,33 +72,36 @@ class broadcast_server { } void on_open(connection_hdl hdl) { - lock_guard lock(m_action_lock); - //std::cout << "on_open" << std::endl; - m_actions.push(action(SUBSCRIBE,hdl)); - lock.unlock(); + { + lock_guard lock(m_action_lock); + //std::cout << "on_open" << std::endl; + m_actions.push(action(SUBSCRIBE,hdl)); + } // unlock m_action_cond.notify_one(); } void on_close(connection_hdl hdl) { - lock_guard lock(m_action_lock); - //std::cout << "on_close" << std::endl; - m_actions.push(action(UNSUBSCRIBE,hdl)); - lock.unlock(); + { + lock_guard lock(m_action_lock); + //std::cout << "on_close" << std::endl; + m_actions.push(action(UNSUBSCRIBE,hdl)); + } // unlock m_action_cond.notify_one(); } void on_message(connection_hdl hdl, server::message_ptr msg) { // queue message up for sending by processing thread - lock_guard lock(m_action_lock); - //std::cout << "on_message" << std::endl; - m_actions.push(action(MESSAGE,hdl,msg)); - lock.unlock(); + { + lock_guard lock(m_action_lock); + //std::cout << "on_message" << std::endl; + m_actions.push(action(MESSAGE,hdl,msg)); + } // unlock m_action_cond.notify_one(); } void process_messages() { while(1) { - lock_guard lock(m_action_lock); + unique_lock lock(m_action_lock); while(m_actions.empty()) { m_action_cond.wait(lock); From 3ab08a53e73a482dbfd3e138f9762988c563d6e1 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 30 Jul 2018 15:09:47 -0700 Subject: [PATCH 12/14] More CR comments from Robert. --- Release/include/cpprest/details/basic_types.h | 6 ++-- .../broadcast_server/broadcast_server.cpp | 34 ++++++++----------- .../src/http/client/http_client_winhttp.cpp | 2 +- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/Release/include/cpprest/details/basic_types.h b/Release/include/cpprest/details/basic_types.h index 20555b45b7..a3f7181a0c 100644 --- a/Release/include/cpprest/details/basic_types.h +++ b/Release/include/cpprest/details/basic_types.h @@ -50,7 +50,8 @@ typedef uint32_t HRESULT; // Needed for PPLX typedef wchar_t char_t ; typedef std::wstring string_t; #define _XPLATSTR(x) L ## x -#define _XPLATTOSTR(x) std::to_wstring(x) +template +std::wstring to_tstring(const T& x) { return std::to_wstring(x); } typedef std::wostringstream ostringstream_t; typedef std::wofstream ofstream_t; typedef std::wostream ostream_t; @@ -68,7 +69,8 @@ typedef std::wstringstream stringstream_t; typedef char char_t; typedef std::string string_t; #define _XPLATSTR(x) x -#define _XPLATTOSTR(x) std::to_string(x) +template +std::string to_tstring(const T& x) { return std::to_string(x); } typedef std::ostringstream ostringstream_t; typedef std::ofstream ofstream_t; typedef std::ostream ostream_t; diff --git a/Release/libs/websocketpp/examples/broadcast_server/broadcast_server.cpp b/Release/libs/websocketpp/examples/broadcast_server/broadcast_server.cpp index f95dce0e61..e87e879eee 100644 --- a/Release/libs/websocketpp/examples/broadcast_server/broadcast_server.cpp +++ b/Release/libs/websocketpp/examples/broadcast_server/broadcast_server.cpp @@ -19,7 +19,6 @@ using websocketpp::lib::bind; using websocketpp::lib::thread; using websocketpp::lib::mutex; -using websocketpp::lib::lock_guard; using websocketpp::lib::unique_lock; using websocketpp::lib::condition_variable; @@ -72,30 +71,27 @@ class broadcast_server { } void on_open(connection_hdl hdl) { - { - lock_guard lock(m_action_lock); - //std::cout << "on_open" << std::endl; - m_actions.push(action(SUBSCRIBE,hdl)); - } // unlock + unique_lock lock(m_action_lock); + //std::cout << "on_open" << std::endl; + m_actions.push(action(SUBSCRIBE,hdl)); + lock.unlock(); m_action_cond.notify_one(); } void on_close(connection_hdl hdl) { - { - lock_guard lock(m_action_lock); - //std::cout << "on_close" << std::endl; - m_actions.push(action(UNSUBSCRIBE,hdl)); - } // unlock + unique_lock lock(m_action_lock); + //std::cout << "on_close" << std::endl; + m_actions.push(action(UNSUBSCRIBE,hdl)); + lock.unlock(); m_action_cond.notify_one(); } void on_message(connection_hdl hdl, server::message_ptr msg) { // queue message up for sending by processing thread - { - lock_guard lock(m_action_lock); - //std::cout << "on_message" << std::endl; - m_actions.push(action(MESSAGE,hdl,msg)); - } // unlock + unique_lock lock(m_action_lock); + //std::cout << "on_message" << std::endl; + m_actions.push(action(MESSAGE,hdl,msg)); + lock.unlock(); m_action_cond.notify_one(); } @@ -113,13 +109,13 @@ class broadcast_server { lock.unlock(); if (a.type == SUBSCRIBE) { - lock_guard con_lock(m_connection_lock); + unique_lock con_lock(m_connection_lock); m_connections.insert(a.hdl); } else if (a.type == UNSUBSCRIBE) { - lock_guard con_lock(m_connection_lock); + unique_lock con_lock(m_connection_lock); m_connections.erase(a.hdl); } else if (a.type == MESSAGE) { - lock_guard con_lock(m_connection_lock); + unique_lock con_lock(m_connection_lock); con_list::iterator it; for (it = m_connections.begin(); it != m_connections.end(); ++it) { diff --git a/Release/src/http/client/http_client_winhttp.cpp b/Release/src/http/client/http_client_winhttp.cpp index ec9ffe2df0..1b86e5ff39 100644 --- a/Release/src/http/client/http_client_winhttp.cpp +++ b/Release/src/http/client/http_client_winhttp.cpp @@ -490,7 +490,7 @@ class winhttp_client final : public _http_client_communicator if (uri.port() > 0) { proxy_str.push_back(_XPLATSTR(':')); - proxy_str.append(_XPLATTOSTR(uri.port())); + proxy_str.append(utility::to_tstring(uri.port())); } proxy_name = proxy_str.c_str(); From beb660c9ceec3836add087f71d323bb960fcebbe Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 30 Jul 2018 16:00:37 -0700 Subject: [PATCH 13/14] Add static. --- Release/src/http/client/http_client_asio.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp index 3a7061bc85..56def29e39 100644 --- a/Release/src/http/client/http_client_asio.cpp +++ b/Release/src/http/client/http_client_asio.cpp @@ -79,7 +79,7 @@ using utility::conversions::details::to_string; using std::to_string; #endif -const std::string CRLF("\r\n"); +static const std::string CRLF("\r\n"); namespace web { namespace http { From 6788781efb0a880a1e4fb21f01dfe8d96494b299 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 30 Jul 2018 17:23:59 -0700 Subject: [PATCH 14/14] Use existing to_string_t. --- Release/include/cpprest/asyncrt_utils.h | 8 ++++---- Release/include/cpprest/details/basic_types.h | 4 ---- Release/src/http/client/http_client_winhttp.cpp | 2 +- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/Release/include/cpprest/asyncrt_utils.h b/Release/include/cpprest/asyncrt_utils.h index 68aa738be0..0eee8f2a72 100644 --- a/Release/include/cpprest/asyncrt_utils.h +++ b/Release/include/cpprest/asyncrt_utils.h @@ -228,7 +228,7 @@ namespace conversions #if defined(__ANDROID__) template - inline std::string to_string(const T& t) + inline std::string to_string(const T t) { std::ostringstream os; os.imbue(std::locale::classic()); @@ -238,16 +238,16 @@ namespace conversions #endif template - inline utility::string_t to_string_t(T&& t) + inline utility::string_t to_string_t(const T t) { #ifdef _UTF16_STRINGS using std::to_wstring; - return to_wstring(std::forward(t)); + return to_wstring(t); #else #if !defined(__ANDROID__) using std::to_string; #endif - return to_string(std::forward(t)); + return to_string(t); #endif } diff --git a/Release/include/cpprest/details/basic_types.h b/Release/include/cpprest/details/basic_types.h index a3f7181a0c..eadba01684 100644 --- a/Release/include/cpprest/details/basic_types.h +++ b/Release/include/cpprest/details/basic_types.h @@ -50,8 +50,6 @@ typedef uint32_t HRESULT; // Needed for PPLX typedef wchar_t char_t ; typedef std::wstring string_t; #define _XPLATSTR(x) L ## x -template -std::wstring to_tstring(const T& x) { return std::to_wstring(x); } typedef std::wostringstream ostringstream_t; typedef std::wofstream ofstream_t; typedef std::wostream ostream_t; @@ -69,8 +67,6 @@ typedef std::wstringstream stringstream_t; typedef char char_t; typedef std::string string_t; #define _XPLATSTR(x) x -template -std::string to_tstring(const T& x) { return std::to_string(x); } typedef std::ostringstream ostringstream_t; typedef std::ofstream ofstream_t; typedef std::ostream ostream_t; diff --git a/Release/src/http/client/http_client_winhttp.cpp b/Release/src/http/client/http_client_winhttp.cpp index 1b86e5ff39..8fa007a5a1 100644 --- a/Release/src/http/client/http_client_winhttp.cpp +++ b/Release/src/http/client/http_client_winhttp.cpp @@ -490,7 +490,7 @@ class winhttp_client final : public _http_client_communicator if (uri.port() > 0) { proxy_str.push_back(_XPLATSTR(':')); - proxy_str.append(utility::to_tstring(uri.port())); + proxy_str.append(::utility::conversions::details::to_string_t(uri.port())); } proxy_name = proxy_str.c_str();