Issue #6367: The viewer would hang on exit if still connected to a plugin. Updates how the viewer dispatches coroutines to the main thread and removes an extraneous wait, that was incorrectly serializing asynchronous messages.

8 files changed, 131 insertions, 41 deletions

diff --git a/indra/llcommon/workqueue.cpp b/indra/llcommon/workqueue.cpp
index fbd92d089e..4f19ab2cc2 100644
--- a/indra/llcommon/workqueue.cpp
+++ b/indra/llcommon/workqueue.cpp
@@ -319,6 +319,12 @@ bool LL::WorkQueue::tryPost(const Work& callable)
     }
 }
 
+bool LL::WorkQueue::postToMainLoop(const Work& work)
+{
+    auto queue = getInstance("mainloop");
+    return queue && queue->tryPost(work);
+}
+
 LL::WorkQueue::Work LL::WorkQueue::pop_()
 {
     return mQueue.pop();
diff --git a/indra/llcommon/workqueue.h b/indra/llcommon/workqueue.h
index 69f3286c1b..322baa36e6 100644
--- a/indra/llcommon/workqueue.h
+++ b/indra/llcommon/workqueue.h
@@ -265,6 +265,16 @@ namespace LL
          */
         bool tryPost(const Work&) override;
 
+        /**
+         * @brief Post work to the well-known main-loop queue without blocking
+         * @return true if posted, false if the main loop is gone or its
+         *         queue is closed/full
+         *
+         * For callers on a secondary thread that must not block waiting for
+         * the main thread (e.g. a websocket I/O thread).
+         */
+        static bool postToMainLoop(const Work& work);
+
     private:
         using Queue = LLThreadSafeQueue<Work>;
         Queue mQueue;
diff --git a/indra/llcorehttp/lljsonrpcws.cpp b/indra/llcorehttp/lljsonrpcws.cpp
index 5b704e6322..4745be64b3 100644
--- a/indra/llcorehttp/lljsonrpcws.cpp
+++ b/indra/llcorehttp/lljsonrpcws.cpp
@@ -31,9 +31,8 @@
 #include "llsdjson.h"
 #include "lldate.h"
 #include "llcoros.h"
-#include "llmainthreadtask.h"
-#include "lleventtimer.h"
 #include "lltimer.h"
+#include "workqueue.h"
 
 #include <boost/json.hpp>
 
@@ -44,27 +43,10 @@
 void LLJSONRPCConnection::onOpen()
 {
     LL_INFOS("JSONRPC") << "JSON-RPC connection opened" << LL_ENDL;
-
-    // Start the recurring timeout sweep timer on the main thread. The timer
-    // is canceled in onClose() before the connection can be destroyed, so
-    // capturing `this` is safe. Keep a weak_ptr so we can safely test
-    // whether the timer instance still exists at cancellation time.
-    LLEventTimer* timer = LLEventTimer::run_every(TIMEOUT_SWEEP_INTERVAL,
-        [this]() { sweepTimeouts(); });
-    mTimeoutTimer = timer->getWeak();
 }
 
 void LLJSONRPCConnection::onClose()
 {
-    // Cancel the sweep timer if it is still alive. LLEventTimer's instance
-    // tracker keeps a shared_ptr with a no-op deleter, so raw `delete` is
-    // the documented cancellation idiom (see lleventtimer.h).
-    if (auto timer = mTimeoutTimer.lock())
-    {
-        delete timer.get();
-    }
-    mTimeoutTimer.reset();
-
     // Move the pending-request map out under the lock so we can invoke the
     // callbacks without holding it (callbacks may themselves call into this
     // connection).
@@ -219,7 +201,7 @@ void LLJSONRPCConnection::processRequest(const LLSD& request)
             throw InternalError("Connection expired before method " + method
                                 + " could be launched");
         }
-        LLMainThreadTask::dispatch(
+        bool posted = LL::WorkQueue::postToMainLoop(
             [handler, method, id, params, conn]()
             {
                 LLCoros::instance().launch(
@@ -265,6 +247,11 @@ void LLJSONRPCConnection::processRequest(const LLSD& request)
                         }
                     });
             });
+        if (!posted)
+        {
+            LL_WARNS("JSONRPC") << "Main loop unavailable; failing async method " << method << LL_ENDL;
+            throw InternalError("Viewer is shutting down; cannot service " + method);
+        }
         return;
     }
 
@@ -715,6 +702,25 @@ void LLJSONRPCServer::onConnectionClosed(const LLWebsocketMgr::WSConnection::ptr
                         << getConnectionCount() << LL_ENDL;
 }
 
+bool LLJSONRPCServer::update()
+{
+    // Main thread, once per frame; throttled to TIMEOUT_SWEEP_INTERVAL.
+    if (mSweepTimer.getElapsedTimeF32() < LLJSONRPCConnection::TIMEOUT_SWEEP_INTERVAL)
+    {
+        return true;
+    }
+    mSweepTimer.reset();
+
+    for (const auto& conn : getConnections())
+    {
+        if (auto rpc = std::dynamic_pointer_cast<LLJSONRPCConnection>(conn))
+        {
+            rpc->sweepTimeouts();
+        }
+    }
+    return true;
+}
+
 void LLJSONRPCServer::setupConnectionMethods(LLJSONRPCConnection::ptr_t connection)
 {
     LLMutexLock lock(&mGlobalMethodsMutex);
diff --git a/indra/llcorehttp/lljsonrpcws.h b/indra/llcorehttp/lljsonrpcws.h
index aa4a96be4d..08a6cefd22 100644
--- a/indra/llcorehttp/lljsonrpcws.h
+++ b/indra/llcorehttp/lljsonrpcws.h
@@ -29,14 +29,13 @@
 #include "llwebsocketmgr.h"
 #include "llsd.h"
 #include "lluuid.h"
+#include "lltimer.h"
 
 #include <functional>
 #include <unordered_map>
 #include <memory>
 #include <queue>
 
-class LLEventTimer;
-
 /**
  * @class LLJSONRPCConnection
  * @brief JSON-RPC 2.0 WebSocket connection implementation
@@ -390,7 +389,8 @@ private:
     // Per-request timeout tracking. mPendingDeadlines is a min-heap of
     // (deadline, request_id) ordered by deadline; entries whose request has
     // already been answered become tombstones (skipped when they reach the
-    // top). A single recurring timer per connection sweeps the heap.
+    // top). The owning server calls sweepTimeouts() once per frame (see
+    // LLJSONRPCServer::update()) to fire any expired entries.
     struct PendingDeadline
     {
         F64         mDeadline;   // absolute time in seconds (LLTimer::getTotalSeconds)
@@ -399,16 +399,18 @@ private:
         bool operator<(const PendingDeadline& rhs) const { return mDeadline > rhs.mDeadline; }
     };
     std::priority_queue<PendingDeadline> mPendingDeadlines;
-    std::weak_ptr<LLEventTimer>          mTimeoutTimer;
 
     static constexpr F64 REQUEST_TIMEOUT_SECONDS = 120.0;
-    static constexpr F32 TIMEOUT_SWEEP_INTERVAL  = 1.0f;
 
-    /// Invoked by the sweep timer; fires the timeout callback for any
-    /// request whose deadline has passed. Safe to call from the main thread.
+public:
+    /// Sweep interval used by LLJSONRPCServer::update() to throttle sweepTimeouts().
+    static constexpr F32 TIMEOUT_SWEEP_INTERVAL = 1.0f;
+
+    /// Invoked once per frame by the owning server (see LLJSONRPCServer::update());
+    /// fires the timeout callback for any request whose deadline has passed.
+    /// Main-thread only.
     void sweepTimeouts();
 
-public:
     void testInjectPendingRequest(const std::string& id, F64 deadline, ResponseCallback callback);
     void testSweepTimeouts();
     size_t testPendingRequestCount() const;
@@ -497,12 +499,15 @@ public:
      */
     LLSD getServerStats() const;
 
+    bool update() override;
+
 protected:
     LLWebsocketMgr::WSConnection::ptr_t connectionFactory(LLWebsocketMgr::WSServer::ptr_t server,
                                                          LLWebsocketMgr::connection_h handle) override;
 
     virtual LLSD handlePing(const LLJSONRPCConnection::ptr_t& connection,
                             const LLSD& params) const;
+
     virtual LLSD handleGetVersion(const LLJSONRPCConnection::ptr_t& connection,
                                   const LLSD& params) const = 0;
     virtual LLSD handleStatus(const LLJSONRPCConnection::ptr_t& connection,
@@ -521,4 +526,5 @@ private:
     std::string mServerName;  // Store server name for stats
     std::atomic<U64> mTotalRequestsHandled{0};
     std::atomic<U64> mTotalNotificationsSent{0};
+    LLTimer mSweepTimer;
 };
diff --git a/indra/llcorehttp/llwebsocketmgr.cpp b/indra/llcorehttp/llwebsocketmgr.cpp
index be1ab54efd..3c0e0ed952 100644
--- a/indra/llcorehttp/llwebsocketmgr.cpp
+++ b/indra/llcorehttp/llwebsocketmgr.cpp
@@ -31,6 +31,7 @@
 #include "llsdserialize.h"
 #include "llhost.h"
 #include "llsdjson.h"
+#include "lltimer.h"
 
 #include <websocketpp/config/boost_config.hpp>
 #include <websocketpp/config/asio_no_tls.hpp>
@@ -47,6 +48,10 @@ namespace
     using Server_t      = websocketpp::server<websocketpp::config::asio>;
     using Client_t      = websocketpp::client<websocketpp::config::asio>;
     using Connection_t  = websocketpp::connection<websocketpp::config::asio>;
+
+    // How long stop() waits for the I/O thread to exit before logging a
+    // warning. It still joins after this -- this is diagnostic, not a timeout.
+    constexpr F32 SERVER_STOP_WARN_SECONDS = 5.0f;
 }
 
 //------------------------------------------------------------------------
@@ -407,9 +412,11 @@ bool LLWebsocketMgr::WSServer::start()
 
     // Reset the stop flag
     mShouldStop = false;
+    mRunning = true;
 
     // Start the server thread
     mServerThread = std::thread([this]() {
+        LL_PROFILER_SET_THREAD_NAME(mServerName.c_str());
         LL_INFOS("WebSocket") << "WebSocket server thread starting for: " << mServerName << LL_ENDL;
 
         // Run the controlled server loop that checks the stop flag
@@ -422,6 +429,7 @@ bool LLWebsocketMgr::WSServer::start()
         }
 
         LL_INFOS("WebSocket") << "WebSocket server thread exiting for: " << mServerName << LL_ENDL;
+        mRunning = false;
     });
 
     onStarted();
@@ -455,6 +463,22 @@ void LLWebsocketMgr::WSServer::stop()
 
     if (mServerThread.joinable())
     {
+        // The I/O thread should exit promptly once mImpl->stop() returns. If
+        // it doesn't, something on that thread is blocked -- most likely a
+        // regression reintroducing a cross-thread wait. Warn instead of
+        // hanging silently; still join (detaching would use-after-free mImpl).
+        LLTimer wait_timer;
+        while (mRunning && wait_timer.getElapsedTimeF32() < SERVER_STOP_WARN_SECONDS)
+        {
+            ms_sleep(10);
+        }
+        if (mRunning)
+        {
+            LL_WARNS("WebSocket") << mServerName << ": I/O thread did not exit within "
+                << SERVER_STOP_WARN_SECONDS << "s; joining anyway. This indicates a "
+                << "thread-boundary regression." << LL_ENDL;
+        }
+
         mServerThread.join();
         LL_INFOS("WebSocket") << "WebSocket server thread joined for: " << mServerName << LL_ENDL;
     }
@@ -463,10 +487,11 @@ void LLWebsocketMgr::WSServer::stop()
 
 bool LLWebsocketMgr::WSServer::isRunning() const
 {
-    LL_ERRS_IF(!mImpl, "WebSocket") << "WebSocket server " << mServerName << " implementation is null !" << LL_ENDL;
-
-    // Check both the thread state, websocket server state, and the stop flag
-    return mServerThread.joinable() && !mImpl->mServer.stopped() && !mShouldStop;
+    // mRunning is set true just before the I/O thread launches and false as
+    // the last thing that thread does before exiting -- lock-free, and safe
+    // to call while start()/stop() hold mThreadMutex on this thread (unlike
+    // a mutex-based check, which would self-deadlock there).
+    return mRunning && !mShouldStop;
 }
 
 void LLWebsocketMgr::WSServer::broadcastMessage(const std::string& message)
@@ -540,6 +565,18 @@ LLWebsocketMgr::WSConnection::ptr_t LLWebsocketMgr::WSServer::getConnection(cons
     return nullptr;
 }
 
+std::vector<LLWebsocketMgr::WSConnection::ptr_t> LLWebsocketMgr::WSServer::getConnections() const
+{
+    LLMutexLock lock(&mConnectionMutex);
+    std::vector<WSConnection::ptr_t> result;
+    result.reserve(mConnections.size());
+    for (const auto& [handle, conn] : mConnections)
+    {
+        result.push_back(conn);
+    }
+    return result;
+}
+
 LLWebsocketMgr::connection_state_t LLWebsocketMgr::WSServer::getConnectionState(const connection_h& handle) const
 {
     websocketpp::lib::error_code ec;
diff --git a/indra/llcorehttp/llwebsocketmgr.h b/indra/llcorehttp/llwebsocketmgr.h
index 2c335307e3..b962b4b117 100644
--- a/indra/llcorehttp/llwebsocketmgr.h
+++ b/indra/llcorehttp/llwebsocketmgr.h
@@ -246,6 +246,10 @@ public:
         connection_state_t  getConnectionState(const connection_h& handle) const;
 
     protected:
+        /// Snapshot of live connections. Safe to iterate after the lock is
+        /// released -- these are real shared_ptrs, not weak references.
+        std::vector<WSConnection::ptr_t> getConnections() const;
+
         virtual WSConnection::ptr_t connectionFactory(WSServer::ptr_t server, connection_h handle);
 
         bool            start();
@@ -283,6 +287,7 @@ public:
         // Threading support
         std::thread                  mServerThread;        ///< Thread running the ASIO event loop
         std::atomic<bool>            mShouldStop{ false }; ///< Thread-safe stop flag
+        std::atomic<bool>            mRunning{ false };     ///< True from just before the I/O thread launches until it exits
         mutable LLMutex              mThreadMutex;         ///< Mutex for thread synchronization
     };
 
diff --git a/indra/newview/llscripteditorws.cpp b/indra/newview/llscripteditorws.cpp
index 8413547935..178b03d4da 100644
--- a/indra/newview/llscripteditorws.cpp
+++ b/indra/newview/llscripteditorws.cpp
@@ -69,6 +69,7 @@
 #include "llviewertexteditor.h"
 #include "llvoinventorylistener.h"
 #include "roles_constants.h"
+#include "workqueue.h"
 
 #include <array>
 
@@ -627,27 +628,27 @@ void LLScriptEditorWSServer::setupConnectionMethods(LLJSONRPCConnection::ptr_t c
         LL_DEBUGS("ScriptEditorWS") << "Setting up script editor connection methods" << LL_ENDL;
         U32 connection_id = script_connection->getConnectionID();
 
-        // Sync methods (run on the WebSocket I/O thread; must not touch
-        // main-thread-only viewer state).
-        script_connection->registerMethod("language.syntax.id",
+        // All per-connection methods dispatch to the main thread inside a
+        // coroutine (see LLJSONRPCConnection::processRequest).
+        script_connection->registerAsyncMethod("language.syntax.id",
             bindHandler([](LLScriptEditorWSServer& s, auto&, auto&, auto&)
             {
                 return s.handleLanguageIdRequest();
             }));
 
-        script_connection->registerMethod("language.syntax",
+        script_connection->registerAsyncMethod("language.syntax",
             bindHandler([](LLScriptEditorWSServer& s, auto&, auto&, const LLSD& params)
             {
                 return s.handleSyntaxRequest(params);
             }));
 
-        script_connection->registerMethod("language.syntax.cache",
+        script_connection->registerAsyncMethod("language.syntax.cache",
             bindHandler([](LLScriptEditorWSServer& s, auto&, auto&, auto&)
             {
                 return s.handleSyntaxCacheRequest();
             }));
 
-        script_connection->registerMethod("language.syntax.get",
+        script_connection->registerAsyncMethod("language.syntax.get",
             bindHandler([](LLScriptEditorWSServer& s, auto&, auto&, const LLSD& params)
             {
                 return s.handleSyntaxCacheFileRequest(params);
@@ -659,7 +660,7 @@ void LLScriptEditorWSServer::setupConnectionMethods(LLJSONRPCConnection::ptr_t c
                 return s.handleScriptSubscribe(connection_id, params);
             }));
 
-        script_connection->registerMethod("script.list",
+        script_connection->registerAsyncMethod("script.list",
             bindHandler([](LLScriptEditorWSServer& s, auto&, auto&, auto&)
             {
                 return s.handleFileWatcherFileListRequest();
@@ -671,7 +672,6 @@ void LLScriptEditorWSServer::setupConnectionMethods(LLJSONRPCConnection::ptr_t c
                 return s.handleObjectUnpublish(connection_id, params);
             }));
 
-        // Async methods (dispatched to the main thread inside a coroutine).
         script_connection->registerAsyncMethod("script.unsubscribe",
             bindHandler([connection_id](LLScriptEditorWSServer& s, auto&, auto&, const LLSD& params)
             {
@@ -2299,7 +2299,6 @@ void LLScriptEditorWSServer::forwardChatToIDE(
 void LLScriptEditorWSServer::sendRuntimeEvent(
     const LLPublishedObjectMgr::RuntimeChatEvent& event) const
 {
-
     std::string script_id;
     if (event.mItemID.notNull())
     {
@@ -2787,6 +2786,26 @@ void LLScriptEditorWSConnection::onOpen()
 
     LL_INFOS("ScriptEditorWS") << "Script editor JSON-RPC connection opened" << LL_ENDL;
 
+    // gAgent, LLVersionInfo, LLSyntaxDefCache, and challenge file I/O below
+    // are main-thread only; this callback runs on the WS I/O thread.
+    wptr_t that = weak_from_this();
+    if (!LL::WorkQueue::postToMainLoop([that]()
+        {
+            auto self = that.lock();
+            if (self)
+            {
+                self->sendHandshake();
+            }
+        }))
+    {
+        LL_WARNS("ScriptEditorWS") << "Main loop unavailable; dropping handshake for new connection" << LL_ENDL;
+    }
+}
+
+void LLScriptEditorWSConnection::sendHandshake()
+{
+    LL_PROFILE_ZONE_SCOPED_CATEGORY_SCRIPTDEV;
+
     // Build hello data
     LLSD handshake;
     handshake["server_version"]   = "1.0.0";
diff --git a/indra/newview/llscripteditorws.h b/indra/newview/llscripteditorws.h
index f85c3e6475..4498ca9107 100644
--- a/indra/newview/llscripteditorws.h
+++ b/indra/newview/llscripteditorws.h
@@ -99,6 +99,7 @@ private:
      */
     void handleHandshakeResponse(const LLSD& result);
     std::string generateChallenge();
+    void sendHandshake();
 
     LLScriptEdContainer*                    getEditor() const;
     std::shared_ptr<LLScriptEditorWSServer> getServer() const;