Get rid of the evilness of Q_GLOBAL_STATIC_WITH_INITIALIZER

That macro is a nightmare. It leads to writing code that is
thread-unsafe or other problems. So rewrite the code that used this
macro to use special-purpose classes with constructors.

This commit does not introduce new errors. The FIXME in qicon.cpp
(qtIconCache()) was a condition already present. It does fix the race
conditions that were present in qbrush.cpp nullBrushInstance() and
qfontengine.cpp qt_grayPalette().

Specialising QGlobalStatic is also evil.

Change-Id: I039311f6a5ac9ea4ad7b310b870a2adf888da7e5
Merge-request: 10
Reviewed-by: Olivier Goffart <olivier.goffart@nokia.com>
Reviewed-on: http://codereview.qt.nokia.com/1895
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>

diff --git a/src/gui/image/qicon.cpp b/src/gui/image/qicon.cpp
index 7cf749f..724205f 100644 (file)
--- a/src/gui/image/qicon.cpp
+++ b/src/gui/image/qicon.cpp
@@ -107,8 +107,18 @@ QT_BEGIN_NAMESPACE
 static QBasicAtomicInt serialNumCounter = Q_BASIC_ATOMIC_INITIALIZER(1);
 
 static void qt_cleanup_icon_cache();
-typedef QCache<QString, QIcon> IconCache;
-Q_GLOBAL_STATIC_WITH_INITIALIZER(IconCache, qtIconCache, qAddPostRoutine(qt_cleanup_icon_cache))
+namespace {
+    class IconCache: public QCache<QString, QIcon>
+    {
+    public:
+        IconCache()
+        {
+            // ### FIXME: needs to be re-added if qApp is re-created
+            qAddPostRoutine(qt_cleanup_icon_cache);
+        }
+    };
+}
+Q_GLOBAL_STATIC(IconCache, qtIconCache)
 
 static void qt_cleanup_icon_cache()
 {

diff --git a/src/gui/painting/qbrush.cpp b/src/gui/painting/qbrush.cpp
index 8599cb1..97ea4db 100644 (file)
--- a/src/gui/painting/qbrush.cpp
+++ b/src/gui/painting/qbrush.cpp
@@ -112,6 +112,7 @@ QPixmap qt_pixmapForBrush(int brushStyle, bool invert)
     return pm;
 }
 
+static void qt_cleanup_brush_pattern_image_cache();
 class QBrushPatternImageCache
 {
 public:
@@ -123,6 +124,7 @@ public:
 
     void init()
     {
+        qAddPostRoutine(qt_cleanup_brush_pattern_image_cache);
         for (int style = Qt::Dense1Pattern; style <= Qt::DiagCrossPattern; ++style) {
             int i = style - Qt::Dense1Pattern;
             m_images[i][0] = QImage(qt_patternForBrush(style, 0), 8, 8, 1, QImage::Format_MonoLSB);
@@ -153,11 +155,7 @@ private:
     bool m_initialized;
 };
 
-static void qt_cleanup_brush_pattern_image_cache();
-Q_GLOBAL_STATIC_WITH_INITIALIZER(QBrushPatternImageCache, qt_brushPatternImageCache,
-                                 {
-                                     qAddPostRoutine(qt_cleanup_brush_pattern_image_cache);
-                                 })
+Q_GLOBAL_STATIC(QBrushPatternImageCache, qt_brushPatternImageCache)
 
 static void qt_cleanup_brush_pattern_image_cache()
 {
@@ -339,33 +337,29 @@ struct QBrushDataPointerDeleter
     \sa Qt::BrushStyle, QPainter, QColor
 */
 
-#ifndef QT_NO_THREAD
-// Special deleter that only deletes if the ref-count goes to zero
-template <>
-class QGlobalStaticDeleter<QBrushData>
+class QNullBrushData
 {
 public:
-    QGlobalStatic<QBrushData> &globalStatic;
-    QGlobalStaticDeleter(QGlobalStatic<QBrushData> &_globalStatic)
-        : globalStatic(_globalStatic)
-    { }
-
-    inline ~QGlobalStaticDeleter()
+    QBrushData *brush;
+    QNullBrushData() : brush(new QBrushData)
+    {
+        brush->ref = 1;
+        brush->style = Qt::BrushStyle(0);
+        brush->color = Qt::black;
+    }
+    ~QNullBrushData()
     {
-        if (!globalStatic.pointer->ref.deref())
-            delete globalStatic.pointer;
-        globalStatic.pointer = 0;
-        globalStatic.destroyed = true;
+        if (!brush->ref.deref())
+            delete brush;
+        brush = 0;
     }
 };
-#endif
 
-Q_GLOBAL_STATIC_WITH_INITIALIZER(QBrushData, nullBrushInstance,
-                                 {
-                                     x->ref = 1;
-                                     x->style = Qt::BrushStyle(0);
-                                     x->color = Qt::black;
-                                 })
+Q_GLOBAL_STATIC(QNullBrushData, nullBrushInstance_holder)
+static QBrushData *nullBrushInstance()
+{
+    return nullBrushInstance_holder()->brush;
+}
 
 static bool qbrush_check_type(Qt::BrushStyle style) {
     switch (style) {

diff --git a/src/gui/painting/qpen.cpp b/src/gui/painting/qpen.cpp
index 7185f0d..a79e3a0 100644 (file)
--- a/src/gui/painting/qpen.cpp
+++ b/src/gui/painting/qpen.cpp
@@ -244,30 +244,25 @@ inline QPenPrivate::QPenPrivate(const QBrush &_brush, qreal _width, Qt::PenStyle
 static const Qt::PenCapStyle qpen_default_cap = Qt::SquareCap;
 static const Qt::PenJoinStyle qpen_default_join = Qt::BevelJoin;
 
-#ifndef QT_NO_THREAD
-// Special deleter that only deletes if the ref-count goes to zero
-template <>
-class QGlobalStaticDeleter<QPenPrivate>
+class QPenDataHolder
 {
 public:
-    QGlobalStatic<QPenPrivate> &globalStatic;
-    QGlobalStaticDeleter(QGlobalStatic<QPenPrivate> &_globalStatic)
-        : globalStatic(_globalStatic)
+    QPenData *pen;
+    QPenDataHolder(const QBrush &brush, qreal width, Qt::PenStyle penStyle,
+                   Qt::PenCapStyle penCapStyle, Qt::PenJoinStyle _joinStyle)
+        : pen(new QPenData(brush, width, penStyle, penCapStyle, _joinStyle))
     { }
-
-    inline ~QGlobalStaticDeleter()
+    ~QPenDataHolder()
     {
-        if (!globalStatic.pointer->ref.deref())
-            delete globalStatic.pointer;
-        globalStatic.pointer = 0;
-        globalStatic.destroyed = true;
+        if (!pen->ref.deref())
+            delete pen;
+        pen = 0;
     }
 };
-#endif
 
-Q_GLOBAL_STATIC_WITH_ARGS(QPenData, defaultPenInstance,
+Q_GLOBAL_STATIC_WITH_ARGS(QPenDataHolder, defaultPenInstance,
                           (Qt::black, 0, Qt::SolidLine, qpen_default_cap, qpen_default_join))
-Q_GLOBAL_STATIC_WITH_ARGS(QPenData, nullPenInstance,
+Q_GLOBAL_STATIC_WITH_ARGS(QPenDataHolder, nullPenInstance,
                           (Qt::black, 0, Qt::NoPen, qpen_default_cap, qpen_default_join))
 
 /*!
@@ -276,7 +271,7 @@ Q_GLOBAL_STATIC_WITH_ARGS(QPenData, nullPenInstance,
 
 QPen::QPen()
 {
-    d = defaultPenInstance();
+    d = defaultPenInstance()->pen;
     d->ref.ref();
 }
 
@@ -289,7 +284,7 @@ QPen::QPen()
 QPen::QPen(Qt::PenStyle style)
 {
     if (style == Qt::NoPen) {
-        d = nullPenInstance();
+        d = nullPenInstance()->pen;
         d->ref.ref();
     } else {
         d = new QPenData(Qt::black, 0, style, qpen_default_cap, qpen_default_join);

diff --git a/src/gui/text/qfontengine.cpp b/src/gui/text/qfontengine.cpp
index dec0982..a258b72 100644 (file)
--- a/src/gui/text/qfontengine.cpp
+++ b/src/gui/text/qfontengine.cpp
@@ -1110,12 +1110,19 @@ QByteArray QFontEngine::convertToPostscriptFontFamilyName(const QByteArray &fami
     return f;
 }
 
-Q_GLOBAL_STATIC_WITH_INITIALIZER(QVector<QRgb>, qt_grayPalette, {
-    x->resize(256);
-    QRgb *it = x->data();
-    for (int i = 0; i < x->size(); ++i, ++it)
-        *it = 0xff000000 | i | (i<<8) | (i<<16);
-})
+class QRgbGreyPalette: public QVector<QRgb>
+{
+public:
+    QRgbGreyPalette()
+    {
+        resize(256);
+        QRgb *it = data();
+        for (int i = 0; i < size(); ++i, ++it)
+            *it = 0xff000000 | i | (i<<8) | (i<<16);
+    }
+};
+
+Q_GLOBAL_STATIC(QVector<QRgb>, qt_grayPalette)
 
 const QVector<QRgb> &QFontEngine::grayPalette()
 {

diff --git a/src/opengl/qwindowsurface_gl.cpp b/src/opengl/qwindowsurface_gl.cpp
index ff55142..4f00908 100644 (file)
--- a/src/opengl/qwindowsurface_gl.cpp
+++ b/src/opengl/qwindowsurface_gl.cpp
@@ -178,6 +178,8 @@ QGLGraphicsSystem::QGLGraphicsSystem(bool useX11GL)
 #endif
 }
 
+static void qt_cleanup_gl_share_widget();
+
 //
 // QGLWindowSurface
 //
@@ -185,6 +187,8 @@ class QGLGlobalShareWidget
 {
 public:
     QGLGlobalShareWidget() : firstPixmap(0), widgetRefCount(0), widget(0), initializing(false) {
+        // ### FIXME - readd the post routine if the qApp is recreated
+        qAddPostRoutine(qt_cleanup_gl_share_widget);
         created = true;
     }
 
@@ -238,11 +242,7 @@ private:
 bool QGLGlobalShareWidget::cleanedUp = false;
 bool QGLGlobalShareWidget::created = false;
 
-static void qt_cleanup_gl_share_widget();
-Q_GLOBAL_STATIC_WITH_INITIALIZER(QGLGlobalShareWidget, _qt_gl_share_widget,
-                                 {
-                                     qAddPostRoutine(qt_cleanup_gl_share_widget);
-                                 })
+Q_GLOBAL_STATIC(QGLGlobalShareWidget, _qt_gl_share_widget)
 
 static void qt_cleanup_gl_share_widget()
 {