Finish blocking run and adding documentation.

1 files changed, 273 insertions, 14 deletions

diff --git a/indra/llcorehttp/llwebsocketmgr.cpp b/indra/llcorehttp/llwebsocketmgr.cpp
index 0c61e0f68d..9fc9027be3 100644
--- a/indra/llcorehttp/llwebsocketmgr.cpp
+++ b/indra/llcorehttp/llwebsocketmgr.cpp
@@ -36,6 +36,10 @@
 #include <websocketpp/client.hpp>
 #include <websocketpp/server.hpp>
 
+#include <thread>
+#include <atomic>
+#include <chrono>
+
 //------------------------------------------------------------------------
 namespace
 {
@@ -149,10 +153,39 @@ void LLWebsocketMgr::stopAllServers()
     mServers.clear();
 }
 
-
 //------------------------------------------------------------------------
+/**
+ * @struct Server_impl
+ * @brief Internal implementation wrapper for websocketpp server functionality
+ *
+ * This structure serves as a PIMPL (Pointer to Implementation) wrapper around
+ * the websocketpp::server template, providing a clean interface between the
+ * high-level WSServer class and the low-level websocketpp library. It handles
+ * all direct websocketpp interactions including server lifecycle management,
+ * event handling, and connection management.
+ *
+ * The Server_impl follows Linden Lab conventions while maintaining compatibility
+ * with the websocketpp library. It provides thread-safe operations where possible
+ * and integrates with the existing logging infrastructure.
+ *
+ * @note This class uses the websocketpp::config::asio configuration which provides
+ * ASIO-based networking without TLS/SSL support.
+ */
 struct Server_impl
 {
+    /**
+     * @brief Constructor - Initializes the websocketpp server with configuration
+     * @param owner Pointer to the owning WSServer instance (must not be null)
+     * @param port The port number to bind the server to (1-65535)
+     * @param local_only If true, binds only to localhost; if false, binds to all interfaces
+     *
+     * Sets up the websocketpp server instance and registers lambda-based event handlers
+     * for connection open, close, and message events. The handlers delegate back to the
+     * owning WSServer instance for processing, maintaining the abstraction layer.
+     *
+     * @warning The owner pointer must remain valid for the lifetime of this object
+     * @pre port must be a valid port number (typically > 1024 for non-privileged access)
+     */
     Server_impl(LLWebsocketMgr::WSServer *owner, U16 port, bool local_only) :
         mOwner(owner),
         mPort(port),
@@ -165,6 +198,19 @@ struct Server_impl
 
     ~Server_impl() = default;
 
+    /**
+     * @brief Initialize the websocketpp server and configure listening
+     *
+     * Performs the initial setup of the websocketpp server by calling init_asio()
+     * to initialize the ASIO networking layer, then configures the server to listen
+     * on the specified port. The binding behavior depends on the mLocalOnly flag:
+     * - If mLocalOnly is true: binds to "127.0.0.1" (localhost only)
+     * - If mLocalOnly is false: binds to all available network interfaces
+     *
+     * @note This method must be called before attempting to start the server
+     * @pre The server must not already be initialized
+     * @post The server is ready to accept connections when start() is called
+     */
     void init()
     {
         mServer.init_asio();
@@ -180,6 +226,20 @@ struct Server_impl
         }
     }
 
+    /**
+     * @brief Start the websocket server and begin accepting connections
+     * @return true if server started successfully, false on error
+     *
+     * Runs a controlled event loop that periodically checks the stop flag for clean shutdown.
+     * Instead of calling run() once and blocking indefinitely, this implementation uses
+     * run_for() with a timeout to process events in chunks, checking mOwner->mShouldStop between
+     * iterations to allow for responsive termination.
+     *
+     * @note This method blocks the calling thread until the server stops
+     * @pre init() must have been called successfully
+     * @post On success, the server is actively accepting connections
+     * @warning Any exceptions during startup are caught and logged as warnings
+     */
     bool start()
     {
         if (!mServer.stopped())
@@ -187,19 +247,61 @@ struct Server_impl
             LL_WARNS("WebSocket") << "WebSocket server is already running" << LL_ENDL;
             return false;
         }
+
         try
         {
             mServer.start_accept();
-            mServer.run();
+
+            // Run controlled event loop with periodic stop flag checking
+            while (!mOwner->mShouldStop && !mServer.stopped())
+            {
+                // Process events for up to 100ms, then check the stop flag
+                std::chrono::milliseconds timeout(100);
+                std::size_t handlers_run = mServer.get_io_service().run_for(timeout);
+
+                // If no handlers were run and the server isn't stopped,
+                // reset the io_service for the next iteration
+                if (handlers_run == 0 && !mServer.stopped() && !mOwner->mShouldStop)
+                {
+                    mServer.get_io_service().restart();
+                }
+            }
+
+            LL_INFOS("WebSocket") << "WebSocket server event loop exited cleanly" << LL_ENDL;
+            return true;
+        }
+        catch (const websocketpp::exception& e)
+        {
+            LL_WARNS("WebSocket") << "WebSocket server exception: " << e.what() << LL_ENDL;
+            return false;
         }
         catch (const std::exception& e)
         {
-            LL_WARNS("WebSocket") << "WebSocket server encountered an error: " << e.what() << LL_ENDL;
+            LL_WARNS("WebSocket") << "WebSocket server std::exception: " << e.what() << LL_ENDL;
+            return false;
+        }
+        catch (...)
+        {
+            LL_WARNS("WebSocket") << "WebSocket server unknown exception" << LL_ENDL;
             return false;
         }
-        return true;
     }
 
+    /**
+     * @brief Stop the websocket server and cease accepting new connections
+     *
+     * Gracefully shuts down the server by first stopping the listener to prevent
+     * new connections, then stopping the ASIO event loop. Existing connections
+     * may remain active briefly during the shutdown process.
+     *
+     * The method performs a safe shutdown by checking if the server is already
+     * stopped before attempting shutdown operations. Any exceptions during shutdown
+     * are caught and logged but do not propagate.
+     *
+     * @note This method is non-blocking and safe to call multiple times
+     * @post The server will no longer accept new connections
+     * @post Existing connections will be cleanly terminated
+     */
     void stop()
     {
         if (mServer.stopped())
@@ -217,22 +319,65 @@ struct Server_impl
         }
     }
 
+    /**
+     * @brief Handle new connection establishment event
+     * @param hdl WebSocket connection handle from websocketpp
+     *
+     * Called automatically by the websocketpp library when a new client connection
+     * is successfully established. This method serves as a bridge between the
+     * low-level websocketpp callback and the high-level WSServer interface.
+     *
+     * @note This is an internal callback method called by websocketpp
+     * @pre mOwner must be valid (assertion will fail if null)
+     * @post The owning WSServer will be notified of the new connection
+     */
     void onOpen(websocketpp::connection_hdl hdl) const
     {
-        assert(mOwner); // mOwner should never be null. If it is, something is very wrong!
+        LL_ERRS_IF(!mOwner, "WebSocket") << "mOwner should never be null. If it is, something is very wrong!" << LL_ENDL;
 
         mOwner->handleOpenConnection(hdl);
     }
 
+    /**
+     * @brief Handle connection closure event
+     * @param hdl WebSocket connection handle from websocketpp
+     *
+     * Called automatically by the websocketpp library when a client connection
+     * is closed, either by the client, server, or due to a network error.
+     * This method delegates the event to the owning WSServer for processing.
+     *
+     * @note This is an internal callback method called by websocketpp
+     * @pre mOwner must be valid (assertion will fail if null)
+     * @post The owning WSServer will be notified of the connection closure
+     */
     void onClose(websocketpp::connection_hdl hdl) const
     {
-        assert(mOwner); // mOwner should never be null
+        LL_ERRS_IF(!mOwner, "WebSocket") << "mOwner should never be null" << LL_ENDL;
         mOwner->handleCloseConnection(hdl);
     }
 
+    /**
+     * @brief Handle incoming message from client
+     * @param hdl WebSocket connection handle identifying the sender
+     * @param msg Shared pointer to the message object containing payload and metadata
+     *
+     * Called automatically by the websocketpp library when a complete message is
+     * received from a client. This method validates the connection exists, extracts
+     * the message payload, and forwards it to the appropriate connection handler.
+     *
+     * Currently handles text messages only. Binary message support and message
+     * fragmentation handling are noted as TODO items for future implementation.
+     *
+     * @note This is an internal callback method called by websocketpp
+     * @pre mOwner must be valid (assertion will fail if null)
+     * @post If connection exists, the message is forwarded for processing
+     * @todo Add support for binary messages
+     * @todo Implement fragmented message handling
+     * @todo Process connection close codes for graceful closure
+     */
     void onMessage(websocketpp::connection_hdl hdl, Server_t::message_ptr msg) const
     {
-        assert(mOwner); // mOwner should never be null
+        LL_ERRS_IF(!mOwner, "WebSocket") << "mOwner should never be null" << LL_ENDL;
         LLWebsocketMgr::WSConnection::ptr_t connection = mOwner->getConnection(hdl);
         if (!connection)
         {
@@ -247,10 +392,10 @@ struct Server_impl
     }
 
     //-------------------------------------------
-    Server_t                    mServer;
-    LLWebsocketMgr::WSServer*   mOwner{ nullptr }; // Back-reference to the owning server, note that this pointer can never be null
-    U16                         mPort{ 0 };
-    bool                        mLocalOnly{ true };
+    Server_t                    mServer;       ///< The underlying websocketpp server instance
+    LLWebsocketMgr::WSServer*   mOwner{ nullptr }; ///< Back-reference to the owning WSServer instance (guaranteed non-null)
+    U16                         mPort{ 0 };    ///< TCP port number the server listens on
+    bool                        mLocalOnly{ true }; ///< Whether to bind to localhost only (true) or all interfaces (false)
 };
 
 //------------------------------------------------------------------------
@@ -266,6 +411,12 @@ LLWebsocketMgr::WSServer::WSServer(std::string_view name, U16 port, bool local_o
         " on port " << mImpl->mPort << LL_ENDL;
 }
 
+LLWebsocketMgr::WSServer::~WSServer()
+{
+    // Ensure the server is stopped before destruction
+    stop();
+}
+
 LLWebsocketMgr::WSConnection::ptr_t LLWebsocketMgr::WSServer::connectionFactory(LLWebsocketMgr::WSServer::ptr_t server,
     LLWebsocketMgr::connection_h handle)
 {
@@ -275,19 +426,75 @@ LLWebsocketMgr::WSConnection::ptr_t LLWebsocketMgr::WSServer::connectionFactory(
 bool LLWebsocketMgr::WSServer::start()
 {
     LL_ERRS_IF(!mImpl, "WebSocket") << "WebSocket server " << mServerName << " implementation is null !" << LL_ENDL;
-    return mImpl->start();
+
+    LLMutexLock lock(&mThreadMutex);
+
+    // Check if already running
+    if (isRunning())
+    {
+        LL_WARNS("WebSocket") << "Server " << mServerName << " is already running" << LL_ENDL;
+        return false;
+    }
+
+    // Reset the stop flag
+    mShouldStop = false;
+
+    // Start the server thread
+    mServerThread = std::thread([this]() {
+        LL_INFOS("WebSocket") << "WebSocket server thread starting for: " << mServerName << LL_ENDL;
+
+        // Run the controlled server loop that checks the stop flag
+        // Server_impl accesses mShouldStop through the mOwner pointer
+        bool success = mImpl->start();
+
+        if (!success)
+        {
+            LL_WARNS("WebSocket") << "WebSocket server thread failed to start for: " << mServerName << LL_ENDL;
+        }
+
+        LL_INFOS("WebSocket") << "WebSocket server thread exiting for: " << mServerName << LL_ENDL;
+    });
+
+    LL_INFOS("WebSocket") << "Started WebSocket server thread: " << mServerName << LL_ENDL;
+    return true;
 }
 
 void LLWebsocketMgr::WSServer::stop()
 {
     LL_ERRS_IF(!mImpl, "WebSocket") << "WebSocket server " << mServerName << " implementation is null !" << LL_ENDL;
-    mImpl->stop();
+
+    {
+        LLMutexLock lock(&mThreadMutex);
+
+        // Check if already stopped
+        if (!isRunning())
+        {
+            return;
+        }
+
+        LL_INFOS("WebSocket") << "Stopping WebSocket server: " << mServerName << LL_ENDL;
+
+        // Signal the thread to stop
+        mShouldStop = true;
+
+        // Stop the websocket server (this will cause the controlled run loop to exit)
+        mImpl->stop();
+    } // Release the lock here
+
+    // Wait for the thread to finish (outside the lock to avoid deadlock)
+    if (mServerThread.joinable())
+    {
+        mServerThread.join();
+        LL_INFOS("WebSocket") << "WebSocket server thread joined for: " << mServerName << LL_ENDL;
+    }
 }
 
 bool LLWebsocketMgr::WSServer::isRunning() const
 {
     LL_ERRS_IF(!mImpl, "WebSocket") << "WebSocket server " << mServerName << " implementation is null !" << LL_ENDL;
-    return !mImpl->mServer.stopped();
+
+    // Check both the thread state, websocket server state, and the stop flag
+    return mServerThread.joinable() && !mImpl->mServer.stopped() && !mShouldStop;
 }
 
 void LLWebsocketMgr::WSServer::broadcastMessage(const std::string& message)
@@ -313,6 +520,41 @@ bool LLWebsocketMgr::WSServer::sendMessageTo(const connection_h& handle, const s
     return true;
 }
 
+bool LLWebsocketMgr::WSServer::closeConnection(const connection_h& handle, U16 code, const std::string& reason)
+{
+    LL_ERRS_IF(!mImpl, "WebSocket") << "WebSocket server " << mServerName << " implementation is null !" << LL_ENDL;
+
+    try
+    {
+        websocketpp::lib::error_code ec;
+        mImpl->mServer.close(handle, code, reason, ec);
+        if (ec)
+        {
+            LL_WARNS("WebSocket") << mServerName << " failed to close connection: " << ec.message() << LL_ENDL;
+            return false;
+        }
+
+        LL_INFOS("WebSocket") << mServerName << " initiated close for connection with code "
+                              << code << " and reason: " << reason << LL_ENDL;
+        return true;
+    }
+    catch (const websocketpp::exception& e)
+    {
+        LL_WARNS("WebSocket") << mServerName << " exception closing connection: " << e.what() << LL_ENDL;
+        return false;
+    }
+    catch (const std::exception& e)
+    {
+        LL_WARNS("WebSocket") << mServerName << " std::exception closing connection: " << e.what() << LL_ENDL;
+        return false;
+    }
+    catch (...)
+    {
+        LL_WARNS("WebSocket") << mServerName << " unknown exception closing connection" << LL_ENDL;
+        return false;
+    }
+}
+
 LLWebsocketMgr::WSConnection::ptr_t LLWebsocketMgr::WSServer::getConnection(const connection_h& handle)
 {
     LLMutexLock lock(&mConnectionMutex);
@@ -409,3 +651,20 @@ bool LLWebsocketMgr::WSConnection::sendMessage(const std::string& message)
     }
     return mServer->sendMessageTo(mConnectionHandle, message);
 }
+
+void LLWebsocketMgr::WSConnection::closeConnection(U16 code, const std::string& reason)
+{
+    if (!mServer)
+    {
+        LL_WARNS("WebSocket") << "Attempted to close connection with null server reference" << LL_ENDL;
+        return;
+    }
+
+    LL_INFOS("WebSocket") << "WSConnection closing connection with code " << code
+                          << " and reason: " << (reason.empty() ? "(no reason)" : reason) << LL_ENDL;
+
+    if (!mServer->closeConnection(mConnectionHandle, code, reason))
+    {
+        LL_WARNS("WebSocket") << "Failed to close connection through server" << LL_ENDL;
+    }
+}