Delay initialization of the logging system.

Apparently I didn't run the unit tests at some point because they were
failing.  The ultimate cause was the default category object, which is
constructed before main(). It was causing the logging system to be
initialized early and the unit test's qputenv() was being ignored.

However, we want the logging system to delay initialization until
someone explicitly calls a logging-related function.

The solution is to delay setting the _enabledDebug flag on the default
category object until the logging system is initialized.

However, for this to work, the d_func() method had to be moved into
QLoggingPrivate to avoid a recursive call to qLogging(). Since d_func()
and the code in ~QLoggingCategory() deals primarily with structures
owned by QLoggingPrivate anyway, this works well.

There was also a code path where the environment checking code wasn't being
called. Rather than inserting more calls to checkEnvironment(), I made that
a simple getter and now check the environment from the QLoggingPrivate
constructor.

After all this, the unit tests pass again :)

Change-Id: I97fdfa7e7dd92ea620618a426fe309a535d917bc
Reviewed-by: Zsolt Simon <zsolt.simon@nokia.com>

3 files changed, 93 insertions, 96 deletions

diff --git a/src/logger/qlogger.cpp b/src/logger/qlogger.cpp
index 42fb45d..604b063 100644
--- a/src/logger/qlogger.cpp
+++ b/src/logger/qlogger.cpp
@@ -106,7 +106,6 @@ QT_LOGGER_BEGIN_NAMESPACE
 
 Q_GLOBAL_STATIC(QLoggingPrivate, qLogging)
 
-// This indirectly calls qLogging()
 // This object is returned from an exported API so it lives longer than QLoggingPrivate
 static class QLoggingCategoryDefault : public QLoggingCategory
 {
@@ -114,8 +113,6 @@ public:
     QLoggingCategoryDefault()
         : QLoggingCategory("default")
     {
-        //Note d_func creates private object and assign it to the d_ptr
-        d_func()->_enabledDebug = true;
     }
 } default_QLoggingCategory;
 
@@ -160,26 +157,10 @@ QLoggingCategory& QLoggingCategory::defaultCategory()
 */
 QLoggingCategory::~QLoggingCategory()
 {
+    if (!d_ptr) return;
     QLoggingPrivate *qlp = qLogging();
     if (!qlp) return; // logging system is gone
-    if (d_ptr) {
-        QMutexLocker locker1(&qlp->_privateCategoryObjectsMutex);
-        //Don't use QExplicitlySharedDataPointer for only reference counting
-        //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)
-                qlp->unregisterCategory(*this);
-            if (d_ptr->_references == 0) {
-                QString strcategory = QString::fromLatin1(d_ptr->_categoryName);
-                QMap<QString, QLoggingCategoryPrivate* >::iterator it = qlp->_privateCategoryObjects.find(strcategory);
-                if (it != qlp->_privateCategoryObjects.end())
-                    qlp->_privateCategoryObjects.remove(strcategory);
-
-                delete d_ptr;
-            }
-        }
-    }
+    qlp->releasePrivate(*this);
 }
 
 /*!
@@ -229,37 +210,6 @@ bool QLoggingCategory::isEnabled(QtMsgType msgtype)
 }
 
 /*!
-     \internal Overwrite the private object access function.
- */
-QLoggingCategoryPrivate * QLoggingCategory::d_func()
-{
-    if (!d_ptr) {
-        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) {
-            QString strcategory;
-            //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 = qlp->_privateCategoryObjects.find(strcategory);
-            if (it != qlp->_privateCategoryObjects.end())
-                d_ptr = *it;
-            else {
-                d_ptr = new QLoggingCategoryPrivate(_categoryName);
-                qlp->_privateCategoryObjects.insert(strcategory, d_ptr);
-            }
-            //Don't use QExplicitlySharedDataPointer for only reference counting
-            //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.
 
@@ -368,15 +318,26 @@ QLoggingPrivate *qtLoggerInstance()
 QLoggingPrivate::QLoggingPrivate()
     : QObject(0)
     , _configFileWatcher(0)
-    , _environment(EnvironmentNotChecked)
+    , _environment(false)
     , _registerCategories(false)
 {
     //Move object to the application thread
     if (QCoreApplication::instance())
         this->moveToThread(QCoreApplication::instance()->thread());
 
-    //add default category
-    _registeredCategories.append(&QLoggingCategory::defaultCategory());
+    //setup the default category
+    categoryPrivate(default_QLoggingCategory)->_enabledDebug = true;
+    _registeredCategories.append(&default_QLoggingCategory);
+
+    QByteArray ba = qgetenv("QT_LOGGING_CONFIG");
+    if (!ba.isEmpty()) {
+        QString path = QString::fromLocal8Bit(ba);
+        QString config = QLoggingPrivate::resolveConfigFile(path);
+        if (!config.isEmpty()) {
+            _environment = true;
+            setLoggingRulesFile(config);
+        }
+    }
 }
 
 /*!
@@ -514,16 +475,17 @@ void QLoggingPrivate::readSettings(QIODevice &device)
 */
 void QLoggingPrivate::updateCategory(QLoggingCategory *log)
 {
+    QLoggingCategoryPrivate *d_ptr = categoryPrivate(*log);
     //set the default back (debug disable, warning and critical enabled)
     if (log == &QLoggingCategory::defaultCategory()) {
-        log->d_func()->_enabledDebug = true;
-        log->d_func()->_enabledWarning = true;
-        log->d_func()->_enabledCritical = true;
+        d_ptr->_enabledDebug = true;
+        d_ptr->_enabledWarning = true;
+        d_ptr->_enabledCritical = true;
 
     } else {
-        log->d_func()->_enabledDebug = false;
-        log->d_func()->_enabledWarning = true;
-        log->d_func()->_enabledCritical = true;
+        d_ptr->_enabledDebug = false;
+        d_ptr->_enabledWarning = true;
+        d_ptr->_enabledCritical = true;
     }
 
     foreach (QLogConfigFilterItem item, _logConfigItemList) {
@@ -531,15 +493,15 @@ void QLoggingPrivate::updateCategory(QLoggingCategory *log)
         int filterpass = item.pass(log, QtDebugMsg);
         //apply filter if filterpass is not 0
         if (filterpass != 0)
-            log->d_func()->_enabledDebug = (filterpass > 0);
+            d_ptr->_enabledDebug = (filterpass > 0);
         //Warning
         filterpass = item.pass(log, QtWarningMsg);
         if (filterpass != 0)
-            log->d_func()->_enabledWarning = (filterpass > 0);
+            d_ptr->_enabledWarning = (filterpass > 0);
         //Critical
         filterpass = item.pass(log, QtCriticalMsg);
         if (filterpass != 0)
-            log->d_func()->_enabledCritical = (filterpass > 0);
+            d_ptr->_enabledCritical = (filterpass > 0);
     }
 }
 
@@ -549,8 +511,9 @@ void QLoggingPrivate::updateCategory(QLoggingCategory *log)
 */
 bool QLoggingPrivate::isEnabled(QLoggingCategory &category, QtMsgType type)
 {
-    if (category.d_func()->_registered)
-        return category.d_func()->statusMessageType(type);
+    QLoggingCategoryPrivate *d_ptr = categoryPrivate(category);
+    if (d_ptr->_registered)
+        return d_ptr->statusMessageType(type);
 
     //category is unregistered.
     //First update category (let it through the filter)
@@ -558,11 +521,11 @@ bool QLoggingPrivate::isEnabled(QLoggingCategory &category, QtMsgType type)
         QMutexLocker locker(&_mutexRegisteredCategory);
         //lock against _logConfigItemList between updateCategory and readSettings
         updateCategory(&category);
-        category.d_func()->_registered = true;
+        d_ptr->_registered = true;
        _registeredCategories.append(&category);
     }
 
-    return category.d_func()->statusMessageType(type);
+    return d_ptr->statusMessageType(type);
 }
 
 /*!
@@ -573,42 +536,82 @@ void QLoggingPrivate::unregisterCategory(QLoggingCategory &category)
 {
     QMutexLocker locker(&_mutexRegisteredCategory);
     //lock against _logConfigItemList between updateCategory and readSettings
-    category.d_func()->_registered = false;
+    categoryPrivate(category)->_registered = false;
     _registeredCategories.removeOne(&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.
+    Returns the private object for \a cat
 */
-bool QLoggingPrivate::checkEnvironment()
+QLoggingCategoryPrivate *QLoggingPrivate::categoryPrivate(QLoggingCategory &cat)
 {
-    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);
+    if (!cat.d_ptr) {
+        QMutexLocker locker(&_privateCategoryObjectsMutex);
+        //Another thread can call this function for the same QLoggingCategory object now
+        //Check the d_ptr after mutex lock again.
+        if (!cat.d_ptr) {
+            QString strcategory;
+            //just for the insane case someone calls this constructor with an empty category parameter
+            if (cat._categoryName)
+                strcategory = QString::fromLatin1(cat._categoryName);
+            QMap<QString, QLoggingCategoryPrivate* >::iterator it = _privateCategoryObjects.find(strcategory);
+            if (it != _privateCategoryObjects.end())
+                cat.d_ptr = *it;
+            else {
+                cat.d_ptr = new QLoggingCategoryPrivate;
+                _privateCategoryObjects.insert(strcategory, cat.d_ptr);
             }
+            cat.d_ptr->_references++;
         }
     }
-    return (_environment == EnvironmentFound);
+    return cat.d_ptr;
+}
+
+/*!
+    \internal
+    Releases the private object for \a cat
+*/
+void QLoggingPrivate::releasePrivate(QLoggingCategory &cat)
+{
+    QMutexLocker locker1(&_privateCategoryObjectsMutex);
+    cat.d_ptr->_references--;
+    if (cat.d_ptr->_references == 0) {
+        if (cat.d_ptr->_registered)
+            unregisterCategory(cat);
+        QString strcategory = QString::fromLatin1(cat._categoryName);
+        QMap<QString, QLoggingCategoryPrivate* >::iterator it = _privateCategoryObjects.find(strcategory);
+        if (it != _privateCategoryObjects.end())
+            _privateCategoryObjects.remove(strcategory);
+
+        delete cat.d_ptr;
+    }
 }
 
 /*!
+    \fn QLoggingPrivate::checkEnvironment()
+    \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.
+*/
+
+/*!
+    \fn QLoggingPrivate::regsterCategories()
+    \internal
+    Returns true if category objects should be registered.
+    This is primarily used as an optimization to avoid registering
+    category objects if no logging config has been specified.
+*/
+
+/*!
     \internal Constructor of the private QLoggingCategory object
 */
-QLoggingCategoryPrivate::QLoggingCategoryPrivate(const char *categoryname)
+QLoggingCategoryPrivate::QLoggingCategoryPrivate()
     : _enabledDebug(false)
     , _enabledWarning(true)
     , _enabledCritical(true)
     , _registered(false)
-    , _categoryName(categoryname)
     , _references(0)
 {
 }
diff --git a/src/logger/qlogger.h b/src/logger/qlogger.h
index 165a380..84bb5c7 100644
--- a/src/logger/qlogger.h
+++ b/src/logger/qlogger.h
@@ -58,8 +58,6 @@ public:
     bool isEnabled(QtMsgType msgtype);
     const char * categoryName();
     static QLoggingCategory& defaultCategory();
-protected:
-    QLoggingCategoryPrivate * d_func();
 private:
     QLoggingCategoryPrivate *d_ptr;
     const char *_categoryName;
diff --git a/src/logger/qlogger_p.h b/src/logger/qlogger_p.h
index ef74a1d..33e17e0 100644
--- a/src/logger/qlogger_p.h
+++ b/src/logger/qlogger_p.h
@@ -81,13 +81,10 @@ 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 checkEnvironment() { return _environment; }
     bool registerCategories() { return _registerCategories; }
+    QLoggingCategoryPrivate *categoryPrivate(QLoggingCategory &category);
+    void releasePrivate(QLoggingCategory &category);
 
     Q_INVOKABLE void createFileWatcher();
 
@@ -106,7 +103,7 @@ public:
     QString _configFile;
     QMutex _mutexRegisteredCategory;
     QList<QLogConfigFilterItem> _logConfigItemList;
-    EnvironmentFlag _environment;
+    bool _environment;
     bool _registerCategories;
     QMutex _privateCategoryObjectsMutex;
     QMap<QString, QLoggingCategoryPrivate *> _privateCategoryObjects;
@@ -115,14 +112,13 @@ public:
 class QLoggingCategoryPrivate
 {
 public:
-    QLoggingCategoryPrivate(const char *category);
+    QLoggingCategoryPrivate();
     virtual ~QLoggingCategoryPrivate();
     bool statusMessageType(const QtMsgType &type);
     bool _enabledDebug;
     bool _enabledWarning;
     bool _enabledCritical;
     bool _registered;
-    const char *_categoryName;
     int _references;
 };