From 18323f019f51a0ca27470ec770232fc4618fcd4b Mon Sep 17 00:00:00 2001
From: Andrey Kleshchev <andreykproductengine@lindenlab.com>
Date: Wed, 31 Mar 2021 21:45:35 +0300
Subject: [PATCH] SL-15061 Crash deleting non-zero reference in
LLConversationItemSession
---
indra/newview/llconversationmodel.cpp | 28 +++++++++++++++++----------
indra/newview/llconversationmodel.h | 2 +-
indra/newview/llconversationview.cpp | 9 ++++-----
3 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/indra/newview/llconversationmodel.cpp b/indra/newview/llconversationmodel.cpp
index fbdf08d8aaa..4cfde21e328 100644
--- a/indra/newview/llconversationmodel.cpp
+++ b/indra/newview/llconversationmodel.cpp
@@ -350,16 +350,24 @@ void LLConversationItemSession::clearParticipants()
}
-void LLConversationItemSession::deleteParticipantModels()
-{
- // Make sure that no views exist before use and that view-owned items were removed!
- //
- // Normally we are not supposed to delete models directly, they should be
- // owned by views and this action will result in crashes, but LLParticipantList
- // creates models separately from views (it probably shouldn't) and then those
- // models wait for idle cycles to be assigned to view.
- // this code is meant to delete 'waiting' models
- std::for_each(mChildren.begin(), mChildren.end(), DeletePointer());
+void LLConversationItemSession::clearAndDeparentModels()
+{
+ std::for_each(mChildren.begin(), mChildren.end(),
+ [](LLFolderViewModelItem* c)
+ {
+ if (c->getNumRefs() == 0)
+ {
+ // LLConversationItemParticipant can be created but not assigned to any view,
+ // it was waiting for an "add_participant" event to be processed
+ delete c;
+ }
+ else
+ {
+ // Model is still assigned to some view/widget
+ c->setParent(NULL);
+ }
+ }
+ );
mChildren.clear();
}
diff --git a/indra/newview/llconversationmodel.h b/indra/newview/llconversationmodel.h
index e30bfbb7591..787deeb5940 100644
--- a/indra/newview/llconversationmodel.h
+++ b/indra/newview/llconversationmodel.h
@@ -165,7 +165,7 @@ class LLConversationItemSession : public LLConversationItem
void removeParticipant(LLConversationItemParticipant* participant);
void removeParticipant(const LLUUID& participant_id);
void clearParticipants();
- void deleteParticipantModels(); // do not use while there are existing participant views
+ void clearAndDeparentModels(); // will delete unowned models and deparent owned ones
LLConversationItemParticipant* findParticipant(const LLUUID& participant_id);
void setParticipantIsMuted(const LLUUID& participant_id, bool is_muted);
diff --git a/indra/newview/llconversationview.cpp b/indra/newview/llconversationview.cpp
index 71346b4b43c..df16868132c 100644
--- a/indra/newview/llconversationview.cpp
+++ b/indra/newview/llconversationview.cpp
@@ -108,8 +108,7 @@ void LLConversationViewSession::destroyView()
// Chat can create and parent models(listeners) to session's model before creating
// coresponding views, such participant's models normally will wait for idle cycles
// but since we are deleting session and won't be processing any more events, make
- // sure unowned models are removed as well.
- // Might be good idea to just have an LLPointer list somewhere in LLConversationItemSession
+ // sure unowned LLConversationItemParticipant models are removed as well.
LLConversationItemSession* vmi = dynamic_cast<LLConversationItemSession*>(getViewModelItem());
@@ -146,9 +145,9 @@ void LLConversationViewSession::destroyView()
folderp->destroyView();
}
- // Now everything that is left in model(listener) is unowned,
- // it is safe to remove
- vmi->deleteParticipantModels();
+ // Now everything that is left in model(listener) is not owned by views,
+ // only by sessions, deparent so it won't point to soon to be dead model
+ vmi->clearAndDeparentModels();
}
LLFolderViewFolder::destroyView();
--
GitLab