From d186daa361567fe499b80efe8481e192ffe8199b Mon Sep 17 00:00:00 2001 From: Rider Linden Date: Wed, 23 Sep 2026 11:51:39 -0700 Subject: Issue #6205: There was an issue with permissions not being sent in the update message. --- doc/external-editor-json-rpc.md | 18 +++- indra/newview/llpublishedobjectmgr.cpp | 166 +++++++++++++++++++++++++++------ indra/newview/llpublishedobjectmgr.h | 15 +++ indra/newview/llscripteditorws.cpp | 62 ++++++------ indra/newview/llscripteditorws.h | 3 +- indra/newview/llselectmgr.cpp | 2 + 6 files changed, 206 insertions(+), 60 deletions(-) diff --git a/doc/external-editor-json-rpc.md b/doc/external-editor-json-rpc.md index 0c32c043fe..b62e002a33 100644 --- a/doc/external-editor-json-rpc.md +++ b/doc/external-editor-json-rpc.md @@ -1084,7 +1084,7 @@ interface LinkedObject { interface ObjectPermissions { owner: number; - next_owner: number; + next_owner?: number; // Absent until the viewer receives ObjectProperties for the prim } /** Root of a linkset, as published to the extension */ @@ -1235,19 +1235,30 @@ Sent when an explored object's inventory, properties, or linkset membership chan | ------- | ------- | | Root prim inventory changed | `object_id`, `inventory` (complete replacement array) | | Child prim inventory changed | `object_id`, `changes.linked_objects.modified[]` with `link_id` and `inventory` (complete replacement array for that child) | -| Root prim name/description changed | `object_id`, `object_name` and/or `object_description` | -| Child prim name/description changed | `object_id`, `changes.linked_objects.modified[]` with `link_id` and `link_name` and/or `link_description` | +| Root prim properties changed | `object_id`, plus any changed subset of `object_name`, `object_description`, `owner_id`, `permissions`, `can_save_back` | +| Child prim properties changed | `object_id`, `changes.linked_objects.modified[]` with `link_id` plus any changed subset of `link_name`, `link_description`, `permissions` | | Linkset membership changed | `object_id`, `linked_objects` (complete replacement array covering every child prim) | `inventory` and `linked_objects` are always complete replacements of the prior state, never increments. Linkset membership changes are coalesced behind a short flush delay, so several rapid link or unlink operations may arrive as a single update. +**Deferred permission fields.** `object.publish` is gated on inventory loading, which can finish +before the `ObjectProperties` reply arrives. Until it does the viewer derives `permissions.owner` +from the object's update flags, which describe the current agent's rights, and omits `next_owner`. +`owner_id` may be absent or the null UUID, and `can_save_back` may be `false` because it depends on +selection state. Once properties arrive the viewer sends an `object.update` carrying the full +`owner`/`next_owner` block and the real `owner_id`, so clients must merge these fields on update +rather than assuming the publish payload is final. + ```typescript interface ObjectUpdateMessage { object_id: string; object_name?: string; object_description?: string; + owner_id?: string; + permissions?: ObjectPermissions; + can_save_back?: boolean; // Full replacement inventory?: ObjectInventoryItem[]; linked_objects?: LinkedObject[]; @@ -1264,6 +1275,7 @@ interface LinkedObjectChanges { link_id: string; link_name?: string; link_description?: string; + permissions?: ObjectPermissions; inventory?: ObjectInventoryItem[]; // Complete replacement array, not a delta }[]; } diff --git a/indra/newview/llpublishedobjectmgr.cpp b/indra/newview/llpublishedobjectmgr.cpp index 85a9542dce..5cbb7ca76d 100644 --- a/indra/newview/llpublishedobjectmgr.cpp +++ b/indra/newview/llpublishedobjectmgr.cpp @@ -32,6 +32,7 @@ #include "llchat.h" #include "llinventorydefines.h" #include "llregex.h" +#include "llsdutil.h" #include "llselectmgr.h" #include "llviewerinventory.h" #include "llviewerobject.h" @@ -66,7 +67,7 @@ namespace return std::string(s); } - std::string get_prim_name(LLViewerObject* obj) + std::string get_prim_name(LLViewerObject* obj, S32 link_number = 1) { std::string name = nv_string(obj, "Name"); if (!name.empty()) @@ -85,25 +86,17 @@ namespace return node->mName; } - // Never emit an empty prim/object name to downstream tooling. - return obj->getID().asString(); + // Real prim names need a full ObjectProperties reply, which requires selection. + return (link_number > 1) ? llformat("Link #%d", link_number) : std::string("Object"); } void add_object_permissions(LLSD& object_data, LLViewerObject* object) { - LLPermissions* permissions = - LLSelectMgr::getInstance()->findObjectPermissions(object); - if (!permissions) + LLSD permissions = LLPublishedObjectMgr::getObjectPermissionsLLSD(object); + if (permissions.isDefined()) { - return; + object_data["permissions"] = permissions; } - - LLSD permission_entry; - permission_entry["owner"] = - static_cast(permissions->getMaskOwner()); - permission_entry["next_owner"] = - static_cast(permissions->getMaskNextOwner()); - object_data["permissions"] = permission_entry; } } @@ -742,6 +735,59 @@ LLSD LLPublishedObjectMgr::buildPrimInventoryLLSD(LLViewerObject* object) const return items; } +LLSD LLPublishedObjectMgr::makePermissionsLLSD(U32 owner_mask, U32 next_owner_mask) +{ + LLSD permissions; + permissions["owner"] = static_cast(owner_mask); + permissions["next_owner"] = static_cast(next_owner_mask); + return permissions; +} + +LLSD LLPublishedObjectMgr::getObjectPermissionsLLSD(LLViewerObject* object) +{ + if (!object) + { + return LLSD(); + } + + LLPermissions* permissions = + LLSelectMgr::getInstance()->findObjectPermissions(object); + if (permissions) + { + return makePermissionsLLSD(permissions->getMaskOwner(), permissions->getMaskNextOwner()); + } + + // Unselected prims carry no LLPermissions, but object flags still hold this agent's rights. + // next_owner is unavailable from flags and is omitted until ObjectProperties arrives. + U32 owner_mask = 0; + if (object->permModify()) { owner_mask |= PERM_MODIFY; } + if (object->permCopy()) { owner_mask |= PERM_COPY; } + if (object->permTransfer()) { owner_mask |= PERM_TRANSFER; } + if (object->permMove()) { owner_mask |= PERM_MOVE; } + + LLSD permission_entry; + permission_entry["owner"] = static_cast(owner_mask); + return permission_entry; +} + +bool LLPublishedObjectMgr::computeCanSaveBack(LLViewerObject* root, LLUUID& source_task_id) +{ + source_task_id.setNull(); + if (!root || root->isAttachment()) + { + return false; + } + + LLSelectNode* node = LLSelectMgr::instance().getSelection()->findNode(root); + if (!node || !node->mValid || node->mFromTaskID.isNull()) + { + return false; + } + + source_task_id = node->mFromTaskID; + return true; +} + LLSD LLPublishedObjectMgr::buildPublishedObjectLLSD(LLViewerObject* root) const { LL_PROFILE_ZONE_SCOPED_CATEGORY_SCRIPTDEV; @@ -768,12 +814,13 @@ LLSD LLPublishedObjectMgr::buildPublishedObjectLLSD(LLViewerObject* root) const { LLSD link; link["link_id"] = child->getID(); - link["link_number"] = link_number++; - link["link_name"] = get_prim_name(child); + link["link_number"] = link_number; + link["link_name"] = get_prim_name(child, link_number); link["link_description"] = nv_string(child, "Desc"); add_object_permissions(link, child); link["inventory"] = buildPrimInventoryLLSD(child); linked_objects.append(link); + ++link_number; } if (linked_objects.size() > 0) { @@ -1083,25 +1130,37 @@ bool LLPublishedObjectMgr::applyPropertyChange( if (prim_id == root_id) { - bool has_name = !name.empty(); - bool name_changed = has_name && (pub_info->mObjectName != name); - bool desc_changed = (pub_info->mObjectDescription != desc); - if (!name_changed && !desc_changed) - { - return false; - } + bool changed = false; - if (name_changed) + if (!name.empty() && pub_info->mObjectName != name) { pub_info->mObjectName = name; update["object_name"] = name; + changed = true; } - if (desc_changed) + if (pub_info->mObjectDescription != desc) { pub_info->mObjectDescription = desc; update["object_description"] = desc; + changed = true; } - return true; + + // Properties arrive after publish, so this field is backfilled here. + LLViewerObject* root = gObjectList.findObject(root_id); + if (root) + { + LLUUID source_task_id; + const bool can_save_back = computeCanSaveBack(root, source_task_id); + if (can_save_back != pub_info->mCanSaveBackToContents) + { + pub_info->mCanSaveBackToContents = can_save_back; + pub_info->mSourceTaskID = source_task_id; + update["can_save_back"] = can_save_back; + changed = true; + } + } + + return changed; } auto prim_it = std::find_if(pub_info->mPrims.begin(), pub_info->mPrims.end(), @@ -1135,6 +1194,61 @@ bool LLPublishedObjectMgr::applyPropertyChange( return true; } +bool LLPublishedObjectMgr::applyPermissionsChange( + const LLUUID& root_id, + const LLUUID& prim_id, + const LLUUID& owner_id, + U32 owner_mask, + U32 next_owner_mask, + LLSD& update) +{ + PublishedObjectInfo* pub_info = getPublished(root_id); + if (!pub_info) + { + return false; + } + + const LLSD permissions = makePermissionsLLSD(owner_mask, next_owner_mask); + + update = LLSD(); + update["object_id"] = root_id; + + if (prim_id == root_id) + { + bool changed = false; + if (!llsd_equals(permissions, pub_info->mPermissions)) + { + pub_info->mPermissions = permissions; + update["permissions"] = permissions; + changed = true; + } + if (owner_id.notNull() && pub_info->mOwnerID != owner_id) + { + pub_info->mOwnerID = owner_id; + update["owner_id"] = owner_id; + changed = true; + } + return changed; + } + + auto prim_it = std::find_if(pub_info->mPrims.begin(), pub_info->mPrims.end(), + [&](const PublishedPrimInfo& p) { return p.mPrimID == prim_id; }); + if (prim_it == pub_info->mPrims.end() || llsd_equals(permissions, prim_it->mPermissions)) + { + return false; + } + + prim_it->mPermissions = permissions; + + LLSD modified_entry; + modified_entry["link_id"] = prim_id; + modified_entry["permissions"] = permissions; + LLSD modified_arr = LLSD::emptyArray(); + modified_arr.append(modified_entry); + update["changes"]["linked_objects"]["modified"] = modified_arr; + return true; +} + bool LLPublishedObjectMgr::hasActiveLinksetFlushTimer(const LLUUID& root_id) const { auto it = mLinksetFlushTimers.find(root_id); diff --git a/indra/newview/llpublishedobjectmgr.h b/indra/newview/llpublishedobjectmgr.h index 51af129dcb..8a6c8b705c 100644 --- a/indra/newview/llpublishedobjectmgr.h +++ b/indra/newview/llpublishedobjectmgr.h @@ -55,6 +55,7 @@ public: S32 mLinkNumber; std::string mPrimDescription; S16 mInventorySerial; + LLSD mPermissions; }; struct PublishedObjectInfo @@ -73,6 +74,7 @@ public: std::string mRegionName; bool mCanSaveBackToContents{ false }; LLUUID mSourceTaskID; + LLSD mPermissions; std::vector mPrims; std::vector> mListeners; }; @@ -256,6 +258,12 @@ public: LLSD buildPrimInventoryLLSD(LLViewerObject* object) const; LLSD buildPublishedObjectLLSD(LLViewerObject* root) const; LLSD buildObjectListLLSD() const; + + // Both depend on a valid LLSelectNode, so they yield nothing until ObjectProperties arrives. + static LLSD getObjectPermissionsLLSD(LLViewerObject* object); + static bool computeCanSaveBack(LLViewerObject* root, LLUUID& source_task_id); + static LLSD makePermissionsLLSD(U32 owner_mask, U32 next_owner_mask); + bool buildLinksetUpdateLLSD(const LLUUID& root_id, LLSD& update) const; bool reconcileLinksetChildAdded(const LLUUID& root_id, LLViewerObject* child, F64 request_start_sec); bool reconcileLinksetChildRemoved(const LLUUID& root_id, const LLUUID& child_id); @@ -275,6 +283,13 @@ public: const std::string& name, const std::string& desc, LLSD& update); + bool applyPermissionsChange( + const LLUUID& root_id, + const LLUUID& prim_id, + const LLUUID& owner_id, + U32 owner_mask, + U32 next_owner_mask, + LLSD& update); void beginPendingPublish(const LLUUID& object_id, const std::vector& prims); bool hasPendingPublish(const LLUUID& object_id) const; diff --git a/indra/newview/llscripteditorws.cpp b/indra/newview/llscripteditorws.cpp index 017d9a5540..8413547935 100644 --- a/indra/newview/llscripteditorws.cpp +++ b/indra/newview/llscripteditorws.cpp @@ -2395,7 +2395,7 @@ void LLScriptEditorWSServer::notifyAll(const std::string& method, const LLSD& pa // static -std::string LLScriptEditorWSServer::getPrimName(LLViewerObject* obj) +std::string LLScriptEditorWSServer::getPrimName(LLViewerObject* obj, S32 link_number) { std::string name = nv_string(obj, "Name"); if (!name.empty()) @@ -2414,8 +2414,8 @@ std::string LLScriptEditorWSServer::getPrimName(LLViewerObject* obj) return node->mName; } - // Never emit an empty prim/object name to downstream tooling. - return obj->getID().asString(); + // Real prim names need a full ObjectProperties reply, which requires selection. + return (link_number > 1) ? llformat("Link #%d", link_number) : std::string("Object"); } bool LLScriptEditorWSServer::publishObject(const LLUUID& object_id) @@ -2443,12 +2443,8 @@ bool LLScriptEditorWSServer::publishObject(const LLUUID& object_id) // Collect root + all children std::vector prims = collect_linkset(root); - // Request object properties for each prim in the linkset (root + children), - // matching the hover path so name/description metadata is refreshed. - for (LLViewerObject* prim : prims) - { - LLSelectMgr::instance().requestObjectPropertiesFamily(prim); - } + // The sim coalesces properties-family requests to the root, so one request covers the linkset. + LLSelectMgr::instance().requestObjectPropertiesFamily(root); // Set up a PendingPublish to coordinate inventory loading across all prims. // We register a listener and call requestInventory() on every prim. @@ -2513,24 +2509,14 @@ void LLScriptEditorWSServer::buildAndSendPublish(const LLUUID& object_id) info.mOwnerID = root->mOwnerID; info.mObjectName = pub["object_name"].asString(); info.mObjectDescription = pub["object_description"].asString(); + info.mPermissions = LLPublishedObjectMgr::getObjectPermissionsLLSD(root); if (root->getRegion()) { info.mRegionName = root->getRegion()->getName(); } - LLSelectNode* root_select_node = LLSelectMgr::instance().getSelection()->findNode(root); - if (root_select_node - && root_select_node->mValid - && !root_select_node->mFromTaskID.isNull() - && !root->isAttachment()) - { - info.mCanSaveBackToContents = true; - info.mSourceTaskID = root_select_node->mFromTaskID; - } - else - { - info.mCanSaveBackToContents = false; - info.mSourceTaskID.setNull(); - } + LLUUID source_task_id; + info.mCanSaveBackToContents = LLPublishedObjectMgr::computeCanSaveBack(root, source_task_id); + info.mSourceTaskID = source_task_id; S32 link_num = 1; std::vector prims = collect_linkset(root); @@ -2538,9 +2524,10 @@ void LLScriptEditorWSServer::buildAndSendPublish(const LLUUID& object_id) { LLPublishedObjectMgr::PublishedPrimInfo prim_info; prim_info.mPrimID = prim->getID(); - prim_info.mPrimName = getPrimName(prim); // Use helper with selection fallback prim_info.mLinkNumber = link_num++; + prim_info.mPrimName = getPrimName(prim, prim_info.mLinkNumber); prim_info.mInventorySerial = static_cast(prim->getInventorySerial()); + prim_info.mPermissions = LLPublishedObjectMgr::getObjectPermissionsLLSD(prim); info.mPrims.push_back(prim_info); } @@ -2581,12 +2568,9 @@ void LLScriptEditorWSServer::buildAndSendPublish(const LLUUID& object_id) << " (" << pub["object_name"].asString() << ") with " << (prims.size() - 1) << " linked prim(s)" << LL_ENDL; - // Re-request object properties now that the object is published so - // onObjectPropertyChanged can emit object.update for root and linked prims. - for (LLViewerObject* prim : prims) - { - LLSelectMgr::instance().requestObjectPropertiesFamily(prim); - } + // Re-request now that the object is published so onObjectPropertyChanged can emit + // object.update. The sim answers per linkset, so only the root needs asking. + LLSelectMgr::instance().requestObjectPropertiesFamily(root); } void LLScriptEditorWSServer::onLinksetChildAdded(const LLUUID& root_id, LLViewerObject* child) @@ -2748,6 +2732,24 @@ void LLScriptEditorWSServer::onObjectPropertyChanged( } } +void LLScriptEditorWSServer::onObjectPermissionsReceived( + const LLUUID& prim_id, const LLUUID& owner_id, U32 owner_mask, U32 next_owner_mask) +{ + LL_PROFILE_ZONE_SCOPED_CATEGORY_SCRIPTDEV; + LLViewerObject* prim = gObjectList.findObject(prim_id); + if (!prim) + { + return; + } + + LLSD update; + if (mPublishedObjectManager.applyPermissionsChange( + prim->getRootEdit()->getID(), prim_id, owner_id, owner_mask, next_owner_mask, update)) + { + notifyAll("object.update", update); + } +} + void LLScriptEditorWSServer::unpublishObject(const LLUUID& object_id, const std::string& reason) { LL_PROFILE_ZONE_SCOPED_CATEGORY_SCRIPTDEV; diff --git a/indra/newview/llscripteditorws.h b/indra/newview/llscripteditorws.h index 6eb8385210..f85c3e6475 100644 --- a/indra/newview/llscripteditorws.h +++ b/indra/newview/llscripteditorws.h @@ -225,6 +225,7 @@ public: void onPrimInventoryReady(const LLUUID& object_id, const LLUUID& prim_id); void onPrimInventoryChanged(const LLUUID& object_id, const LLUUID& prim_id); void onObjectPropertyChanged(const LLUUID& prim_id, const std::string& name, const std::string& desc, S16 inventory_serial = -1); + void onObjectPermissionsReceived(const LLUUID& prim_id, const LLUUID& owner_id, U32 owner_mask, U32 next_owner_mask); void onLinksetChildAdded(const LLUUID& root_id, LLViewerObject* child); void onLinksetChildRemoved(const LLUUID& root_id, const LLUUID& child_id); @@ -280,7 +281,7 @@ protected: ValidatedItem validatePublishedItem(const LLSD& params, U32 permMask) const; // --- Object Content Publishing (helpers) --- - static std::string getPrimName(LLViewerObject* obj); + static std::string getPrimName(LLViewerObject* obj, S32 link_number = 1); void notifyConnection(U32 connection_id, const std::string& method, const LLSD& params) const; void notifyAll(const std::string& method, const LLSD& params) const; void buildAndSendPublish(const LLUUID& object_id); diff --git a/indra/newview/llselectmgr.cpp b/indra/newview/llselectmgr.cpp index 432678b132..f9108fcc64 100644 --- a/indra/newview/llselectmgr.cpp +++ b/indra/newview/llselectmgr.cpp @@ -6115,6 +6115,7 @@ void LLSelectMgr::processObjectProperties(LLMessageSystem* msg, void** user_data if (ws_server) { ws_server->onObjectPropertyChanged(id, name, desc, inv_serial); + ws_server->onObjectPermissionsReceived(id, owner_id, owner_mask, next_owner_mask); } } @@ -6215,6 +6216,7 @@ void LLSelectMgr::processObjectPropertiesFamily(LLMessageSystem* msg, void** use if (auto ws_server = LLScriptEditorWSServer::getServer()) { ws_server->onObjectPropertyChanged(id, name, desc); + ws_server->onObjectPermissionsReceived(id, owner_id, owner_mask, next_owner_mask); } } -- cgit v1.3 From 500587a04bf3a8793e8f32fe397e0b4e469aedb4 Mon Sep 17 00:00:00 2001 From: Rider Linden Date: Wed, 23 Sep 2026 16:28:04 -0700 Subject: 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. --- indra/llcommon/workqueue.cpp | 6 +++++ indra/llcommon/workqueue.h | 10 ++++++++ indra/llcorehttp/lljsonrpcws.cpp | 46 +++++++++++++++++++++---------------- indra/llcorehttp/lljsonrpcws.h | 22 +++++++++++------- indra/llcorehttp/llwebsocketmgr.cpp | 45 ++++++++++++++++++++++++++++++++---- indra/llcorehttp/llwebsocketmgr.h | 5 ++++ indra/newview/llscripteditorws.cpp | 37 +++++++++++++++++++++-------- indra/newview/llscripteditorws.h | 1 + 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; 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 @@ -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(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 #include #include #include -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 mPendingDeadlines; - std::weak_ptr 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 mTotalRequestsHandled{0}; std::atomic 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 #include @@ -47,6 +48,10 @@ namespace using Server_t = websocketpp::server; using Client_t = websocketpp::client; using Connection_t = websocketpp::connection; + + // 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::WSServer::getConnections() const +{ + LLMutexLock lock(&mConnectionMutex); + std::vector 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 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 mShouldStop{ false }; ///< Thread-safe stop flag + std::atomic 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 @@ -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 getServer() const; -- cgit v1.3