summaryrefslogtreecommitdiff
path: root/indra/newview
diff options
context:
space:
mode:
authorNat Goodspeed <nat@lindenlab.com>2019-12-02 14:39:24 -0500
committerNat Goodspeed <nat@lindenlab.com>2020-03-25 15:28:17 -0400
commit9d5b897600a8f9405ec37a141b9417f34a11c557 (patch)
tree60ee2942d3061fce7a28efe25dfe63ddfbab4ce1 /indra/newview
parent70a0b52039a75c2c482224c9b4a6c133cac0ae76 (diff)
DRTVWR-494: Defend LLInstanceTracker against multi-thread usage.
The previous implementation went to some effort to crash if anyone attempted to create or destroy an LLInstanceTracker subclass instance during traversal. That restriction is manageable within a single thread, but becomes unworkable if it's possible that a given subclass might be used on more than one thread. Remove LLInstanceTracker::instance_iter, beginInstances(), endInstances(), also key_iter, beginKeys() and endKeys(). Instead, introduce key_snapshot() and instance_snapshot(), the only means of iterating over LLInstanceTracker instances. (These are intended to resemble functions, but in fact the current implementation simply presents the classes.) Iterating over a captured snapshot defends against container modifications during traversal. The term 'snapshot' reminds the coder that a new instance created during traversal will not be considered. To defend against instance deletion during traversal, a snapshot stores std::weak_ptrs which it lazily dereferences, skipping on the fly any that have expired. Dereferencing instance_snapshot::iterator gets you a reference rather than a pointer. Because some use cases want to delete all existing instances, add an instance_snapshot::deleteAll() method that extracts the pointer. Those cases used to require explicitly copying instance pointers into a separate container; instance_snapshot() now takes care of that. It remains the caller's responsibility to ensure that all instances of that LLInstanceTracker subclass were allocated on the heap. Replace unkeyed static LLInstanceTracker::getInstance(T*) -- which returned nullptr if that instance had been destroyed -- with new getWeak() method returning std::weak_ptr<T>. Caller must detect expiration of that weak_ptr. Adjust tests accordingly. Use of std::weak_ptr to detect expired instances requires engaging std::shared_ptr in the constructor. We now store shared_ptrs in the static containers (std::map for keyed, std::set for unkeyed). Make LLInstanceTrackerBase a template parameterized on the type of the static data it manages. For that reason, hoist static data class declarations out of the class definitions to an LLInstanceTrackerStuff namespace. Remove the static atomic sIterationNestDepth and its methods incrementDepth(), decrementDepth() and getDepth(), since they were used only to forbid creation and destruction during traversal. Add a std::mutex to static data. Introduce an internal LockStatic class that locks the mutex while providing a pointer to static data, making that the only way to access the static data. The LLINSTANCETRACKER_DTOR_NOEXCEPT macro goes away because we no longer expect ~LLInstanceTracker() to throw an exception in test programs. That affects LLTrace::StatBase as well as LLInstanceTracker itself. Adapt consumers to the new LLInstanceTracker API.
Diffstat (limited to 'indra/newview')
-rw-r--r--indra/newview/llappviewer.cpp26
-rw-r--r--indra/newview/llchathistory.cpp4
-rw-r--r--indra/newview/llimprocessing.cpp4
-rw-r--r--indra/newview/llscenemonitor.cpp44
-rw-r--r--indra/newview/lltoast.cpp23
-rw-r--r--indra/newview/llviewercontrollistener.cpp12
6 files changed, 31 insertions, 82 deletions
diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp
index b232a8c3bb..a76ac58724 100644
--- a/indra/newview/llappviewer.cpp
+++ b/indra/newview/llappviewer.cpp
@@ -1681,24 +1681,9 @@ bool LLAppViewer::cleanup()
gDirUtilp->deleteFilesInDir(logdir, "*-*-*-*-*.dmp");
}
- {
- // Kill off LLLeap objects. We can find them all because LLLeap is derived
- // from LLInstanceTracker. But collect instances first: LLInstanceTracker
- // specifically forbids adding/deleting instances while iterating.
- std::vector<LLLeap*> leaps;
- leaps.reserve(LLLeap::instanceCount());
- for (LLLeap::instance_iter li(LLLeap::beginInstances()), lend(LLLeap::endInstances());
- li != lend; ++li)
- {
- leaps.push_back(&*li);
- }
- // Okay, now trash them all. We don't have to NULL or erase the entry
- // in 'leaps' because the whole vector is going away momentarily.
- BOOST_FOREACH(LLLeap* leap, leaps)
- {
- delete leap;
- }
- } // destroy 'leaps'
+ // Kill off LLLeap objects. We can find them all because LLLeap is derived
+ // from LLInstanceTracker.
+ LLLeap::instance_snapshot().deleteAll();
//flag all elements as needing to be destroyed immediately
// to ensure shutdown order
@@ -2858,12 +2843,11 @@ bool LLAppViewer::initConfiguration()
// Let anyone else who cares know that we've populated our settings
// variables.
- for (LLControlGroup::key_iter ki(LLControlGroup::beginKeys()), kend(LLControlGroup::endKeys());
- ki != kend; ++ki)
+ for (const auto& key : LLControlGroup::key_snapshot())
{
// For each named instance of LLControlGroup, send an event saying
// we've initialized an LLControlGroup instance by that name.
- LLEventPumps::instance().obtain("LLControlGroup").post(LLSDMap("init", *ki));
+ LLEventPumps::instance().obtain("LLControlGroup").post(LLSDMap("init", key));
}
return true; // Config was successful.
diff --git a/indra/newview/llchathistory.cpp b/indra/newview/llchathistory.cpp
index 1099d4bc09..4131af828e 100644
--- a/indra/newview/llchathistory.cpp
+++ b/indra/newview/llchathistory.cpp
@@ -1344,10 +1344,8 @@ void LLChatHistory::appendMessage(const LLChat& chat, const LLSD &args, const LL
// We don't want multiple friendship offers to appear, this code checks if there are previous offers
// by iterating though all panels.
// Note: it might be better to simply add a "pending offer" flag somewhere
- for (LLToastNotifyPanel::instance_iter ti(LLToastNotifyPanel::beginInstances())
- , tend(LLToastNotifyPanel::endInstances()); ti != tend; ++ti)
+ for (auto& panel : LLToastNotifyPanel::instance_snapshot())
{
- LLToastNotifyPanel& panel = *ti;
LLIMToastNotifyPanel * imtoastp = dynamic_cast<LLIMToastNotifyPanel *>(&panel);
const std::string& notification_name = panel.getNotificationName();
if (notification_name == "OfferFriendship"
diff --git a/indra/newview/llimprocessing.cpp b/indra/newview/llimprocessing.cpp
index c3375a3779..c1dcc61010 100644
--- a/indra/newview/llimprocessing.cpp
+++ b/indra/newview/llimprocessing.cpp
@@ -1404,10 +1404,8 @@ void LLIMProcessing::processNewMessage(LLUUID from_id,
payload["sender"] = sender.getIPandPort();
bool add_notification = true;
- for (LLToastNotifyPanel::instance_iter ti(LLToastNotifyPanel::beginInstances())
- , tend(LLToastNotifyPanel::endInstances()); ti != tend; ++ti)
+ for (auto& panel : LLToastNotifyPanel::instance_snapshot())
{
- LLToastNotifyPanel& panel = *ti;
const std::string& notification_name = panel.getNotificationName();
if (notification_name == "OfferFriendship" && panel.isControlPanelEnabled())
{
diff --git a/indra/newview/llscenemonitor.cpp b/indra/newview/llscenemonitor.cpp
index 5ab0013055..2c0c38dc75 100644
--- a/indra/newview/llscenemonitor.cpp
+++ b/indra/newview/llscenemonitor.cpp
@@ -559,16 +559,14 @@ void LLSceneMonitor::dumpToFile(std::string file_name)
typedef StatType<CountAccumulator> trace_count;
- for (trace_count::instance_iter it = trace_count::beginInstances(), end_it = trace_count::endInstances();
- it != end_it;
- ++it)
+ for (auto& it : trace_count::instance_snapshot())
{
std::ostringstream row;
row << std::setprecision(10);
- row << it->getName();
+ row << it.getName();
- const char* unit_label = it->getUnitLabel();
+ const char* unit_label = it.getUnitLabel();
if(unit_label[0])
{
row << "(" << unit_label << ")";
@@ -579,8 +577,8 @@ void LLSceneMonitor::dumpToFile(std::string file_name)
for (S32 frame = 1; frame <= frame_count; frame++)
{
Recording& recording = scene_load_recording.getPrevRecording(frame_count - frame);
- samples += recording.getSampleCount(*it);
- row << ", " << recording.getSum(*it);
+ samples += recording.getSampleCount(it);
+ row << ", " << recording.getSum(it);
}
row << '\n';
@@ -593,15 +591,13 @@ void LLSceneMonitor::dumpToFile(std::string file_name)
typedef StatType<EventAccumulator> trace_event;
- for (trace_event::instance_iter it = trace_event::beginInstances(), end_it = trace_event::endInstances();
- it != end_it;
- ++it)
+ for (auto& it : trace_event::instance_snapshot())
{
std::ostringstream row;
row << std::setprecision(10);
- row << it->getName();
+ row << it.getName();
- const char* unit_label = it->getUnitLabel();
+ const char* unit_label = it.getUnitLabel();
if(unit_label[0])
{
row << "(" << unit_label << ")";
@@ -612,8 +608,8 @@ void LLSceneMonitor::dumpToFile(std::string file_name)
for (S32 frame = 1; frame <= frame_count; frame++)
{
Recording& recording = scene_load_recording.getPrevRecording(frame_count - frame);
- samples += recording.getSampleCount(*it);
- F64 mean = recording.getMean(*it);
+ samples += recording.getSampleCount(it);
+ F64 mean = recording.getMean(it);
if (llisnan(mean))
{
row << ", n/a";
@@ -634,15 +630,13 @@ void LLSceneMonitor::dumpToFile(std::string file_name)
typedef StatType<SampleAccumulator> trace_sample;
- for (trace_sample::instance_iter it = trace_sample::beginInstances(), end_it = trace_sample::endInstances();
- it != end_it;
- ++it)
+ for (auto& it : trace_sample::instance_snapshot())
{
std::ostringstream row;
row << std::setprecision(10);
- row << it->getName();
+ row << it.getName();
- const char* unit_label = it->getUnitLabel();
+ const char* unit_label = it.getUnitLabel();
if(unit_label[0])
{
row << "(" << unit_label << ")";
@@ -653,8 +647,8 @@ void LLSceneMonitor::dumpToFile(std::string file_name)
for (S32 frame = 1; frame <= frame_count; frame++)
{
Recording& recording = scene_load_recording.getPrevRecording(frame_count - frame);
- samples += recording.getSampleCount(*it);
- F64 mean = recording.getMean(*it);
+ samples += recording.getSampleCount(it);
+ F64 mean = recording.getMean(it);
if (llisnan(mean))
{
row << ", n/a";
@@ -674,15 +668,13 @@ void LLSceneMonitor::dumpToFile(std::string file_name)
}
typedef StatType<MemAccumulator> trace_mem;
- for (trace_mem::instance_iter it = trace_mem::beginInstances(), end_it = trace_mem::endInstances();
- it != end_it;
- ++it)
+ for (auto& it : trace_mem::instance_snapshot())
{
- os << it->getName() << "(KiB)";
+ os << it.getName() << "(KiB)";
for (S32 frame = 1; frame <= frame_count; frame++)
{
- os << ", " << scene_load_recording.getPrevRecording(frame_count - frame).getMax(*it).valueInUnits<LLUnits::Kilobytes>();
+ os << ", " << scene_load_recording.getPrevRecording(frame_count - frame).getMax(it).valueInUnits<LLUnits::Kilobytes>();
}
os << '\n';
diff --git a/indra/newview/lltoast.cpp b/indra/newview/lltoast.cpp
index 870e0d94f0..bf56a10d4d 100644
--- a/indra/newview/lltoast.cpp
+++ b/indra/newview/lltoast.cpp
@@ -612,11 +612,8 @@ S32 LLToast::notifyParent(const LLSD& info)
//static
void LLToast::updateClass()
{
- for (LLInstanceTracker<LLToast>::instance_iter iter = LLInstanceTracker<LLToast>::beginInstances();
- iter != LLInstanceTracker<LLToast>::endInstances(); )
+ for (auto& toast : LLInstanceTracker<LLToast>::instance_snapshot())
{
- LLToast& toast = *iter++;
-
toast.updateHoveredState();
}
}
@@ -624,22 +621,6 @@ void LLToast::updateClass()
// static
void LLToast::cleanupToasts()
{
- LLToast * toastp = NULL;
-
- while (LLInstanceTracker<LLToast>::instanceCount() > 0)
- {
- { // Need to scope iter to allow deletion
- LLInstanceTracker<LLToast>::instance_iter iter = LLInstanceTracker<LLToast>::beginInstances();
- toastp = &(*iter);
- }
-
- //LL_INFOS() << "Cleaning up toast id " << toastp->getNotificationID() << LL_ENDL;
-
- // LLToast destructor will remove it from the LLInstanceTracker.
- if (!toastp)
- break; // Don't get stuck in the loop if a null pointer somehow got on the list
-
- delete toastp;
- }
+ LLInstanceTracker<LLToast>::instance_snapshot().deleteAll();
}
diff --git a/indra/newview/llviewercontrollistener.cpp b/indra/newview/llviewercontrollistener.cpp
index d2484b2b23..3443bb644a 100644
--- a/indra/newview/llviewercontrollistener.cpp
+++ b/indra/newview/llviewercontrollistener.cpp
@@ -50,11 +50,9 @@ LLViewerControlListener::LLViewerControlListener()
std::ostringstream groupnames;
groupnames << "[\"group\"] is one of ";
const char* delim = "";
- for (LLControlGroup::key_iter cgki(LLControlGroup::beginKeys()),
- cgkend(LLControlGroup::endKeys());
- cgki != cgkend; ++cgki)
+ for (const auto& key : LLControlGroup::key_snapshot())
{
- groupnames << delim << '"' << *cgki << '"';
+ groupnames << delim << '"' << key << '"';
delim = ", ";
}
groupnames << '\n';
@@ -181,11 +179,9 @@ void LLViewerControlListener::groups(LLSD const & request)
{
// No Info, we're not looking up either a group or a control name.
Response response(LLSD(), request);
- for (LLControlGroup::key_iter cgki(LLControlGroup::beginKeys()),
- cgkend(LLControlGroup::endKeys());
- cgki != cgkend; ++cgki)
+ for (const auto& key : LLControlGroup::key_snapshot())
{
- response["groups"].append(*cgki);
+ response["groups"].append(key);
}
}