#4184 clearAndDeparentModels crash

To resolve dual ownership switched children to LLPointer

6 files changed, 54 insertions, 96 deletions

diff --git a/indra/llui/llfolderviewmodel.h b/indra/llui/llfolderviewmodel.h
index ba9d0b8de3..2865b789b9 100644
--- a/indra/llui/llfolderviewmodel.h
+++ b/indra/llui/llfolderviewmodel.h
@@ -224,6 +224,7 @@ public:
     virtual S32 getSortVersion() = 0;
     virtual void setSortVersion(S32 version) = 0;
     virtual void setParent(LLFolderViewModelItem* parent) = 0;
+    virtual const LLFolderViewModelItem* getParent() = 0;
     virtual bool hasParent() = 0;
 
 protected:
@@ -254,14 +255,14 @@ public:
         mChildren.clear();
     }
 
-    void requestSort() { mSortVersion = -1; }
-    S32 getSortVersion() { return mSortVersion; }
-    void setSortVersion(S32 version) { mSortVersion = version;}
+    void requestSort() override { mSortVersion = -1; }
+    S32 getSortVersion() override { return mSortVersion; }
+    void setSortVersion(S32 version) override { mSortVersion = version;}
 
-    S32 getLastFilterGeneration() const { return mLastFilterGeneration; }
+    S32 getLastFilterGeneration() const override { return mLastFilterGeneration; }
     S32 getLastFolderFilterGeneration() const { return mLastFolderFilterGeneration; }
-    S32 getMarkedDirtyGeneration() const { return mMarkedDirtyGeneration; }
-    void dirtyFilter()
+    S32 getMarkedDirtyGeneration() const override { return mMarkedDirtyGeneration; }
+    void dirtyFilter() override
     {
         if(mMarkedDirtyGeneration < 0)
         {
@@ -276,7 +277,7 @@ public:
             mParent->dirtyFilter();
         }
     }
-    void dirtyDescendantsFilter()
+    void dirtyDescendantsFilter() override
     {
         mMostFilteredDescendantGeneration = -1;
         if (mParent)
@@ -284,13 +285,13 @@ public:
             mParent->dirtyDescendantsFilter();
         }
     }
-    bool hasFilterStringMatch();
-    std::string::size_type getFilterStringOffset();
-    std::string::size_type getFilterStringSize();
+    bool hasFilterStringMatch() override;
+    std::string::size_type getFilterStringOffset() override;
+    std::string::size_type getFilterStringSize() override;
 
-    typedef std::list<LLFolderViewModelItem*> child_list_t;
+    typedef std::list<LLPointer<LLFolderViewModelItem> > child_list_t;
 
-    virtual void addChild(LLFolderViewModelItem* child)
+    virtual void addChild(LLFolderViewModelItem* child) override
     {
         mChildren.push_back(child);
         child->setParent(this);
@@ -298,15 +299,15 @@ public:
         requestSort();
     }
 
-    virtual void removeChild(LLFolderViewModelItem* child)
+    virtual void removeChild(LLFolderViewModelItem* child) override final
     {
-        mChildren.remove(child);
         child->setParent(NULL);
+        mChildren.remove(child);
         dirtyDescendantsFilter();
         dirtyFilter();
     }
 
-    virtual void clearChildren()
+    virtual void clearChildren() override
     {
         // We are working with models that belong to views as LLPointers, clean the list, let poiters handle the rest
         std::for_each(mChildren.begin(), mChildren.end(), [](LLFolderViewModelItem* c) {c->setParent(NULL); });
@@ -319,7 +320,7 @@ public:
     child_list_t::const_iterator getChildrenEnd() const { return mChildren.end(); }
     child_list_t::size_type getChildrenCount() const { return mChildren.size(); }
 
-    void setPassedFilter(bool passed, S32 filter_generation, std::string::size_type string_offset = std::string::npos, std::string::size_type string_size = 0)
+    void setPassedFilter(bool passed, S32 filter_generation, std::string::size_type string_offset = std::string::npos, std::string::size_type string_size = 0) override
     {
         mPassedFilter = passed;
         mLastFilterGeneration = filter_generation;
@@ -328,20 +329,20 @@ public:
         mMarkedDirtyGeneration = -1;
     }
 
-    void setPassedFolderFilter(bool passed, S32 filter_generation)
+    void setPassedFolderFilter(bool passed, S32 filter_generation) override
     {
         mPassedFolderFilter = passed;
         mLastFolderFilterGeneration = filter_generation;
     }
 
-    virtual bool potentiallyVisible()
+    virtual bool potentiallyVisible() override
     {
         return passedFilter() // we've passed the filter
             || (getLastFilterGeneration() < mRootViewModel.getFilter().getFirstSuccessGeneration()) // or we don't know yet
             || descendantsPassedFilter();
     }
 
-    virtual bool passedFilter(S32 filter_generation = -1)
+    virtual bool passedFilter(S32 filter_generation = -1) override
     {
         if (filter_generation < 0)
         {
@@ -352,7 +353,7 @@ public:
         return passed_folder_filter && (passed_filter || descendantsPassedFilter(filter_generation));
     }
 
-    virtual bool descendantsPassedFilter(S32 filter_generation = -1)
+    virtual bool descendantsPassedFilter(S32 filter_generation = -1) override
     {
         if (filter_generation < 0)
         {
@@ -361,10 +362,10 @@ public:
         return mMostFilteredDescendantGeneration >= filter_generation;
     }
 
-
 protected:
-    virtual void setParent(LLFolderViewModelItem* parent) { mParent = parent; }
-    virtual bool hasParent() { return mParent != NULL; }
+    virtual void setParent(LLFolderViewModelItem* parent) override final { mParent = parent; }
+    virtual const LLFolderViewModelItem* getParent() override { return mParent; };
+    virtual bool hasParent() override { return mParent != NULL; }
 
     S32                         mSortVersion;
     bool                        mPassedFilter;
@@ -381,7 +382,7 @@ protected:
     LLFolderViewModelItem*      mParent;
     LLFolderViewModelInterface& mRootViewModel;
 
-    void setFolderViewItem(LLFolderViewItem* folder_view_item) { mFolderViewItem = folder_view_item;}
+    void setFolderViewItem(LLFolderViewItem* folder_view_item) override { mFolderViewItem = folder_view_item;}
     LLFolderViewItem*       mFolderViewItem;
 };
 
@@ -395,15 +396,15 @@ public:
         mFolderView(NULL)
     {}
 
-    virtual void requestSortAll()
+    virtual void requestSortAll() override
     {
         // sort everything
         mTargetSortVersion++;
     }
-    virtual std::string getStatusText(bool is_empty_folder = false);
-    virtual void filter();
+    virtual std::string getStatusText(bool is_empty_folder = false) override;
+    virtual void filter() override;
 
-    void setFolderView(LLFolderView* folder_view) { mFolderView = folder_view;}
+    void setFolderView(LLFolderView* folder_view) override { mFolderView = folder_view;}
 
 protected:
     bool needsSort(class LLFolderViewModelItem* item);
@@ -433,14 +434,14 @@ public:
     virtual const SortType& getSorter() const        { return *mSorter; }
     virtual void setSorter(const SortType& sorter)   { mSorter.reset(new SortType(sorter)); requestSortAll(); }
 
-    virtual FilterType& getFilter()                  { return *mFilter; }
-    virtual const FilterType& getFilter() const      { return *mFilter; }
+    virtual FilterType& getFilter() override                  { return *mFilter; }
+    virtual const FilterType& getFilter() const override      { return *mFilter; }
     virtual void setFilter(const FilterType& filter) { mFilter.reset(new FilterType(filter)); }
 
     // By default, we assume the content is available. If a network fetch mechanism is implemented for the model,
     // this method needs to be overloaded and return the relevant fetch status.
-    virtual bool contentsReady()                    { return true; }
-    virtual bool isFolderComplete(LLFolderViewFolder* folder)                   { return true; }
+    virtual bool contentsReady() override                     { return true; }
+    virtual bool isFolderComplete(LLFolderViewFolder* folder) override { return true; }
 
     struct ViewModelCompare
     {
@@ -461,7 +462,7 @@ public:
         const SortType& mSorter;
     };
 
-    void sort(LLFolderViewFolder* folder)
+    void sort(LLFolderViewFolder* folder) override
     {
         if (needsSort(folder->getViewModelItem()))
         {
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);