Make QRawFont usage safer in a multi-threaded environment

QFontEngine must be accessed amd deleted only in a thread in which
it was instantiated, so we remember that thread and check if it
hasn't been changed every time we access the engine.

Change-Id: I28bc0394ced9cbd437dc950d35ffcbd99cfa7152
Reviewed-by: Pierre Rossi <pierre.rossi@theqtcompany.com>
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

3 files changed, 39 insertions, 34 deletions

diff --git a/src/gui/text/qrawfont.cpp b/src/gui/text/qrawfont.cpp
index 0c3c9bb493..d21138e7ac 100644
--- a/src/gui/text/qrawfont.cpp
+++ b/src/gui/text/qrawfont.cpp
@@ -247,7 +247,6 @@ void QRawFont::loadFromData(const QByteArray &fontData,
     d.detach();
     d->cleanUp();
     d->hintingPreference = hintingPreference;
-    d->thread = QThread::currentThread();
     d->loadFromData(fontData, pixelSize, hintingPreference);
 }
 
@@ -700,8 +699,7 @@ QRawFont QRawFont::fromFont(const QFont &font, QFontDatabase::WritingSystem writ
     }
 
     if (fe != 0) {
-        rawFont.d.data()->fontEngine = fe;
-        rawFont.d.data()->fontEngine->ref.ref();
+        rawFont.d.data()->setFontEngine(fe);
         rawFont.d.data()->hintingPreference = font.hintingPreference();
     }
     return rawFont;
@@ -712,42 +710,23 @@ QRawFont QRawFont::fromFont(const QFont &font, QFontDatabase::WritingSystem writ
 */
 void QRawFont::setPixelSize(qreal pixelSize)
 {
-    if (d->fontEngine == 0 || qFuzzyCompare(d->fontEngine->fontDef.pixelSize, pixelSize))
+    if (!d->isValid() || qFuzzyCompare(d->fontEngine->fontDef.pixelSize, pixelSize))
         return;
 
     d.detach();
-    QFontEngine *oldFontEngine = d->fontEngine;
-
-    d->fontEngine = d->fontEngine->cloneWithSize(pixelSize);
-    if (d->fontEngine != 0)
-        d->fontEngine->ref.ref();
-
-    if (!oldFontEngine->ref.deref())
-        delete oldFontEngine;
+    d->setFontEngine(d->fontEngine->cloneWithSize(pixelSize));
 }
 
 /*!
     \internal
 */
-void QRawFontPrivate::cleanUp()
-{
-    if (fontEngine != 0) {
-        if (!fontEngine->ref.deref())
-            delete fontEngine;
-        fontEngine = 0;
-    }
-    hintingPreference = QFont::PreferDefaultHinting;
-}
-
 void QRawFontPrivate::loadFromData(const QByteArray &fontData, qreal pixelSize,
                                            QFont::HintingPreference hintingPreference)
 {
     Q_ASSERT(fontEngine == 0);
 
     QPlatformFontDatabase *pfdb = QGuiApplicationPrivate::platformIntegration()->fontDatabase();
-    fontEngine = pfdb->fontEngine(fontData, pixelSize, hintingPreference);
-    if (fontEngine != 0)
-        fontEngine->ref.ref();
+    setFontEngine(pfdb->fontEngine(fontData, pixelSize, hintingPreference));
 }
 
 /*!
@@ -757,7 +736,7 @@ void QRawFontPrivate::loadFromData(const QByteArray &fontData, qreal pixelSize,
 */
 QRectF QRawFont::boundingRect(quint32 glyphIndex) const
 {
-    if (!isValid())
+    if (!d->isValid())
         return QRectF();
 
     glyph_metrics_t gm = d->fontEngine->boundingBox(glyphIndex);
diff --git a/src/gui/text/qrawfont_p.h b/src/gui/text/qrawfont_p.h
index f7a951ec59..9b0846de9a 100644
--- a/src/gui/text/qrawfont_p.h
+++ b/src/gui/text/qrawfont_p.h
@@ -66,10 +66,11 @@ public:
     {}
 
     QRawFontPrivate(const QRawFontPrivate &other)
-        : hintingPreference(other.hintingPreference)
+        : fontEngine(other.fontEngine)
+        , hintingPreference(other.hintingPreference)
         , thread(other.thread)
     {
-        fontEngine = other.fontEngine;
+        Q_ASSERT(fontEngine == 0 || thread == QThread::currentThread());
         if (fontEngine != 0)
             fontEngine->ref.ref();
     }
@@ -80,13 +81,38 @@ public:
         cleanUp();
     }
 
+    inline void cleanUp()
+    {
+        setFontEngine(0);
+        hintingPreference = QFont::PreferDefaultHinting;
+    }
+
     inline bool isValid() const
     {
-        Q_ASSERT(thread == 0 || thread == QThread::currentThread());
+        Q_ASSERT(fontEngine == 0 || thread == QThread::currentThread());
         return fontEngine != 0;
     }
 
-    void cleanUp();
+    inline void setFontEngine(QFontEngine *engine)
+    {
+        Q_ASSERT(fontEngine == 0 || thread == QThread::currentThread());
+        if (fontEngine == engine)
+            return;
+
+        if (fontEngine != 0) {
+            if (!fontEngine->ref.deref())
+                delete fontEngine;
+            thread = 0;
+        }
+
+        fontEngine = engine;
+
+        if (fontEngine != 0) {
+            fontEngine->ref.ref();
+            Q_ASSERT(thread = QThread::currentThread()); // set only if assertions enabled
+        }
+    }
+
     void loadFromData(const QByteArray &fontData,
                               qreal pixelSize,
                               QFont::HintingPreference hintingPreference);
@@ -95,9 +121,10 @@ public:
 
     QFontEngine *fontEngine;
     QFont::HintingPreference hintingPreference;
-    QThread *thread;
     QAtomicInt ref;
 
+private:
+    QThread *thread;
 };
 
 QT_END_NAMESPACE
diff --git a/src/gui/text/qtextlayout.cpp b/src/gui/text/qtextlayout.cpp
index 1ac50d3e5c..733bb57928 100644
--- a/src/gui/text/qtextlayout.cpp
+++ b/src/gui/text/qtextlayout.cpp
@@ -2063,9 +2063,8 @@ static QGlyphRun glyphRunWithInfo(QFontEngine *fontEngine,
     // Make a font for this particular engine
     QRawFont font;
     QRawFontPrivate *fontD = QRawFontPrivate::get(font);
-    fontD->fontEngine = fontEngine;
-    fontD->thread = QThread::currentThread();
-    fontD->fontEngine->ref.ref();
+    fontD->setFontEngine(fontEngine);
+
     QVarLengthArray<glyph_t> glyphsArray;
     QVarLengthArray<QFixedPoint> positionsArray;