MAINT-1979 Viewer crashes while attempting to join group in the moment of loading group members

* Fix one race condition that could dereference a dangling pointer.
reviewed with Simon and Baker.

6 files changed, 45 insertions, 38 deletions

diff --git a/indra/newview/llgroupmgr.cpp b/indra/newview/llgroupmgr.cpp
index 6916cf813a..81eb1d397e 100644
--- a/indra/newview/llgroupmgr.cpp
+++ b/indra/newview/llgroupmgr.cpp
@@ -238,6 +238,7 @@ LLGroupMgrGroupData::LLGroupMgrGroupData(const LLUUID& id) :
 	mPendingRoleMemberRequest(FALSE),
 	mAccessTime(0.0f)
 {
+	mMemberVersion.generate();
 }
 
 void LLGroupMgrGroupData::setAccessed()
@@ -318,14 +319,14 @@ void LLGroupMgrGroupData::setRoleData(const LLUUID& role_id, LLRoleData role_dat
 			role_data.mChangeType = RC_UPDATE_DATA;
 		}
 		else
-	{
+		{
 			role_data.mChangeType = RC_UPDATE_POWERS;
 		}
 
 		mRoleChanges[role_id] = role_data;
 	}
 	else
-		{
+	{
 		llwarns << "Change being made to non-existant role " << role_id << llendl;
 	}
 }
@@ -424,6 +425,7 @@ void LLGroupMgrGroupData::removeMemberData()
 	}
 	mMembers.clear();
 	mMemberDataComplete = FALSE;
+	mMemberVersion.generate();
 }
 
 void LLGroupMgrGroupData::removeRoleData()
@@ -945,6 +947,8 @@ void LLGroupMgr::processGroupMembersReply(LLMessageSystem* msg, void** data)
 		}
 	}
 
+	group_datap->mMemberVersion.generate();
+
 	if (group_datap->mMembers.size() ==  (U32)group_datap->mMemberCount)
 	{
 		group_datap->mMemberDataComplete = TRUE;
@@ -1771,8 +1775,6 @@ void LLGroupMgr::sendGroupMemberEjects(const LLUUID& group_id,
 	bool start_message = true;
 	LLMessageSystem* msg = gMessageSystem;
 
-	
-
 	LLGroupMgrGroupData* group_datap = LLGroupMgr::getInstance()->getGroupData(group_id);
 	if (!group_datap) return;
 
@@ -1833,6 +1835,8 @@ void LLGroupMgr::sendGroupMemberEjects(const LLUUID& group_id,
 	{
 		gAgent.sendReliableMessage();
 	}
+
+	group_datap->mMemberVersion.generate();
 }
 
 
@@ -1990,6 +1994,8 @@ void LLGroupMgr::processCapGroupMembersRequest(const LLSD& content)
 		group_datap->mMembers[member_id] = data;
 	}
 
+	group_datap->mMemberVersion.generate();
+
 	// Technically, we have this data, but to prevent completely overhauling
 	// this entire system (it would be nice, but I don't have the time), 
 	// I'm going to be dumb and just call services I most likely don't need 
diff --git a/indra/newview/llgroupmgr.h b/indra/newview/llgroupmgr.h
index 62b2978f21..d8c1ab7ef5 100644
--- a/indra/newview/llgroupmgr.h
+++ b/indra/newview/llgroupmgr.h
@@ -236,6 +236,8 @@ public:
 	F32 getAccessTime() const { return mAccessTime; }
 	void setAccessed();
 
+	const LLUUID& getMemberVersion() const { return mMemberVersion; }
+
 public:
 	typedef	std::map<LLUUID,LLGroupMemberData*> member_list_t;
 	typedef	std::map<LLUUID,LLGroupRoleData*> role_list_t;
@@ -284,6 +286,9 @@ private:
 
 	BOOL				mPendingRoleMemberRequest;
 	F32					mAccessTime;
+
+	// Generate a new ID every time mMembers
+	LLUUID				mMemberVersion;
 };
 
 struct LLRoleAction
diff --git a/indra/newview/llpanelgroupgeneral.cpp b/indra/newview/llpanelgroupgeneral.cpp
index 51b4d2ea65..993ffb7825 100644
--- a/indra/newview/llpanelgroupgeneral.cpp
+++ b/indra/newview/llpanelgroupgeneral.cpp
@@ -670,7 +670,6 @@ void LLPanelGroupGeneral::update(LLGroupChange gc)
 		{
 			mMemberProgress = gdatap->mMembers.begin();
 			mPendingMemberUpdate = TRUE;
-			mUdpateSessionID.generate();
 
 			sSDTime = 0.0f;
 			sElementTime = 0.0f;
@@ -730,7 +729,7 @@ void LLPanelGroupGeneral::updateMembers()
 			// If name is not cached, onNameCache() should be called when it is cached and add this member to list.
 			LLAvatarNameCache::get(mMemberProgress->first, 
 									boost::bind(&LLPanelGroupGeneral::onNameCache,
-												this, mUdpateSessionID, member, _1, _2));
+												this, gdatap->getMemberVersion(), member, _2));
 		}
 	}
 
@@ -768,11 +767,15 @@ void LLPanelGroupGeneral::addMember(LLGroupMemberData* member)
 	}
 }
 
-void LLPanelGroupGeneral::onNameCache(const LLUUID& update_id, LLGroupMemberData* member, const LLUUID& id, const LLAvatarName& av_name)
+void LLPanelGroupGeneral::onNameCache(const LLUUID& update_id, LLGroupMemberData* member, const LLAvatarName& av_name)
 {
-	if (!member 
-		|| update_id != mUdpateSessionID)
+	LLGroupMgrGroupData* gdatap = LLGroupMgr::getInstance()->getGroupData(mGroupID);
+
+	if (!gdatap
+		|| !gdatap->isMemberDataComplete()
+		|| gdatap->getMemberVersion() != update_id)
 	{
+		// Stale data
 		return;
 	}
 
diff --git a/indra/newview/llpanelgroupgeneral.h b/indra/newview/llpanelgroupgeneral.h
index b179f78c56..1b4e8e2645 100644
--- a/indra/newview/llpanelgroupgeneral.h
+++ b/indra/newview/llpanelgroupgeneral.h
@@ -63,7 +63,7 @@ public:
 
 	virtual void setupCtrls	(LLPanel* parent);
 
-	void onNameCache(const LLUUID& update_id, LLGroupMemberData* member, const LLUUID& id, const LLAvatarName& av_name);
+	void onNameCache(const LLUUID& update_id, LLGroupMemberData* member, const LLAvatarName& av_name);
 private:
 	void	reset();
 
@@ -90,7 +90,6 @@ private:
 	BOOL			mChanged;
 	BOOL			mFirstUse;
 	std::string		mIncompleteMemberDataStr;
-	LLUUID			mUdpateSessionID;
 
 	// Group information (include any updates in updateChanged)
 	LLLineEditor		*mGroupNameEditor;
diff --git a/indra/newview/llpanelgrouproles.cpp b/indra/newview/llpanelgrouproles.cpp
index 5720168f81..ff106882f4 100644
--- a/indra/newview/llpanelgrouproles.cpp
+++ b/indra/newview/llpanelgrouproles.cpp
@@ -745,7 +745,6 @@ LLPanelGroupMembersSubTab::LLPanelGroupMembersSubTab()
 	mHasMatch(FALSE),
 	mNumOwnerAdditions(0)
 {
-	mUdpateSessionID = LLUUID::null;
 }
 
 LLPanelGroupMembersSubTab::~LLPanelGroupMembersSubTab()
@@ -1427,13 +1426,20 @@ U64 LLPanelGroupMembersSubTab::getAgentPowersBasedOnRoleChanges(const LLUUID& ag
 		return GP_NO_POWERS;
 	}
 
-	LLGroupMemberData* member_data = gdatap->mMembers[agent_id];
-	if ( !member_data )
+	LLGroupMgrGroupData::member_list_t::iterator iter = gdatap->mMembers.find(agent_id);
+	if ( iter == gdatap->mMembers.end() )
 	{
 		llwarns << "LLPanelGroupMembersSubTab::getAgentPowersBasedOnRoleChanges() -- No member data for member with UUID " << agent_id << llendl;
 		return GP_NO_POWERS;
 	}
 
+	LLGroupMemberData* member_data = (*iter).second;
+	if (!member_data)
+	{
+		llwarns << "LLPanelGroupMembersSubTab::getAgentPowersBasedOnRoleChanges() -- Null member data for member with UUID " << agent_id << llendl;
+		return GP_NO_POWERS;
+	}
+
 	//see if there are unsaved role changes for this agent
 	role_change_data_map_t* role_change_datap = NULL;
 	member_role_changes_map_t::iterator member = mMemberRoleChangeData.find(agent_id);
@@ -1548,10 +1554,6 @@ void LLPanelGroupMembersSubTab::update(LLGroupChange gc)
 		mMemberProgress = gdatap->mMembers.begin();
 		mPendingMemberUpdate = TRUE;
 		mHasMatch = FALSE;
-		// Generate unique ID for current updateMembers()- see onNameCache for details.
-		// Using unique UUID is perhaps an overkill but this way we are perfectly safe
-		// from coincidences.
-		mUdpateSessionID.generate();
 	}
 	else
 	{
@@ -1579,14 +1581,14 @@ void LLPanelGroupMembersSubTab::update(LLGroupChange gc)
 	}
 }
 
-void LLPanelGroupMembersSubTab::addMemberToList(LLUUID id, LLGroupMemberData* data)
+void LLPanelGroupMembersSubTab::addMemberToList(LLGroupMemberData* data)
 {
 	if (!data) return;
 	LLUIString donated = getString("donation_area");
 	donated.setArg("[AREA]", llformat("%d", data->getContribution()));
 
 	LLNameListCtrl::NameItem item_params;
-	item_params.value = id;
+	item_params.value = data->getID();
 
 	item_params.columns.add().column("name").font.name("SANSSERIF_SMALL").style("NORMAL");
 
@@ -1600,17 +1602,12 @@ void LLPanelGroupMembersSubTab::addMemberToList(LLUUID id, LLGroupMemberData* da
 	mHasMatch = TRUE;
 }
 
-void LLPanelGroupMembersSubTab::onNameCache(const LLUUID& update_id, LLGroupMemberData* member, const LLUUID& id, const LLAvatarName& av_name)
+void LLPanelGroupMembersSubTab::onNameCache(const LLUUID& update_id, LLGroupMemberData* member, const LLAvatarName& av_name)
 {
-	// Update ID is used to determine whether member whose id is passed
-	// into onNameCache() was passed after current or previous user-initiated update.
-	// This is needed to avoid probable duplication of members in list after changing filter
-	// or adding of members of another group if gets for their names were called on
-	// previous update. If this id is from get() called from older update,
-	// we do nothing.
-	if (mUdpateSessionID != update_id) return;
-	
-	if (!member)
+	LLGroupMgrGroupData* gdatap = LLGroupMgr::getInstance()->getGroupData(mGroupID);
+	if (!gdatap
+		|| gdatap->getMemberVersion() != update_id
+		|| !member)
 	{
 		return;
 	}
@@ -1618,7 +1615,7 @@ void LLPanelGroupMembersSubTab::onNameCache(const LLUUID& update_id, LLGroupMemb
 	// trying to avoid unnecessary hash lookups
 	if (matchesSearchFilter(av_name.getLegacyName()))
 	{
-		addMemberToList(id, member);
+		addMemberToList(member);
 		if(!mMembersList->getEnabled())
 		{
 			mMembersList->setEnabled(TRUE);
@@ -1672,14 +1669,14 @@ void LLPanelGroupMembersSubTab::updateMembers()
 		{
 			if (matchesSearchFilter(av_name.getLegacyName()))
 			{
-				addMemberToList(mMemberProgress->first, mMemberProgress->second);
+				addMemberToList(mMemberProgress->second);
 			}
 		}
 		else
 		{
 			// If name is not cached, onNameCache() should be called when it is cached and add this member to list.
 			LLAvatarNameCache::get(mMemberProgress->first, boost::bind(&LLPanelGroupMembersSubTab::onNameCache,
-																	   this, mUdpateSessionID, mMemberProgress->second, _1, _2));
+									this, gdatap->getMemberVersion(), mMemberProgress->second, _2));
 		}
 	}
 
diff --git a/indra/newview/llpanelgrouproles.h b/indra/newview/llpanelgrouproles.h
index 8b454e020a..bead8bd85b 100644
--- a/indra/newview/llpanelgrouproles.h
+++ b/indra/newview/llpanelgrouproles.h
@@ -187,8 +187,8 @@ public:
 
 	virtual void setGroupID(const LLUUID& id);
 
-	void addMemberToList(LLUUID id, LLGroupMemberData* data);
-	void onNameCache(const LLUUID& update_id, LLGroupMemberData* member, const LLUUID& id, const LLAvatarName& av_name);
+	void addMemberToList(LLGroupMemberData* data);
+	void onNameCache(const LLUUID& update_id, LLGroupMemberData* member, const LLAvatarName& av_name);
 
 protected:
 	typedef std::map<LLUUID, LLRoleMemberChangeType> role_change_data_map_t;
@@ -210,9 +210,6 @@ protected:
 	BOOL mPendingMemberUpdate;
 	BOOL mHasMatch;
 
-	// This id is generated after each user initiated member list update(opening Roles or changing filter)
-	LLUUID mUdpateSessionID;
-
 	member_role_changes_map_t mMemberRoleChangeData;
 	U32 mNumOwnerAdditions;