livie
/
phoenix-firestorm
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

DRTVWR-494: Move most LLSingleton cleanup back to destructor 

instead of deleteSingleton().

Specifically, clear static SingletonData and remove the instance from the
MasterList in the destructor.

Empirically, some consumers are manually deleting LLSingleton instances,
instead of calling deleteSingleton(). If deleteSingleton() handles cleanup
rather than the destructor, we're left with dangling pointers in the Master
List.

We don't also call cleanupSingleton() from the destructor because only
deleteSingleton() promises to call cleanupSingleton(). Hopefully whoever is
directly deleting an LLSingleton subclass instance isn't relying on
cleanupSingleton().
meow-7.2.2
Nat Goodspeed 2019-12-17 15:42:34 -05:00
parent 24499ac66f
commit b2d0fb4160
2 changed files with 26 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
indra/llcommon/llsingleton.cpp
View File

@ -384,7 +384,7 @@ LLSingletonBase::vec_t LLSingletonBase::dep_sort()
SingletonDeps sdeps;
// Lock while traversing the master list
MasterList::LockedMaster master;
BOOST_FOREACH(LLSingletonBase* sp, master.get())
for (LLSingletonBase* sp : master.get())
{
// Build the SingletonDeps structure by adding, for each
// LLSingletonBase* sp in the master list, sp itself. It has no
@ -401,14 +401,14 @@ LLSingletonBase::vec_t LLSingletonBase::dep_sort()
// extracts just the first (key) element from each sorted_iterator, then
// uses vec_t's range constructor... but frankly this is more
// straightforward, as long as we remember the above reserve() call!
BOOST_FOREACH(SingletonDeps::sorted_iterator::value_type pair, sdeps.sort())
for (const SingletonDeps::sorted_iterator::value_type& pair : sdeps.sort())
{
ret.push_back(pair.first);
}
// The master list is not itself pushed onto the master list. Add it as
// the very last entry -- it is the LLSingleton on which ALL others
// depend! -- so our caller will process it.
ret.push_back(MasterList::getInstance());
ret.push_back(&master.Lock::get());
return ret;
}

44
indra/llcommon/llsingleton.h
View File

@ -428,6 +428,21 @@ protected:
LLSingleton_manage_master<DERIVED_TYPE>().add(this);
}
protected:
virtual ~LLSingleton()
{
// This phase of cleanup is performed in the destructor rather than in
// deleteSingleton() to defend against manual deletion. When we moved
// cleanup to deleteSingleton(), we hit crashes due to dangling
// pointers in the MasterList.
LockStatic lk;
lk->mInstance = nullptr;
lk->mInitState = DELETED;
// Remove this instance from the master list.
LLSingleton_manage_master<DERIVED_TYPE>().remove(this);
}
public:
/**
* @brief Immediately delete the singleton.
@ -452,29 +467,16 @@ public:
*/
static void deleteSingleton()
{
DERIVED_TYPE* lameduck;
{
LockStatic lk;
// Capture the instance and clear SingletonData. This sequence
// guards against the chance that the destructor throws, somebody
// catches it and there's a subsequent call to getInstance().
lameduck = lk->mInstance;
lk->mInstance = nullptr;
lk->mInitState = DELETED;
// At this point we can safely unlock SingletonData during the
// remaining cleanup. If another thread calls deleteSingleton() (or
// getInstance(), or whatever) it won't find our instance, now
// referenced only as 'lameduck'.
}
// Hold the lock while we call cleanupSingleton() and the destructor.
// Our destructor also instantiates LockStatic, requiring a recursive
// mutex.
LockStatic lk;
// of course, only cleanup and delete if there's something there
if (lameduck)
if (lk->mInstance)
{
// remove this instance from the master list BEFORE attempting
// cleanup so possible destructor exception won't leave the master
// list confused
LLSingleton_manage_master<DERIVED_TYPE>().remove(lameduck);
lameduck->cleanup_();
delete lameduck;
lk->mInstance->cleanup_();
delete lk->mInstance;
// destructor clears mInstance (and mInitState)
}
}