#4184 clearAndDeparentModels crash

To resolve dual ownership switched children to LLPointer

5 files changed, 20 insertions, 63 deletions

diff --git a/indra/newview/llconversationmodel.cpp b/indra/newview/llconversationmodel.cpp
index 5004055666..bb1daf4ec1 100644
--- a/indra/newview/llconversationmodel.cpp
+++ b/indra/newview/llconversationmodel.cpp
@@ -300,7 +300,7 @@ void LLConversationItemSession::updateName(LLConversationItemParticipant* partic
 
         for (auto itemp : mChildren)
         {
-            LLConversationItem* current_participant = dynamic_cast<LLConversationItem*>(itemp);
+            LLConversationItem* current_participant = dynamic_cast<LLConversationItem*>(itemp.get());
             // Add the avatar uuid to the list (except if it's the own agent uuid)
             if (current_participant->getUUID() != gAgentID)
             {
@@ -329,6 +329,7 @@ void LLConversationItemSession::updateName(LLConversationItemParticipant* partic
 
 void LLConversationItemSession::removeParticipant(LLConversationItemParticipant* participant)
 {
+    LLPointer<LLFolderViewModelItem> holder(participant);
     removeChild(participant);
     mNeedsRefresh = true;
     updateName(participant);
@@ -360,15 +361,10 @@ void LLConversationItemSession::clearAndDeparentModels()
     for (child_list_t::iterator it = mChildren.begin(); it != mChildren.end();)
     {
         LLFolderViewModelItem* child = *it;
-        if (child->getNumRefs() == 0)
+        // Note that model might still be assigned to some view/widget
+        // and have a different parent
+        if (child->getParent() == this)
         {
-            // LLConversationItemParticipant can be created but not assigned to any view,
-            // it was waiting for an "add_participant" event to be processed
-            delete child;
-        }
-        else
-        {
-            // Model is still assigned to some view/widget
             child->setParent(NULL);
         }
         it = mChildren.erase(it);
@@ -383,7 +379,7 @@ LLConversationItemParticipant* LLConversationItemSession::findParticipant(const
     child_list_t::iterator iter;
     for (iter = mChildren.begin(); iter != mChildren.end(); iter++)
     {
-        participant = dynamic_cast<LLConversationItemParticipant*>(*iter);
+        participant = dynamic_cast<LLConversationItemParticipant*>((*iter).get());
         if (participant && participant->hasSameValue(participant_id))
         {
             break;
@@ -493,7 +489,7 @@ const bool LLConversationItemSession::getTime(F64& time) const
     child_list_t::const_iterator iter;
     for (iter = mChildren.begin(); iter != mChildren.end(); iter++)
     {
-        participant = dynamic_cast<LLConversationItemParticipant*>(*iter);
+        participant = dynamic_cast<LLConversationItemParticipant*>((*iter).get());
         F64 participant_time;
         if (participant && participant->getTime(participant_time))
         {
@@ -517,7 +513,7 @@ void LLConversationItemSession::dumpDebugData(bool dump_children)
     {
         for (child_list_t::iterator iter = mChildren.begin(); iter != mChildren.end(); iter++)
         {
-            LLConversationItemParticipant* participant = dynamic_cast<LLConversationItemParticipant*>(*iter);
+            LLConversationItemParticipant* participant = dynamic_cast<LLConversationItemParticipant*>((*iter).get());
             if (participant)
             {
                 participant->dumpDebugData();
diff --git a/indra/newview/llfloaterchatmentionpicker.cpp b/indra/newview/llfloaterchatmentionpicker.cpp
index 1cfed122a9..a3eb286375 100644
--- a/indra/newview/llfloaterchatmentionpicker.cpp
+++ b/indra/newview/llfloaterchatmentionpicker.cpp
@@ -88,7 +88,7 @@ uuid_vec_t LLFloaterChatMentionPicker::getParticipantIds()
     LLFolderViewModelItemCommon::child_list_t::const_iterator end_participant_model = item->getChildrenEnd();
     while (current_participant_model != end_participant_model)
     {
-        LLConversationItem* participant_model = dynamic_cast<LLConversationItem*>(*current_participant_model);
+        LLConversationItem* participant_model = dynamic_cast<LLConversationItem*>((*current_participant_model).get());
         if (participant_model)
         {
             avatar_ids.push_back(participant_model->getUUID());
diff --git a/indra/newview/llfloaterimcontainer.cpp b/indra/newview/llfloaterimcontainer.cpp
index 72d4d30dcf..4312c058a1 100644
--- a/indra/newview/llfloaterimcontainer.cpp
+++ b/indra/newview/llfloaterimcontainer.cpp
@@ -460,7 +460,7 @@ void LLFloaterIMContainer::processParticipantsStyleUpdate()
         LLFolderViewModelItemCommon::child_list_t::const_iterator end_participant_model = session_model->getChildrenEnd();
         while (current_participant_model != end_participant_model)
         {
-            LLConversationItemParticipant* participant_model = dynamic_cast<LLConversationItemParticipant*>(*current_participant_model);
+            LLConversationItemParticipant* participant_model = dynamic_cast<LLConversationItemParticipant*>((*current_participant_model).get());
             if (participant_model)
             {
                 // Get the avatar name for this participant id from the cache and update the model
@@ -511,7 +511,7 @@ void LLFloaterIMContainer::idleUpdate()
                 bool can_ban = haveAbilityToBan();
                 while (current_participant_model != end_participant_model)
                 {
-                    LLConversationItemParticipant* participant_model = dynamic_cast<LLConversationItemParticipant*>(*current_participant_model);
+                    LLConversationItemParticipant* participant_model = dynamic_cast<LLConversationItemParticipant*>((*current_participant_model).get());
                     if (participant_model)
                     {
                         participant_model->setModeratorOptionsVisible(is_moderator);
@@ -1874,7 +1874,7 @@ LLConversationItem* LLFloaterIMContainer::addConversationListItem(const LLUUID&
         LLFolderViewModelItemCommon::child_list_t::const_iterator end_participant_model = item->getChildrenEnd();
         while (current_participant_model != end_participant_model)
         {
-            LLConversationItem* participant_model = dynamic_cast<LLConversationItem*>(*current_participant_model);
+            LLConversationItem* participant_model = dynamic_cast<LLConversationItem*>((*current_participant_model).get());
             LLConversationViewParticipant* participant_view = createConversationViewParticipant(participant_model);
             participant_view->addToFolder(widget);
             current_participant_model++;
diff --git a/indra/newview/llfloaterimsessiontab.cpp b/indra/newview/llfloaterimsessiontab.cpp
index 7c3d2b511b..e03422780a 100644
--- a/indra/newview/llfloaterimsessiontab.cpp
+++ b/indra/newview/llfloaterimsessiontab.cpp
@@ -107,26 +107,6 @@ LLFloaterIMSessionTab::~LLFloaterIMSessionTab()
     delete mRefreshTimer;
     LLIMMgr::instance().removeSessionObserver(this);
     mEmojiCloseConn.disconnect();
-
-    LLFloaterIMContainer* im_container = LLFloaterIMContainer::findInstance();
-    if (im_container)
-    {
-        LLParticipantList* session = dynamic_cast<LLParticipantList*>(im_container->getSessionModel(mSessionID));
-        if (session)
-        {
-            for (const conversations_widgets_map::value_type& widget_pair : mConversationsWidgets)
-            {
-                LLFolderViewItem* widget = widget_pair.second;
-                LLFolderViewModelItem* item_vmi = widget->getViewModelItem();
-                if (item_vmi && item_vmi->getNumRefs() == 1)
-                {
-                    // This is the last pointer, remove participant from session
-                    // before participant gets deleted on destroyView.
-                    session->removeChild(item_vmi);
-                }
-            }
-        }
-    }
 }
 
 // static
@@ -730,7 +710,7 @@ void LLFloaterIMSessionTab::buildConversationViewParticipant()
     LLFolderViewModelItemCommon::child_list_t::const_iterator end_participant_model = item->getChildrenEnd();
     while (current_participant_model != end_participant_model)
     {
-        LLConversationItem* participant_model = dynamic_cast<LLConversationItem*>(*current_participant_model);
+        LLConversationItem* participant_model = dynamic_cast<LLConversationItem*>((*current_participant_model).get());
         if (participant_model)
         {
             addConversationViewParticipant(participant_model);
@@ -774,27 +754,6 @@ void LLFloaterIMSessionTab::removeConversationViewParticipant(const LLUUID& part
     LLFolderViewItem* widget = get_ptr_in_map(mConversationsWidgets,participant_id);
     if (widget)
     {
-        LLFolderViewModelItem* item_vmi = widget->getViewModelItem();
-        if (item_vmi && item_vmi->getNumRefs() == 1)
-        {
-            // This is the last pointer, remove participant from session
-            // before participant gets deleted on destroyView.
-            //
-            // Floater (widget) and participant's view can simultaneously
-            // co-own the model, in which case view is responsible for
-            // the deletion and floater is free to clear and recreate
-            // the list, yet there are cases where only widget owns
-            // the pointer so it should do the cleanup.
-            // See "add_participant".
-            //
-            // Todo: If it keeps causing issues turn participants
-            // into LLPointers in the session
-            LLParticipantList* session = getParticipantList();
-            if (session)
-            {
-                session->removeChild(item_vmi);
-            }
-        }
         widget->destroyView();
     }
     mConversationsWidgets.erase(participant_id);
@@ -860,7 +819,7 @@ void LLFloaterIMSessionTab::refreshConversation()
             LLIMSpeakerMgr *speaker_mgr = LLIMModel::getInstance()->getSpeakerManager(mSessionID);
             while (current_participant_model != end_participant_model)
             {
-                LLConversationItemParticipant* participant_model = dynamic_cast<LLConversationItemParticipant*>(*current_participant_model);
+                LLConversationItemParticipant* participant_model = dynamic_cast<LLConversationItemParticipant*>((*current_participant_model).get());
                 if (speaker_mgr && participant_model)
                 {
                     LLSpeaker *participant_speaker = speaker_mgr->findSpeaker(participant_model->getUUID());
diff --git a/indra/newview/llinventoryfunctions.cpp b/indra/newview/llinventoryfunctions.cpp
index b577e302a8..0365b06edb 100644
--- a/indra/newview/llinventoryfunctions.cpp
+++ b/indra/newview/llinventoryfunctions.cpp
@@ -3998,15 +3998,17 @@ void LLInventoryAction::buildMarketplaceFolders(LLFolderView* root)
     for (; set_iter != selected_items.end(); ++set_iter)
     {
         viewModel = dynamic_cast<LLFolderViewModelItemInventory *>((*set_iter)->getViewModelItem());
-        if (!viewModel || !viewModel->getInventoryObject()) continue;
-        if (gInventory.isObjectDescendentOf(viewModel->getInventoryObject()->getParentUUID(), marketplacelistings_id))
+        if (!viewModel) continue;
+        LLInventoryObject* inv_obj = viewModel->getInventoryObject();
+        if (!inv_obj) continue;
+        if (gInventory.isObjectDescendentOf(inv_obj->getParentUUID(), marketplacelistings_id))
         {
-            const LLUUID &parent_id = viewModel->getInventoryObject()->getParentUUID();
+            const LLUUID &parent_id = inv_obj->getParentUUID();
             if (parent_id != marketplacelistings_id)
             {
                 sMarketplaceFolders.push_back(parent_id);
             }
-            const LLUUID &curr_id = viewModel->getInventoryObject()->getUUID();
+            const LLUUID &curr_id = inv_obj->getUUID();
             if (curr_id != marketplacelistings_id)
             {
                 sMarketplaceFolders.push_back(curr_id);