From 9d8eced9a15a01c44eb7a229b0aabf2e75b7cb59 Mon Sep 17 00:00:00 2001 From: Peter Thorson Date: Wed, 5 Mar 2014 08:06:14 -0600 Subject: [PATCH] Refactored server_endpoint to allow end users to access start_accept error states references #335 also more documentation --- changelog.md | 5 +- websocketpp/roles/server_endpoint.hpp | 94 +++++++++++++++++---------- 2 files changed, 62 insertions(+), 37 deletions(-) diff --git a/changelog.md b/changelog.md index 6eea8681f6..01b142554f 100644 --- a/changelog.md +++ b/changelog.md @@ -30,6 +30,8 @@ HEAD short reads and quasi-expected socket shutdown related errors will no longer be reported as unclean WebSocket shutdowns to the application. Information about them will remain in the info error channel for debugging purposes. +- Improvement: `start_accept` errors are now reported to the caller either via + an exception or an ec parameter. - Bug: Fix some cases of calls to empty lib::function objects. - Bug: Fix memory leak of connection objects due to cached handlers holding on to reference counted pointers. #310 Thank you otaras for reporting. @@ -48,7 +50,8 @@ HEAD code. Thank you Robin Rowe for reporting. - Bug: Fix an issue where custom timeout values weren't being propagated from endpoints to new connections. -- Bug: Fix a memory leak when a connection fails. #323 Thank you droppy for +- Bug: Fix a number of memory leaks related to server connection failures. #323 + #333 #334 #335 Thank you droppy and aydany for reporting and patches. reporting. - Compatibility: Fix compile time conflict with Visual Studio's MIN/MAX macros. Thank you Robin Rowe for reporting. diff --git a/websocketpp/roles/server_endpoint.hpp b/websocketpp/roles/server_endpoint.hpp index a4d4a971f5..88ceae7710 100644 --- a/websocketpp/roles/server_endpoint.hpp +++ b/websocketpp/roles/server_endpoint.hpp @@ -64,60 +64,76 @@ public: /// Type of the endpoint component of this server typedef endpoint endpoint_type; - - // TODO: clean up these types - explicit server() : endpoint_type(true) { endpoint_type::m_alog.write(log::alevel::devel, "server constructor"); } - // return an initialized connection_ptr. Call start() on this object to - // begin the processing loop. + /// Create and initialize a new connection + /** + * The connection will be initialized and ready to begin. Call its start() + * method to begin the processing loop. + * + * Note: The connection must either be started or terminated using + * connection::terminate in order to avoid memory leaks. + * + * @return A pointer to the new connection. + */ connection_ptr get_connection() { - connection_ptr con = endpoint_type::create_connection(); - - return con; + return endpoint_type::create_connection(); } - // Starts the server's async connection acceptance loop. - void start_accept() { + /// Starts the server's async connection acceptance loop (exception free) + /** + * Initiates the server connection acceptance loop. Must be called after + * listen. This method will have no effect until the underlying io_service + * starts running. It may be called after the io_service is already running. + * + * Refer to documentation for the transport policy you are using for + * instructions on how to stop this acceptance loop. + * + * @param [out] ec A status code indicating an error, if any. + */ + void start_accept(lib::error_code & ec) { if (!transport_type::is_listening()) { - endpoint_type::m_elog.write(log::elevel::info, - "Stopping acceptance of new connections because the underlying transport is no longer listening."); + ec = error::make_error_code(error::async_accept_not_listening); return; } + ec = lib::error_code(); connection_ptr con = get_connection(); - lib::error_code ec; - transport_type::async_accept( lib::static_pointer_cast(con), - lib::bind( - &type::handle_accept, - this, - con, - lib::placeholders::_1 - ), + lib::bind(&type::handle_accept,this,con,lib::placeholders::_1), ec ); - if (ec == error::async_accept_not_listening) { - endpoint_type::m_elog.write(log::elevel::info, - "Stopping acceptance of new connections because the underlying transport is no longer listening."); - } else if (ec) { - endpoint_type::m_elog.write(log::elevel::rerror, - "start_accept error: "+ec.message()); + if (ec && con) { + // If the connection was constructed but the accept failed, + // terminate the connection to prevent memory leaks + con->terminate(lib::error_code()); } - - if (ec && con) { - // Terminate the connection to prevent memory leaks. - lib::error_code con_ec; - con->terminate(con_ec); - } - } + } + /// Starts the server's async connection acceptance loop + /** + * Initiates the server connection acceptance loop. Must be called after + * listen. This method will have no effect until the underlying io_service + * starts running. It may be called after the io_service is already running. + * + * Refer to documentation for the transport policy you are using for + * instructions on how to stop this acceptance loop. + */ + void start_accept() { + lib::error_code ec; + start_accept(ec); + if (ec) { + throw ec; + } + } + + /// Handler callback for start_accept void handle_accept(connection_ptr con, lib::error_code const & ec) { if (ec) { con->terminate(ec); @@ -133,10 +149,16 @@ public: con->start(); } - - start_accept(); + lib::error_code start_ec; + start_accept(start_ec); + if (start_ec == error::async_accept_not_listening) { + endpoint_type::m_elog.write(log::elevel::info, + "Stopping acceptance of new connections because the underlying transport is no longer listening."); + } else if (start_ec) { + endpoint_type::m_elog.write(log::elevel::rerror, + "Restarting async_accept loop failed: "+ec.message()); + } } -private: }; } // namespace websocketpp