Refactored server_endpoint to allow end users to access start_accept error states references XRPLF#335

also more documentation

Author: zaphoyd

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.

websocketpp/roles/server_endpoint.hpp

@@ -64,60 +64,76 @@ class server : public endpoint<connection<config>,config> {
     /// Type of the endpoint component of this server
     typedef endpoint<connection_type,config> 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<transport_con_type>(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 @@ class server : public endpoint<connection<config>,config> {
             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