Fix leaking of QFontEngineData

QFontCache now references QFontEngineData-s it maintains,
so that QFont instances not freed prior to calling ~QFontCache()
would destroy QFontEngineData on their own.

Task-number: QTBUG-25434

Change-Id: Ia7679d64de436841f09ac7be62ceb570e50cce5b
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@digia.com>
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@digia.com>
Reviewed-by: jian liang <jianliang79@gmail.com>

4 files changed, 25 insertions, 15 deletions

diff --git a/src/gui/text/qfont.cpp b/src/gui/text/qfont.cpp
index fc694dc497..1d6cf3ceca 100644
--- a/src/gui/text/qfont.cpp
+++ b/src/gui/text/qfont.cpp
@@ -191,8 +191,8 @@ QFontPrivate::QFontPrivate(const QFontPrivate &other)
 
 QFontPrivate::~QFontPrivate()
 {
-    if (engineData)
-        engineData->ref.deref();
+    if (engineData && !engineData->ref.deref())
+        delete engineData;
     engineData = 0;
     if (scFont && scFont != this)
         scFont->ref.deref();
@@ -210,7 +210,8 @@ QFontEngine *QFontPrivate::engineForScript(int script) const
         script = QChar::Script_Common;
     if (engineData && engineData->fontCache != QFontCache::instance()) {
         // throw out engineData that came from a different thread
-        engineData->ref.deref();
+        if (!engineData->ref.deref())
+            delete engineData;
         engineData = 0;
     }
     if (!engineData || !QT_FONT_ENGINE_FROM_DATA(engineData, script))
@@ -316,13 +317,14 @@ void QFontPrivate::resolve(uint mask, const QFontPrivate *other)
 
 
 QFontEngineData::QFontEngineData()
-    : ref(1), fontCache(QFontCache::instance())
+    : ref(0), fontCache(QFontCache::instance())
 {
     memset(engines, 0, QChar::ScriptCount * sizeof(QFontEngine *));
 }
 
 QFontEngineData::~QFontEngineData()
 {
+    Q_ASSERT(ref.load() == 0);
     for (int i = 0; i < QChar::ScriptCount; ++i) {
         if (engines[i]) {
             if (!engines[i]->ref.deref())
@@ -637,8 +639,8 @@ QFont::QFont(QFontPrivate *data)
 void QFont::detach()
 {
     if (d->ref.load() == 1) {
-        if (d->engineData)
-            d->engineData->ref.deref();
+        if (d->engineData && !d->engineData->ref.deref())
+            delete d->engineData;
         d->engineData = 0;
         if (d->scFont && d->scFont != d.data())
             d->scFont->ref.deref();
@@ -2641,7 +2643,7 @@ QFontCache::~QFontCache()
         EngineDataCache::ConstIterator it = engineDataCache.constBegin(),
                                  end = engineDataCache.constEnd();
         while (it != end) {
-            if (it.value()->ref.load() == 0)
+            if (!it.value()->ref.deref())
                 delete it.value();
             else
                 FC_DEBUG("QFontCache::~QFontCache: engineData %p still has refcount %d",
@@ -2713,7 +2715,9 @@ void QFontCache::insertEngineData(const QFontDef &def, QFontEngineData *engineDa
                  def.pixelSize, def.weight, def.style, def.fixedPitch);
     }
 #endif
+    Q_ASSERT(!engineDataCache.contains(def));
 
+    engineData->ref.ref();
     engineDataCache.insert(def, engineData);
     increaseCost(sizeof(QFontEngineData));
 }
@@ -2829,7 +2833,7 @@ void QFontCache::timerEvent(QTimerEvent *)
         for (; it != end; ++it) {
             FC_DEBUG("    %p: ref %2d", it.value(), int(it.value()->ref.load()));
 
-            if (it.value()->ref.load() != 0)
+            if (it.value()->ref.load() != 1)
                 in_use_cost += engine_data_cost;
         }
     }
@@ -2894,9 +2898,10 @@ void QFontCache::timerEvent(QTimerEvent *)
         // clean out all unused engine data
         EngineDataCache::Iterator it = engineDataCache.begin();
         while (it != engineDataCache.end()) {
-            if (it.value()->ref.load() == 0) {
+            if (it.value()->ref.load() == 1) {
                 FC_DEBUG("    %p", it.value());
                 decreaseCost(sizeof(QFontEngineData));
+                it.value()->ref.deref();
                 delete it.value();
                 it = engineDataCache.erase(it);
             } else {
diff --git a/src/gui/text/qfont_p.h b/src/gui/text/qfont_p.h
index 4cbf51d59c..115e866f24 100644
--- a/src/gui/text/qfont_p.h
+++ b/src/gui/text/qfont_p.h
@@ -143,6 +143,9 @@ public:
     QFontCache *fontCache;
 
     QFontEngine *engines[QChar::ScriptCount];
+
+private:
+    Q_DISABLE_COPY(QFontEngineData)
 };
 
 
diff --git a/src/gui/text/qfontdatabase.cpp b/src/gui/text/qfontdatabase.cpp
index 86a5bed15d..62379cd592 100644
--- a/src/gui/text/qfontdatabase.cpp
+++ b/src/gui/text/qfontdatabase.cpp
@@ -619,9 +619,8 @@ static void getEngineData(const QFontPrivate *d, const QFontDef &def)
         // create a new one
         d->engineData = new QFontEngineData;
         QFontCache::instance()->insertEngineData(def, d->engineData);
-    } else {
-        d->engineData->ref.ref();
     }
+    d->engineData->ref.ref();
 }
 
 static QStringList familyList(const QFontDef &req)
diff --git a/tests/auto/gui/text/qfont/tst_qfont.cpp b/tests/auto/gui/text/qfont/tst_qfont.cpp
index 0162c467d4..b8cce2671f 100644
--- a/tests/auto/gui/text/qfont/tst_qfont.cpp
+++ b/tests/auto/gui/text/qfont/tst_qfont.cpp
@@ -690,33 +690,36 @@ void tst_QFont::defaultFamily()
 
 void tst_QFont::sharing()
 {
+    // QFontCache references the engineData
+    int refs_by_cache = 1;
+
     QFont f;
     f.setStyleHint(QFont::Serif);
     f.exactMatch(); // loads engine
     QCOMPARE(QFontPrivate::get(f)->ref.load(), 1);
     QVERIFY(QFontPrivate::get(f)->engineData);
-    QCOMPARE(QFontPrivate::get(f)->engineData->ref.load(), 1);
+    QCOMPARE(QFontPrivate::get(f)->engineData->ref.load(), 1 + refs_by_cache);
 
     QFont f2(f);
     QVERIFY(QFontPrivate::get(f2) == QFontPrivate::get(f));
     QCOMPARE(QFontPrivate::get(f2)->ref.load(), 2);
     QVERIFY(QFontPrivate::get(f2)->engineData);
     QVERIFY(QFontPrivate::get(f2)->engineData == QFontPrivate::get(f)->engineData);
-    QCOMPARE(QFontPrivate::get(f2)->engineData->ref.load(), 1);
+    QCOMPARE(QFontPrivate::get(f2)->engineData->ref.load(), 1 + refs_by_cache);
 
     f2.setKerning(!f.kerning());
     QVERIFY(QFontPrivate::get(f2) != QFontPrivate::get(f));
     QCOMPARE(QFontPrivate::get(f2)->ref.load(), 1);
     QVERIFY(QFontPrivate::get(f2)->engineData);
     QVERIFY(QFontPrivate::get(f2)->engineData == QFontPrivate::get(f)->engineData);
-    QCOMPARE(QFontPrivate::get(f2)->engineData->ref.load(), 2);
+    QCOMPARE(QFontPrivate::get(f2)->engineData->ref.load(), 2 + refs_by_cache);
 
     f2 = f;
     QVERIFY(QFontPrivate::get(f2) == QFontPrivate::get(f));
     QCOMPARE(QFontPrivate::get(f2)->ref.load(), 2);
     QVERIFY(QFontPrivate::get(f2)->engineData);
     QVERIFY(QFontPrivate::get(f2)->engineData == QFontPrivate::get(f)->engineData);
-    QCOMPARE(QFontPrivate::get(f2)->engineData->ref.load(), 1);
+    QCOMPARE(QFontPrivate::get(f2)->engineData->ref.load(), 1 + refs_by_cache);
 
     if (f.pointSize() > 0)
         f2.setPointSize(f.pointSize() * 2 / 3);