Skip to content
Snippets Groups Projects
Commit d415e019 authored by Nat Goodspeed's avatar Nat Goodspeed
Browse files

DRTVWR-418: Remove final shutdown cleanup as a cause of crashes.

The recent LLSingleton work added a hook that would run during the C++
runtime's final destruction of static objects. When the LAST LLSingleton in
any module was destroyed, its destructor would call
LLSingletonBase::deleteAll(). That mechanism was intended to permit an
application consuming LLSingletons to skip making an explicit deleteAll()
call, knowing that all instantiated LLSingleton instances would eventually be
cleaned up anyway.

However -- experience proves that kicking off deleteAll() processing during
the C++ runtime's final cleanup is too late. Too much has already been
destroyed. That call tends to cause more shutdown crashes than it resolves.

This commit deletes that whole mechanism. Going forward, if you want to clean
up LLSingleton instances, you must explicitly call
LLSingletonBase::deleteAll() during the application lifetime. If you don't,
LLSingleton instances will simply be leaked -- which might be okay,
considering the application is terminating anyway.
parent 2a5c47eb
No related branches found
No related tags found
No related merge requests found
...@@ -369,62 +369,20 @@ void LLSingletonBase::deleteAll() ...@@ -369,62 +369,20 @@ void LLSingletonBase::deleteAll()
} }
} }
/*------------------------ Final cleanup management ------------------------*/
class LLSingletonBase::MasterRefcount
{
public:
// store a POD int so it will be statically initialized to 0
int refcount;
};
static LLSingletonBase::MasterRefcount sMasterRefcount;
LLSingletonBase::ref_ptr_t LLSingletonBase::get_master_refcount()
{
// Calling this method constructs a new ref_ptr_t, which implicitly calls
// intrusive_ptr_add_ref(MasterRefcount*).
return &sMasterRefcount;
}
void intrusive_ptr_add_ref(LLSingletonBase::MasterRefcount* mrc)
{
// Count outstanding SingletonLifetimeManager instances.
++mrc->refcount;
}
void intrusive_ptr_release(LLSingletonBase::MasterRefcount* mrc)
{
// Notice when each SingletonLifetimeManager instance is destroyed.
if (! --mrc->refcount)
{
// The last instance was destroyed. Time to kill any remaining
// LLSingletons -- but in dependency order.
LLSingletonBase::deleteAll();
}
}
/*---------------------------- Logging helpers -----------------------------*/ /*---------------------------- Logging helpers -----------------------------*/
namespace { namespace {
bool oktolog() bool oktolog()
{ {
// See comments in log() below. // See comments in log() below.
return sMasterRefcount.refcount && LLError::is_available(); return LLError::is_available();
} }
void log(LLError::ELevel level, void log(LLError::ELevel level,
const char* p1, const char* p2, const char* p3, const char* p4) const char* p1, const char* p2, const char* p3, const char* p4)
{ {
// Check whether we're in the implicit final LLSingletonBase::deleteAll()
// call. We've carefully arranged for deleteAll() to be called when the
// last SingletonLifetimeManager instance is destroyed -- in other words,
// when the last translation unit containing an LLSingleton instance
// cleans up static data. That could happen after std::cerr is destroyed!
// The is_available() test below ensures that we'll stop logging once // The is_available() test below ensures that we'll stop logging once
// LLError has been cleaned up. If we had a similar portable test for // LLError has been cleaned up. If we had a similar portable test for
// std::cerr, this would be a good place to use it. As we do not, just // std::cerr, this would be a good place to use it.
// don't log anything during implicit final deleteAll(). Detect that by
// the master refcount having gone to zero.
if (sMasterRefcount.refcount == 0)
return;
// Check LLError::is_available() because some of LLError's infrastructure // Check LLError::is_available() because some of LLError's infrastructure
// is itself an LLSingleton. If that LLSingleton has not yet been // is itself an LLSingleton. If that LLSingleton has not yet been
......
...@@ -27,7 +27,6 @@ ...@@ -27,7 +27,6 @@
#include <boost/noncopyable.hpp> #include <boost/noncopyable.hpp>
#include <boost/unordered_set.hpp> #include <boost/unordered_set.hpp>
#include <boost/intrusive_ptr.hpp>
#include <list> #include <list>
#include <vector> #include <vector>
#include <typeinfo> #include <typeinfo>
...@@ -36,8 +35,6 @@ class LLSingletonBase: private boost::noncopyable ...@@ -36,8 +35,6 @@ class LLSingletonBase: private boost::noncopyable
{ {
public: public:
class MasterList; class MasterList;
class MasterRefcount;
typedef boost::intrusive_ptr<MasterRefcount> ref_ptr_t;
private: private:
// All existing LLSingleton instances are tracked in this master list. // All existing LLSingleton instances are tracked in this master list.
...@@ -119,9 +116,6 @@ class LLSingletonBase: private boost::noncopyable ...@@ -119,9 +116,6 @@ class LLSingletonBase: private boost::noncopyable
const char* p3="", const char* p4=""); const char* p3="", const char* p4="");
static std::string demangle(const char* mangled); static std::string demangle(const char* mangled);
// obtain canonical ref_ptr_t
static ref_ptr_t get_master_refcount();
// Default methods in case subclass doesn't declare them. // Default methods in case subclass doesn't declare them.
virtual void initSingleton() {} virtual void initSingleton() {}
virtual void cleanupSingleton() {} virtual void cleanupSingleton() {}
...@@ -175,10 +169,6 @@ class LLSingletonBase: private boost::noncopyable ...@@ -175,10 +169,6 @@ class LLSingletonBase: private boost::noncopyable
static void deleteAll(); static void deleteAll();
}; };
// support ref_ptr_t
void intrusive_ptr_add_ref(LLSingletonBase::MasterRefcount*);
void intrusive_ptr_release(LLSingletonBase::MasterRefcount*);
// Most of the time, we want LLSingleton_manage_master() to forward its // Most of the time, we want LLSingleton_manage_master() to forward its
// methods to real LLSingletonBase methods. // methods to real LLSingletonBase methods.
template <class T> template <class T>
...@@ -298,8 +288,7 @@ class LLSingleton : public LLSingletonBase ...@@ -298,8 +288,7 @@ class LLSingleton : public LLSingletonBase
// stores pointer to singleton instance // stores pointer to singleton instance
struct SingletonLifetimeManager struct SingletonLifetimeManager
{ {
SingletonLifetimeManager(): SingletonLifetimeManager()
mMasterRefcount(LLSingletonBase::get_master_refcount())
{ {
construct(); construct();
} }
...@@ -317,17 +306,14 @@ class LLSingleton : public LLSingletonBase ...@@ -317,17 +306,14 @@ class LLSingleton : public LLSingletonBase
// of static-object destruction, mean that we DO NOT WANT this // of static-object destruction, mean that we DO NOT WANT this
// destructor to delete this LLSingleton. This destructor will run // destructor to delete this LLSingleton. This destructor will run
// without regard to any other LLSingleton whose cleanup might // without regard to any other LLSingleton whose cleanup might
// depend on its existence. What we really want is to count the // depend on its existence. If you want to clean up LLSingletons,
// runtime's attempts to cleanup LLSingleton static data -- and on // call LLSingletonBase::deleteAll() sometime before static-object
// the very last one, call LLSingletonBase::deleteAll(). That // destruction begins. That method will properly honor cross-
// method will properly honor cross-LLSingleton dependencies. This // LLSingleton dependencies. Otherwise we simply leak LLSingleton
// is why we store an intrusive_ptr to a MasterRefcount: our // instances at shutdown. Since the whole process is terminating
// ref_ptr_t member counts SingletonLifetimeManager instances. // anyway, that's not necessarily a bad thing; it depends on what
// Once the runtime destroys the last of these, THEN we can delete // resources your LLSingleton instances are managing.
// every remaining LLSingleton.
} }
LLSingletonBase::ref_ptr_t mMasterRefcount;
}; };
protected: protected:
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment