Avoid returning invalidated fonts from freetype cache

If fonts were invalidated, for instance because the
application fonts were cleared, we would sometimes still
return the old freetype faces. This could happen if there
were still references to the fonts when they were
invalidated, in particular on the render thread where we
need to purge the old references after the fact. If the
same key was requested before the last reference to the
stale font had been deleted, we would create new
references to it.

For memory fonts, we use the generic ":qmemoryfonts/0" as
cache key, so we can easily get into the situation that
we already had a match for this in the stale cache.

As a result, we could end up with different fonts in the
main thread and render thread when using the font from Qt
Quick, and the glyph indexes would be from a different
font than the one used for rendering. This would show as
apparently random glyphs.

The patch introduces a secondary list of "stale" freetype
faces. These are the ones that have been invalidated but
cannot yet be deleted. We never look anything up in this,
so we don't risk returning any stale fonts. We also make
sure we purge the list whenever we can, similar to the
main faces list.

Pick-to: 6.10
Task-number: QTBUG-132108
Change-Id: I40adfdbb9aeddfffb4baed6c2f627bce9bd74ad1
Reviewed-by: Eirik Aavitsland <eirik.aavitsland@qt.io>

src/gui/text/freetype/qfontengine_ft.cpp

@@ -111,6 +111,7 @@ public:
 
     FT_Library library;
     QHash<QFontEngine::FaceId, QFreetypeFace *> faces;
+    QList<QFreetypeFace *> staleFaces;
     QHash<FaceStyle, int> faceIndices;
 };
 
@@ -122,6 +123,14 @@ QtFreetypeData::~QtFreetypeData()
             delete iter.value();
     }
     faces.clear();
+
+    for (auto iter = staleFaces.cbegin(); iter != staleFaces.cend(); ++iter) {
+        (*iter)->cleanup();
+        if (!(*iter)->ref.deref())
+            delete *iter;
+    }
+    staleFaces.clear();
+
     FT_Done_FreeType(library);
     library = nullptr;
 }
@@ -217,6 +226,17 @@ QFreetypeFace *QFreetypeFace::getFace(const QFontEngine::FaceId &face_id,
 
     QtFreetypeData *freetypeData = qt_getFreetypeData();
 
+    // Purge any stale face that is now ready to be deleted
+    for (auto it = freetypeData->staleFaces.constBegin(); it != freetypeData->staleFaces.constEnd(); ) {
+        if ((*it)->ref.loadRelaxed() == 1) {
+            (*it)->cleanup();
+            delete *it;
+            it = freetypeData->staleFaces.erase(it);
+        } else {
+            ++it;
+        }
+    }
+
     QFreetypeFace *freetype = nullptr;
     auto it = freetypeData->faces.find(face_id);
     if (it != freetypeData->faces.end()) {
@@ -442,20 +462,35 @@ void QFreetypeFace::release(const QFontEngine::FaceId &face_id)
     // from the cache.
     if (face && ref.loadRelaxed() == 1) {
         QtFreetypeData *freetypeData = qt_getFreetypeData();
-        for (auto it = freetypeData->faces.constBegin(); it != freetypeData->faces.constEnd(); ) {
+
+        for (auto it = freetypeData->staleFaces.constBegin(); it != freetypeData->staleFaces.constEnd(); ) {
+            if ((*it)->ref.loadRelaxed() == 1) {
+                (*it)->cleanup();
+                if ((*it) == this)
+                    deleteThis = true; // This face, delete at end of function for safety
+                else
+                    delete *it;
+                it = freetypeData->staleFaces.erase(it);
+            } else {
+                ++it;
+            }
+        }
+
+        for (auto it = freetypeData->faces.constBegin();
+             it != freetypeData->faces.constEnd();
+             it = freetypeData->faces.erase(it)) {
             if (it.value()->ref.loadRelaxed() == 1) {
                 it.value()->cleanup();
                 if (it.value() == this)
                     deleteThis = true; // This face, delete at end of function for safety
                 else
                     delete it.value();
-                it = freetypeData->faces.erase(it);
             } else {
-                ++it;
+                freetypeData->staleFaces.append(it.value());
             }
         }
 
-        if (freetypeData->faces.isEmpty()) {
+        if (freetypeData->faces.isEmpty() && freetypeData->staleFaces.isEmpty()) {
             FT_Done_FreeType(freetypeData->library);
             freetypeData->library = nullptr;
         }