Don't crash during app shutdown.

We have an object with a well-defined lifetime (QLoggingPrivate)
but we also have a bunch of global static objects. The differing
lifetime causes problems during app shutdown that lead to crashes.

We can simplify this by just putting everything into QLoggingPrivate.
When this object no longer exists, we know the app is shutting down
and can act appropriately (eg. don't try to check the environment,
don't try to register or unregister category objects).

The default category object remains as a global static so that calls
to qDebug() can still work while the app is shutting down.

Change-Id: I934c1d92e7666368fd2c8a35d79835953afc2f69
Reviewed-by: Andrew Stanley-Jones <andrew.stanley-jones@nokia.com>
Reviewed-by: Lorn Potter <lorn.potter@nokia.com>
Reviewed-by: Zsolt Simon <zsolt.simon@nokia.com>

2 files changed, 97 insertions, 102 deletions

diff --git a/src/logger/qlogger.cpp b/src/logger/qlogger.cpp
index d20173d..42fb45d 100644
--- a/src/logger/qlogger.cpp
+++ b/src/logger/qlogger.cpp
@@ -106,37 +106,18 @@ QT_LOGGER_BEGIN_NAMESPACE
 
 Q_GLOBAL_STATIC(QLoggingPrivate, qLogging)
 
-// Set to true after QLoggingPrivate has destructed
-//declare static members
-static bool privateUnloaded = false;
-static QMutex _mutexUnloadedCategory;
-static QMutex _mutexPrivateCategoryObjects;
-static QMap<QString, QLoggingCategoryPrivate *> _privateCategoryObjects;
-static QLoggingCategoryDefault default_QLoggingCategory;
-
-// Set to true if a valid config file or set of rules has been specified
-static bool gEnabled = false;
-
-static enum {
-    EnvironmentNotChecked = -1, // We have not checked QT_LOGGING_CONF
-    EnvironmentNotFound = 0,    // QT_LOGGING_CONF does not indicate a valid file
-    EnvironmentFound = 1,       // QT_LOGGING_CONF is a valid file
-} gEnvironment = EnvironmentNotChecked;
-
-// Returns either the string to the config file identified by QT_LOGGING_CONFIG or an empty string
-static void checkEnvironment()
+// This indirectly calls qLogging()
+// This object is returned from an exported API so it lives longer than QLoggingPrivate
+static class QLoggingCategoryDefault : public QLoggingCategory
 {
-    gEnvironment = EnvironmentNotFound;
-    QByteArray ba = qgetenv("QT_LOGGING_CONFIG");
-    if (!ba.isEmpty()) {
-        QString path = QString::fromLocal8Bit(ba);
-        QString config = QLoggingPrivate::resolveConfigFile(path);
-        if (!config.isEmpty()) {
-            gEnvironment = EnvironmentFound;
-            qLogging()->setLoggingRulesFile(config);
-        }
+public:
+    QLoggingCategoryDefault()
+        : QLoggingCategory("default")
+    {
+        //Note d_func creates private object and assign it to the d_ptr
+        d_func()->_enabledDebug = true;
     }
-}
+} default_QLoggingCategory;
 
 /*!
     \class QLoggingCategory
@@ -179,21 +160,21 @@ QLoggingCategory& QLoggingCategory::defaultCategory()
 */
 QLoggingCategory::~QLoggingCategory()
 {
+    QLoggingPrivate *qlp = qLogging();
+    if (!qlp) return; // logging system is gone
     if (d_ptr) {
-        QMutexLocker locker1(&_mutexPrivateCategoryObjects);
-        QMutexLocker locker(&_mutexUnloadedCategory);
+        QMutexLocker locker1(&qlp->_privateCategoryObjectsMutex);
         //Don't use QExplicitlySharedDataPointer for only reference counting
-        //We have to lock the _privateCategoryObjects map anyways and therfore we lock the reference counting as well.
+        //We have to lock the qlp->_privateCategoryObjects map anyways and therfore we lock the reference counting as well.
         d_ptr->_references--;
         if (d_ptr->_references == 0) {
-            if (d_ptr->_registered && !privateUnloaded) {
-                qLogging()->unregisterCategory(*this);
-            }
+            if (d_ptr->_registered)
+                qlp->unregisterCategory(*this);
             if (d_ptr->_references == 0) {
                 QString strcategory = QString::fromLatin1(d_ptr->_categoryName);
-                QMap<QString, QLoggingCategoryPrivate* >::iterator it = _privateCategoryObjects.find(strcategory);
-                if (it != _privateCategoryObjects.end())
-                    _privateCategoryObjects.remove(strcategory);
+                QMap<QString, QLoggingCategoryPrivate* >::iterator it = qlp->_privateCategoryObjects.find(strcategory);
+                if (it != qlp->_privateCategoryObjects.end())
+                    qlp->_privateCategoryObjects.remove(strcategory);
 
                 delete d_ptr;
             }
@@ -202,16 +183,6 @@ QLoggingCategory::~QLoggingCategory()
 }
 
 /*!
-     \internal Default category class to create and initialisate a default category object
-*/
-QLoggingCategoryDefault::QLoggingCategoryDefault()
-    : QLoggingCategory("default")
-{
-    //Note d_func creates private object and assign it to the d_ptr
-    d_func()->_enabledDebug = true;
-}
-
-/*!
     Returns true if a message of type \a msgtype will be printed. Returns false otherwise.
 
     This function may be useful to avoid doing expensive work to generate data that is only used for debug output.
@@ -236,36 +207,25 @@ QLoggingCategoryDefault::QLoggingCategoryDefault()
 */
 bool QLoggingCategory::isEnabled(QtMsgType msgtype)
 {
-    //Check if the contains of the environment variable QT_LOGGING_CONFIG was checked
-    //If not checkEnvironment will modify the gEnabled static member.
-    if (gEnvironment == EnvironmentNotChecked)
-        checkEnvironment();
-
-    /* Note that default category (qDebug & co) differs from the category objects
-       regarding to their message types initialisation values
-
-            Message Type           | default categor | category Object
-     =========================================================================
-            QtDebugMsg             |    true         |      false
-            QtWarningMsg           |    true         |      true
-            QtCriticalMsg          |    true         |      true
-    */
-    if (!gEnabled) {
-        //Category logging framework is disable so return the default values
-        //Note: Default category already exist so d_ptr is valid for default category
-        if (d_ptr)
-            return d_ptr->statusMessageType(msgtype);
-        //We do this because we want to avoid to create a QLoggingCategoryPrivate object
-        //and doing expensive operations if categroy logging is not active.
-        switch (msgtype) {
-            case QtDebugMsg: return false;
-            case QtWarningMsg: return true;
-            case QtCriticalMsg: return true;
-            default:
-                return false;
-        }
+    QLoggingPrivate *qlp = qLogging();
+
+    // If the logging system is available, we might need to register our category object.
+    if (qlp && qlp->registerCategories())
+        return qLogging()->isEnabled(*this, msgtype);
+
+    // If we're the default category or we were previously registered, we'll have cached values.
+    if (d_ptr)
+        return d_ptr->statusMessageType(msgtype);
+
+    // We don't have cached values. Use the defaults.
+    // NOTE qDebug/qWarning/qCritical never hit this (see default_QLoggingCategory)
+    switch (msgtype) {
+        case QtDebugMsg: return false;
+        case QtWarningMsg: return true;
+        case QtCriticalMsg: return true;
+        default: break;
     }
-    return qLogging()->isEnabled(*this, msgtype);
+    return false;
 }
 
 /*!
@@ -274,7 +234,9 @@ bool QLoggingCategory::isEnabled(QtMsgType msgtype)
 QLoggingCategoryPrivate * QLoggingCategory::d_func()
 {
     if (!d_ptr) {
-        QMutexLocker locker(&_mutexPrivateCategoryObjects);
+        QLoggingPrivate *qlp = qLogging();
+        if (!qlp) qFatal("QLoggingCategory::d_func() cannot continue because qLogging() is 0");
+        QMutexLocker locker(&qlp->_privateCategoryObjectsMutex);
         //Another thread can call this function for the same QLoggingCategory object now
         //Check the d_ptr after mutex lock again.
         if (!d_ptr) {
@@ -282,22 +244,21 @@ QLoggingCategoryPrivate * QLoggingCategory::d_func()
             //just for the insane case someone calls this constructor with an empty category parameter
             if (_categoryName)
                 strcategory = QString::fromLatin1(_categoryName);
-            QMap<QString, QLoggingCategoryPrivate* >::iterator it = _privateCategoryObjects.find(strcategory);
-            if (it != _privateCategoryObjects.end())
+            QMap<QString, QLoggingCategoryPrivate* >::iterator it = qlp->_privateCategoryObjects.find(strcategory);
+            if (it != qlp->_privateCategoryObjects.end())
                 d_ptr = *it;
             else {
                 d_ptr = new QLoggingCategoryPrivate(_categoryName);
-                _privateCategoryObjects.insert(strcategory, d_ptr);
+                qlp->_privateCategoryObjects.insert(strcategory, d_ptr);
             }
             //Don't use QExplicitlySharedDataPointer for only reference counting
-            //We have to lock the _privateCategoryObjects map anyways and therfore we lock the reference counting as well.
+            //We have to lock the qlp->_privateCategoryObjects map anyways and therfore we lock the reference counting as well.
             d_ptr->_references++;
         }
     }
     return d_ptr;
 }
 
-
 /*!
     \relates QLoggingCategory
     Load logging rules from \a path.
@@ -309,11 +270,12 @@ QLoggingCategoryPrivate * QLoggingCategory::d_func()
 */
 void qSetLoggingRulesFile(const QString &path)
 {
-    if (gEnvironment == EnvironmentNotChecked) checkEnvironment();
-    if (gEnvironment == EnvironmentFound) return;
+    QLoggingPrivate *qlp = qLogging();
+    if (!qlp) return; // logging system is gone
+    if (qlp->checkEnvironment()) return; // can't override the environment variable
     QString config = QLoggingPrivate::resolveConfigFile(path);
     if (!config.isEmpty())
-        qLogging()->setLoggingRulesFile(config);
+        qlp->setLoggingRulesFile(config);
 }
 
 /*!
@@ -328,10 +290,11 @@ void qSetLoggingRulesFile(const QString &path)
 */
 void qSetLoggingRules(const QByteArray &rules)
 {
-    if (gEnvironment == EnvironmentNotChecked) checkEnvironment();
-    if (gEnvironment == EnvironmentFound) return;
+    QLoggingPrivate *qlp = qLogging();
+    if (!qlp) return; // logging system is gone
+    if (qlp->checkEnvironment()) return; // can't override the environment variable
     if (!rules.isEmpty())
-        qLogging()->setLoggingRules(rules);
+        qlp->setLoggingRules(rules);
 }
 
 /*!
@@ -392,7 +355,10 @@ void qSetLoggingRules(const QByteArray &rules)
  */
 QLoggingPrivate *qtLoggerInstance()
 {
-    return qLogging();
+    // If we're really unlucky, this will be null (during shutdown of the app)
+    QLoggingPrivate *qlp = qLogging();
+    if (!qlp) qFatal("Cannot call qtLoggerInstance() because qLogging() is 0");
+    return qlp;
 }
 
 /*!
@@ -402,9 +368,12 @@ QLoggingPrivate *qtLoggerInstance()
 QLoggingPrivate::QLoggingPrivate()
     : QObject(0)
     , _configFileWatcher(0)
+    , _environment(EnvironmentNotChecked)
+    , _registerCategories(false)
 {
     //Move object to the application thread
-    this->moveToThread(QCoreApplication::instance()->thread());
+    if (QCoreApplication::instance())
+        this->moveToThread(QCoreApplication::instance()->thread());
 
     //add default category
     _registeredCategories.append(&QLoggingCategory::defaultCategory());
@@ -416,8 +385,6 @@ QLoggingPrivate::QLoggingPrivate()
  */
 QLoggingPrivate::~QLoggingPrivate()
 {
-    QMutexLocker locker(&_mutexUnloadedCategory);
-    privateUnloaded = true;
 }
 
 /*!
@@ -443,7 +410,7 @@ QString QLoggingPrivate::resolveConfigFile(const QString &path)
 */
 void QLoggingPrivate::setLoggingRulesFile(const QString &path)
 {
-    gEnabled = false;
+    _registerCategories = false;
     _configFile = path;
 
     //Create filewatcher only if a config file exists
@@ -480,7 +447,7 @@ void QLoggingPrivate::createFileWatcher()
 */
 void QLoggingPrivate::setLoggingRules(const QByteArray &rules)
 {
-    gEnabled = false;
+    _registerCategories = false;
 
     //Disable file watcher
     if (_configFileWatcher) {
@@ -536,7 +503,7 @@ void QLoggingPrivate::readSettings(QIODevice &device)
             updateCategory(category);
         }
 
-        gEnabled = true;
+        _registerCategories = true;
     }
     emit configurationChanged();
 }
@@ -611,6 +578,29 @@ void QLoggingPrivate::unregisterCategory(QLoggingCategory &category)
 }
 
 /*!
+    \internal
+    Returns true if the environment variable is found.
+    The first time this is called, the logging rules file pointed to by the
+    environment variable will be processed.
+*/
+bool QLoggingPrivate::checkEnvironment()
+{
+    if (_environment == EnvironmentNotChecked) {
+        _environment = EnvironmentNotFound;
+        QByteArray ba = qgetenv("QT_LOGGING_CONFIG");
+        if (!ba.isEmpty()) {
+            QString path = QString::fromLocal8Bit(ba);
+            QString config = QLoggingPrivate::resolveConfigFile(path);
+            if (!config.isEmpty()) {
+                _environment = EnvironmentFound;
+                setLoggingRulesFile(config);
+            }
+        }
+    }
+    return (_environment == EnvironmentFound);
+}
+
+/*!
     \internal Constructor of the private QLoggingCategory object
 */
 QLoggingCategoryPrivate::QLoggingCategoryPrivate(const char *categoryname)
diff --git a/src/logger/qlogger_p.h b/src/logger/qlogger_p.h
index ef75a3c..ef74a1d 100644
--- a/src/logger/qlogger_p.h
+++ b/src/logger/qlogger_p.h
@@ -81,6 +81,13 @@ public:
     void setLoggingRules(const QByteArray &configcontent);
     bool isEnabled(QLoggingCategory &category, QtMsgType type);
     void unregisterCategory(QLoggingCategory &category);
+    enum EnvironmentFlag {
+        EnvironmentNotChecked = -1, // We have not checked QT_LOGGING_CONF
+        EnvironmentNotFound = 0,    // QT_LOGGING_CONF does not indicate a valid file
+        EnvironmentFound = 1       // QT_LOGGING_CONF is a valid file
+    };
+    bool checkEnvironment();
+    bool registerCategories() { return _registerCategories; }
 
     Q_INVOKABLE void createFileWatcher();
 
@@ -99,6 +106,10 @@ public:
     QString _configFile;
     QMutex _mutexRegisteredCategory;
     QList<QLogConfigFilterItem> _logConfigItemList;
+    EnvironmentFlag _environment;
+    bool _registerCategories;
+    QMutex _privateCategoryObjectsMutex;
+    QMap<QString, QLoggingCategoryPrivate *> _privateCategoryObjects;
 };
 
 class QLoggingCategoryPrivate
@@ -115,12 +126,6 @@ public:
     int _references;
 };
 
-class QLoggingCategoryDefault : public QLoggingCategory
-{
-public:
-    QLoggingCategoryDefault();
-};
-
 Q_LOGGER_EXPORT QLoggingPrivate *qtLoggerInstance();