#4184 clearAndDeparentModels crash

To resolve dual ownership switched children to LLPointer
meow-7.2.2
Andrey Kleshchev 2025-06-26 23:31:21 +03:00 committed by Andrey Kleshchev
parent 2d0f9e4737
commit 4bc20a8e53
6 changed files with 54 additions and 96 deletions

View File

@ -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()))
{

View File

@ -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();

View File

@ -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());

View File

@ -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++;

View File

@ -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());

View File

@ -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);