SL-17076 Thread safety of LLNotificationChannelBase's items

Due to odd crashes when cleaning mItems adding thread safety
Viewer runs window in a separate thread, it is possible notification
was added in a way that corrupted the list.

3 files changed, 30 insertions, 35 deletions

diff --git a/indra/llui/llnotifications.cpp b/indra/llui/llnotifications.cpp
index c0a7866926..d87241a9bf 100644
--- a/indra/llui/llnotifications.cpp
+++ b/indra/llui/llnotifications.cpp
@@ -994,6 +994,7 @@ LLBoundListener LLNotificationChannelBase::connectChangedImpl(const LLEventListe
 	// all of the notifications that are already in the channel
 	// we use a special signal called "load" in case the channel wants to care
 	// only about new notifications
+    LLMutexLock lock(&mItemsMutex);
 	for (LLNotificationSet::iterator it = mItems.begin(); it != mItems.end(); ++it)
 	{
 		slot(LLSD().with("sigtype", "load").with("id", (*it)->id()));
@@ -1186,22 +1187,18 @@ bool LLNotificationChannel::isEmpty() const
 
 S32 LLNotificationChannel::size() const
 {
-	return mItems.size();
+    return mItems.size();
 }
 
-LLNotificationChannel::Iterator LLNotificationChannel::begin()
+size_t LLNotificationChannel::size()
 {
-	return mItems.begin();
+	return mItems.size();
 }
 
-LLNotificationChannel::Iterator LLNotificationChannel::end()
+void LLNotificationChannel::forEachNotification(NotificationProcess process)
 {
-	return mItems.end();
-}
-
-size_t LLNotificationChannel::size()
-{
-	return mItems.size();
+    LLMutexLock lock(&mItemsMutex);
+    std::for_each(mItems.begin(), mItems.end(), process);
 }
 
 std::string LLNotificationChannel::summarize()
@@ -1209,7 +1206,8 @@ std::string LLNotificationChannel::summarize()
 	std::string s("Channel '");
 	s += mName;
 	s += "'\n  ";
-	for (LLNotificationChannel::Iterator it = begin(); it != end(); ++it)
+    LLMutexLock lock(&mItemsMutex);
+	for (LLNotificationChannel::Iterator it = mItems.begin(); it != mItems.end(); ++it)
 	{
 		s += (*it)->summarize();
 		s += "\n  ";
@@ -1736,6 +1734,7 @@ void LLNotifications::cancel(LLNotificationPtr pNotif)
 
 void LLNotifications::cancelByName(const std::string& name)
 {
+    LLMutexLock lock(&mItemsMutex);
 	std::vector<LLNotificationPtr> notifs_to_cancel;
 	for (LLNotificationSet::iterator it=mItems.begin(), end_it = mItems.end();
 		it != end_it;
@@ -1760,6 +1759,7 @@ void LLNotifications::cancelByName(const std::string& name)
 
 void LLNotifications::cancelByOwner(const LLUUID ownerId)
 {
+    LLMutexLock lock(&mItemsMutex);
 	std::vector<LLNotificationPtr> notifs_to_cancel;
 	for (LLNotificationSet::iterator it = mItems.begin(), end_it = mItems.end();
 		 it != end_it;
@@ -1807,11 +1807,6 @@ LLNotificationPtr LLNotifications::find(LLUUID uuid)
 	}
 }
 
-void LLNotifications::forEachNotification(NotificationProcess process)
-{
-	std::for_each(mItems.begin(), mItems.end(), process);
-}
-
 std::string LLNotifications::getGlobalString(const std::string& key) const
 {
 	GlobalStringMap::const_iterator it = mGlobalStrings.find(key);
diff --git a/indra/llui/llnotifications.h b/indra/llui/llnotifications.h
index 1577038c36..4d71d189f2 100644
--- a/indra/llui/llnotifications.h
+++ b/indra/llui/llnotifications.h
@@ -738,16 +738,19 @@ class LLNotificationChannelBase :
 {
 	LOG_CLASS(LLNotificationChannelBase);
 public:
-	LLNotificationChannelBase(LLNotificationFilter filter) 
-	:	mFilter(filter), 
-		mItems() 
-	{}
+    LLNotificationChannelBase(LLNotificationFilter filter) 
+    : mFilter(filter)
+    , mItems() 
+    , mItemsMutex()
+    {}
+
     virtual ~LLNotificationChannelBase()
     {
         // explicit cleanup for easier issue detection
         mChanged.disconnect_all_slots();
         mPassedFilter.disconnect_all_slots();
         mFailedFilter.disconnect_all_slots();
+        LLMutexLock lock(&mItemsMutex);
         mItems.clear();
     }
 	// you can also connect to a Channel, so you can be notified of
@@ -786,6 +789,7 @@ protected:
 	LLStandardSignal mChanged;
 	LLStandardSignal mPassedFilter;
 	LLStandardSignal mFailedFilter;
+    LLMutex mItemsMutex;
 	
 	// these are action methods that subclasses can override to take action 
 	// on specific types of changes; the management of the mItems list is
@@ -844,14 +848,14 @@ public:
 	{
 		return boost::iterator_range<parents_iter>(mParents);
 	}
-    
+
     bool isEmpty() const;
     S32 size() const;
-    
-    Iterator begin();
-    Iterator end();
-	size_t size();
-	
+    size_t size();
+
+    typedef boost::function<void(LLNotificationPtr)> NotificationProcess;
+    void forEachNotification(NotificationProcess process);
+
 	std::string summarize();
 
 protected:
@@ -926,10 +930,6 @@ public:
 	void update(const LLNotificationPtr pNotif);
 
 	LLNotificationPtr find(LLUUID uuid);
-	
-	typedef boost::function<void (LLNotificationPtr)> NotificationProcess;
-	
-	void forEachNotification(NotificationProcess process);
 
 	// This is all stuff for managing the templates
 	// take your template out
diff --git a/indra/llui/llnotificationslistener.cpp b/indra/llui/llnotificationslistener.cpp
index e73ba1fbe9..83b5bf03ac 100644
--- a/indra/llui/llnotificationslistener.cpp
+++ b/indra/llui/llnotificationslistener.cpp
@@ -149,11 +149,11 @@ void LLNotificationsListener::listChannelNotifications(const LLSD& params) const
     if (channel)
     {
         LLSD notifications(LLSD::emptyArray());
-        for (LLNotificationChannel::Iterator ni(channel->begin()), nend(channel->end());
-             ni != nend; ++ni)
-        {
-            notifications.append(asLLSD(*ni));
-        }
+        std::function<void(LLNotificationPtr)> func = [notifications](LLNotificationPtr ni) mutable
+            {
+                notifications.append(asLLSD(ni));
+            };
+        channel->forEachNotification(func);
         response["notifications"] = notifications;
     }
     LLEventPumps::instance().obtain(params["reply"]).post(response);