Fix the font engines leaking

1. when there were some engines with ref > 1 in the cache, prior to calling
   QFontDatabase::{add,remove}ApplicationFont()/removeAllApplicationFonts()
   (QFontCache::clear() has never decreased engine's cache_count);
2. when the QFontEngineData's engine is not in cache i.e. the Box or Test font engine
   (~QFontEngineData() didn't free engines it keeps).

Instead of using the font engine's (external) "cache_count" counter,
QFontCache now references a given font engine every time it is inserted to
the cache and dereferences exactly that number of times in clear().

Change-Id: I87677ebd24c1f4a81a53526f2e726e596b043c61
Reviewed-by: Lars Knoll <lars.knoll@digia.com>
This commit is contained in:
Konstantin Ritt 2013-03-15 01:28:40 +02:00 committed by The Qt Project
parent ef9c3c753d
commit 36cb3f3f65
11 changed files with 285 additions and 113 deletions

View File

@ -324,9 +324,11 @@ QFontEngineData::QFontEngineData()
QFontEngineData::~QFontEngineData()
{
for (int i = 0; i < QChar::ScriptCount; ++i) {
if (engines[i])
engines[i]->ref.deref();
engines[i] = 0;
if (engines[i]) {
if (!engines[i]->ref.deref())
delete engines[i];
engines[i] = 0;
}
}
}
@ -2647,24 +2649,6 @@ QFontCache::~QFontCache()
++it;
}
}
EngineCache::ConstIterator it = engineCache.constBegin(),
end = engineCache.constEnd();
while (it != end) {
if (--it.value().data->cache_count == 0) {
if (it.value().data->ref.load() == 0) {
FC_DEBUG("QFontCache::~QFontCache: deleting engine %p key=(%d / %g %g %d %d %d)",
it.value().data, it.key().script, it.key().def.pointSize,
it.key().def.pixelSize, it.key().def.weight, it.key().def.style,
it.key().def.fixedPitch);
delete it.value().data;
} else {
FC_DEBUG("QFontCache::~QFontCache: engine = %p still has refcount %d",
it.value().data, it.value().data->ref.load());
}
}
++it;
}
}
void QFontCache::clear()
@ -2676,7 +2660,10 @@ void QFontCache::clear()
QFontEngineData *data = it.value();
for (int i = 0; i < QChar::ScriptCount; ++i) {
if (data->engines[i]) {
data->engines[i]->ref.deref();
if (!data->engines[i]->ref.deref()) {
Q_ASSERT(engineCacheCount.value(data->engines[i]) == 0);
delete data->engines[i];
}
data->engines[i] = 0;
}
}
@ -2684,23 +2671,25 @@ void QFontCache::clear()
}
}
for (EngineCache::Iterator it = engineCache.begin(), end = engineCache.end();
it != end; ++it) {
if (it->data->ref.load() == 0) {
delete it->data;
it->data = 0;
}
}
for (EngineCache::Iterator it = engineCache.begin(), end = engineCache.end();
it != end; ++it) {
if (it->data && it->data->ref.load() == 0) {
delete it->data;
it->data = 0;
bool mightHaveEnginesLeftForCleanup = true;
while (mightHaveEnginesLeftForCleanup) {
mightHaveEnginesLeftForCleanup = false;
for (EngineCache::Iterator it = engineCache.begin(), end = engineCache.end();
it != end; ++it) {
if (it.value().data && engineCacheCount.value(it.value().data) > 0) {
--engineCacheCount[it.value().data];
if (!it.value().data->ref.deref()) {
Q_ASSERT(engineCacheCount.value(it.value().data) == 0);
delete it.value().data;
mightHaveEnginesLeftForCleanup = true;
}
it.value().data = 0;
}
}
}
engineCache.clear();
engineCacheCount.clear();
}
@ -2716,7 +2705,14 @@ QFontEngineData *QFontCache::findEngineData(const QFontDef &def) const
void QFontCache::insertEngineData(const QFontDef &def, QFontEngineData *engineData)
{
#ifdef QFONTCACHE_DEBUG
FC_DEBUG("QFontCache: inserting new engine data %p", engineData);
if (engineDataCache.contains(def)) {
FC_DEBUG(" QFontCache already contains engine data %p for key=(%g %g %d %d %d)",
engineDataCache.value(def), def.pointSize,
def.pixelSize, def.weight, def.style, def.fixedPitch);
}
#endif
engineDataCache.insert(def, engineData);
increaseCost(sizeof(QFontEngineData));
@ -2741,13 +2737,22 @@ void QFontCache::updateHitCountAndTimeStamp(Engine &value)
FC_DEBUG("QFontCache: found font engine\n"
" %p: timestamp %4u hits %3u ref %2d/%2d, type '%s'",
value.data, value.timestamp, value.hits,
value.data->ref.load(), value.data->cache_count,
value.data->ref.load(), engineCacheCount.value(value.data),
value.data->name());
}
void QFontCache::insertEngine(const Key &key, QFontEngine *engine, bool insertMulti)
{
FC_DEBUG("QFontCache: inserting new engine %p", engine);
#ifdef QFONTCACHE_DEBUG
FC_DEBUG("QFontCache: inserting new engine %p, refcount %d", engine, engine->ref.load());
if (!insertMulti && engineCache.contains(key)) {
FC_DEBUG(" QFontCache already contains engine %p for key=(%g %g %d %d %d)",
engineCache.value(key).data, key.def.pointSize,
key.def.pixelSize, key.def.weight, key.def.style, key.def.fixedPitch);
}
#endif
engine->ref.ref();
Engine data(engine);
data.timestamp = ++current_timestamp;
@ -2756,12 +2761,9 @@ void QFontCache::insertEngine(const Key &key, QFontEngine *engine, bool insertMu
engineCache.insertMulti(key, data);
else
engineCache.insert(key, data);
// only increase the cost if this is the first time we insert the engine
if (engine->cache_count == 0)
if (++engineCacheCount[engine] == 1)
increaseCost(engine->cache_cost);
++engine->cache_count;
}
void QFontCache::increaseCost(uint cost)
@ -2825,11 +2827,8 @@ void QFontCache::timerEvent(QTimerEvent *)
EngineDataCache::ConstIterator it = engineDataCache.constBegin(),
end = engineDataCache.constEnd();
for (; it != end; ++it) {
#ifdef QFONTCACHE_DEBUG
FC_DEBUG(" %p: ref %2d", it.value(), int(it.value()->ref.load()));
#endif // QFONTCACHE_DEBUG
if (it.value()->ref.load() != 0)
in_use_cost += engine_data_cost;
}
@ -2843,11 +2842,11 @@ void QFontCache::timerEvent(QTimerEvent *)
for (; it != end; ++it) {
FC_DEBUG(" %p: timestamp %4u hits %2u ref %2d/%2d, cost %u bytes",
it.value().data, it.value().timestamp, it.value().hits,
it.value().data->ref.load(), it.value().data->cache_count,
it.value().data->ref.load(), engineCacheCount.value(it.value().data),
it.value().data->cache_cost);
if (it.value().data->ref.load() != 0)
in_use_cost += it.value().data->cache_cost / it.value().data->cache_count;
in_use_cost += it.value().data->cache_cost / engineCacheCount.value(it.value().data);
}
// attempt to make up for rounding errors
@ -2893,29 +2892,25 @@ void QFontCache::timerEvent(QTimerEvent *)
FC_DEBUG(" CLEAN engine data:");
// clean out all unused engine data
EngineDataCache::Iterator it = engineDataCache.begin(),
end = engineDataCache.end();
while (it != end) {
if (it.value()->ref.load() != 0) {
EngineDataCache::Iterator it = engineDataCache.begin();
while (it != engineDataCache.end()) {
if (it.value()->ref.load() == 0) {
FC_DEBUG(" %p", it.value());
decreaseCost(sizeof(QFontEngineData));
delete it.value();
it = engineDataCache.erase(it);
} else {
++it;
continue;
}
EngineDataCache::Iterator rem = it++;
decreaseCost(sizeof(QFontEngineData));
FC_DEBUG(" %p", rem.value());
delete rem.value();
engineDataCache.erase(rem);
}
}
FC_DEBUG(" CLEAN engine:");
// clean out the engine cache just enough to get below our new max cost
uint current_cost;
bool cost_decreased;
do {
current_cost = total_cost;
cost_decreased = false;
EngineCache::Iterator it = engineCache.begin(),
end = engineCache.end();
@ -2923,49 +2918,46 @@ void QFontCache::timerEvent(QTimerEvent *)
uint oldest = ~0u;
uint least_popular = ~0u;
for (; it != end; ++it) {
if (it.value().data->ref.load() != 0)
EngineCache::Iterator jt = end;
for ( ; it != end; ++it) {
if (it.value().data->ref.load() != engineCacheCount.value(it.value().data))
continue;
if (it.value().timestamp < oldest &&
it.value().hits <= least_popular) {
if (it.value().timestamp < oldest && it.value().hits <= least_popular) {
oldest = it.value().timestamp;
least_popular = it.value().hits;
jt = it;
}
}
FC_DEBUG(" oldest %u least popular %u", oldest, least_popular);
for (it = engineCache.begin(); it != end; ++it) {
if (it.value().data->ref.load() == 0 &&
it.value().timestamp == oldest &&
it.value().hits == least_popular)
break;
}
it = jt;
if (it != end) {
FC_DEBUG(" %p: timestamp %4u hits %2u ref %2d/%2d, type '%s'",
it.value().data, it.value().timestamp, it.value().hits,
it.value().data->ref.load(), it.value().data->cache_count,
it.value().data->ref.load(), engineCacheCount.value(it.value().data),
it.value().data->name());
if (--it.value().data->cache_count == 0) {
FC_DEBUG(" DELETE: last occurrence in cache");
decreaseCost(it.value().data->cache_cost);
delete it.value().data;
} else {
/*
this particular font engine is in the cache multiple
times... set current_cost to zero, so that we can
keep looping to get rid of all occurrences
*/
current_cost = 0;
QFontEngine *fontEngine = it.value().data;
// get rid of all occurrences
it = engineCache.begin();
while (it != engineCache.end()) {
if (it.value().data == fontEngine) {
fontEngine->ref.deref();
it = engineCache.erase(it);
} else {
++it;
}
}
// and delete the last occurrence
Q_ASSERT(fontEngine->ref.load() == 0);
decreaseCost(fontEngine->cache_cost);
delete fontEngine;
engineCacheCount.remove(fontEngine);
engineCache.erase(it);
cost_decreased = true;
}
} while (current_cost != total_cost && total_cost > max_cost);
} while (cost_decreased && total_cost > max_cost);
}

View File

@ -193,9 +193,8 @@ private:
};
class QFontCache : public QObject
class Q_AUTOTEST_EXPORT QFontCache : public QObject
{
Q_OBJECT
public:
// note: these static functions work on a per-thread basis
static QFontCache *instance();
@ -205,8 +204,7 @@ public:
~QFontCache();
void clear();
// universal key structure. QFontEngineDatas and QFontEngines are cached using
// the same keys
struct Key {
Key() : script(0), screen(0) { }
Key(const QFontDef &d, int c, int s = 0)
@ -245,13 +243,14 @@ public:
typedef QMap<Key,Engine> EngineCache;
EngineCache engineCache;
QHash<QFontEngine *, int> engineCacheCount;
QFontEngine *findEngine(const Key &key);
void updateHitCountAndTimeStamp(Engine &value);
void insertEngine(const Key &key, QFontEngine *engine, bool insertMulti = false);
private:
private:
void increaseCost(uint cost);
void decreaseCost(uint cost);
void timerEvent(QTimerEvent *event);

View File

@ -170,6 +170,28 @@ static void hb_freeFace(void *face)
qHBFreeFace((HB_Face)face);
}
#ifdef QT_BUILD_INTERNAL
// for testing purpose only, not thread-safe!
static QList<QFontEngine *> *enginesCollector = 0;
Q_AUTOTEST_EXPORT void QFontEngine_startCollectingEngines()
{
delete enginesCollector;
enginesCollector = new QList<QFontEngine *>();
}
Q_AUTOTEST_EXPORT QList<QFontEngine *> QFontEngine_stopCollectingEngines()
{
Q_ASSERT(enginesCollector);
QList<QFontEngine *> ret = *enginesCollector;
delete enginesCollector;
enginesCollector = 0;
return ret;
}
#endif // QT_BUILD_INTERNAL
// QFontEngine
QFontEngine::QFontEngine()
@ -177,7 +199,6 @@ QFontEngine::QFontEngine()
font_(0), font_destroy_func(0),
face_(0), face_destroy_func(0)
{
cache_count = 0;
fsType = 0;
symbol = false;
@ -194,6 +215,11 @@ QFontEngine::QFontEngine()
glyphFormat = -1;
m_subPixelPositionCount = 0;
#ifdef QT_BUILD_INTERNAL
if (enginesCollector)
enginesCollector->append(this);
#endif
}
QFontEngine::~QFontEngine()
@ -208,6 +234,11 @@ QFontEngine::~QFontEngine()
face_destroy_func(face_);
face_ = 0;
}
#ifdef QT_BUILD_INTERNAL
if (enginesCollector)
enginesCollector->removeOne(this);
#endif
}
QFixed QFontEngine::lineThickness() const
@ -1384,11 +1415,8 @@ QFontEngineMulti::~QFontEngineMulti()
{
for (int i = 0; i < engines.size(); ++i) {
QFontEngine *fontEngine = engines.at(i);
if (fontEngine) {
fontEngine->ref.deref();
if (fontEngine->cache_count == 0 && fontEngine->ref.load() == 0)
delete fontEngine;
}
if (fontEngine && !fontEngine->ref.deref())
delete fontEngine;
}
}

View File

@ -273,7 +273,6 @@ public:
mutable qt_destroy_func_t face_destroy_func;
uint cache_cost; // amount of mem used in kb by the font
int cache_count;
uint fsType : 16;
bool symbol;
struct KernPair {

View File

@ -738,8 +738,7 @@ void QRawFont::setPixelSize(qreal pixelSize)
if (d->fontEngine != 0)
d->fontEngine->ref.ref();
oldFontEngine->ref.deref();
if (oldFontEngine->cache_count == 0 && oldFontEngine->ref.load() == 0)
if (!oldFontEngine->ref.deref())
delete oldFontEngine;
}
@ -750,8 +749,7 @@ void QRawFontPrivate::cleanUp()
{
platformCleanUp();
if (fontEngine != 0) {
fontEngine->ref.deref();
if (fontEngine->cache_count == 0 && fontEngine->ref.load() == 0)
if (!fontEngine->ref.deref())
delete fontEngine;
fontEngine = 0;
}

View File

@ -1253,11 +1253,8 @@ void QTextEngine::shape(int item) const
static inline void releaseCachedFontEngine(QFontEngine *fontEngine)
{
if (fontEngine) {
fontEngine->ref.deref();
if (fontEngine->cache_count == 0 && fontEngine->ref.load() == 0)
delete fontEngine;
}
if (fontEngine && !fontEngine->ref.deref())
delete fontEngine;
}
void QTextEngine::resetFontEngineCache()

View File

@ -1097,11 +1097,11 @@ QFontEngine *QWindowsFontDatabase::fontEngine(const QByteArray &fontData, qreal
if (request.family != fontEngine->fontDef.family) {
qWarning("%s: Failed to load font. Got fallback instead: %s",
__FUNCTION__, qPrintable(fontEngine->fontDef.family));
if (fontEngine->cache_count == 0 && fontEngine->ref.load() == 0)
if (fontEngine->ref.load() == 0)
delete fontEngine;
fontEngine = 0;
} else {
Q_ASSERT(fontEngine->cache_count == 0 && fontEngine->ref.load() == 0);
Q_ASSERT(fontEngine->ref.load() == 0);
// Override the generated font name
static_cast<QWindowsFontEngine *>(fontEngine)->setUniqueFamilyName(uniqueFamilyName);

View File

@ -0,0 +1 @@
tst_qfontcache

View File

@ -0,0 +1,8 @@
CONFIG += testcase
CONFIG += parallel_test
TARGET = tst_qfontcache
QT += testlib
QT += core-private gui-private
SOURCES += tst_qfontcache.cpp
DEFINES += QT_DISABLE_DEPRECATED_BEFORE=0

View File

@ -0,0 +1,148 @@
/****************************************************************************
**
** Copyright (C) 2012 Digia Plc and/or its subsidiary(-ies).
** Contact: http://www.qt-project.org/legal
**
** This file is part of the test suite of the Qt Toolkit.
**
** $QT_BEGIN_LICENSE:LGPL$
** Commercial License Usage
** Licensees holding valid commercial Qt licenses may use this file in
** accordance with the commercial license agreement provided with the
** Software or, alternatively, in accordance with the terms contained in
** a written agreement between you and Digia. For licensing terms and
** conditions see http://qt.digia.com/licensing. For further information
** use the contact form at http://qt.digia.com/contact-us.
**
** GNU Lesser General Public License Usage
** Alternatively, this file may be used under the terms of the GNU Lesser
** General Public License version 2.1 as published by the Free Software
** Foundation and appearing in the file LICENSE.LGPL included in the
** packaging of this file. Please review the following information to
** ensure the GNU Lesser General Public License version 2.1 requirements
** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
**
** In addition, as a special exception, Digia gives you certain additional
** rights. These rights are described in the Digia Qt LGPL Exception
** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
**
** GNU General Public License Usage
** Alternatively, this file may be used under the terms of the GNU
** General Public License version 3.0 as published by the Free Software
** Foundation and appearing in the file LICENSE.GPL included in the
** packaging of this file. Please review the following information to
** ensure the GNU General Public License version 3.0 requirements will be
** met: http://www.gnu.org/copyleft/gpl.html.
**
**
** $QT_END_LICENSE$
**
****************************************************************************/
#include <QtTest/QtTest>
#include <qfont.h>
#include <private/qfont_p.h>
#include <private/qfontengine_p.h>
class tst_QFontCache : public QObject
{
Q_OBJECT
public:
tst_QFontCache();
virtual ~tst_QFontCache();
private slots:
void clear();
};
QT_BEGIN_NAMESPACE
extern void qt_setQtEnableTestFont(bool value); // qfontdatabase.cpp
#ifdef QT_BUILD_INTERNAL
// qfontengine.cpp
extern void QFontEngine_startCollectingEngines();
extern QList<QFontEngine *> QFontEngine_stopCollectingEngines();
#endif
QT_END_NAMESPACE
tst_QFontCache::tst_QFontCache()
{
}
tst_QFontCache::~tst_QFontCache()
{
}
void tst_QFontCache::clear()
{
#ifdef QT_BUILD_INTERNAL
QFontEngine_startCollectingEngines();
#else
// must not crash, at very least ;)
#endif
QFontEngine *fontEngine = 0;
{
// we're never caching the box (and the "test") font engines
// let's ensure we're not leaking them as well as the cached ones
qt_setQtEnableTestFont(true);
QFont f;
f.setFamily("__Qt__Box__Engine__");
f.exactMatch(); // loads engine
}
{
QFontDatabase db;
QFont f;
f.setStyleHint(QFont::Serif);
const QString familyForHint(f.defaultFamily());
// it should at least return a family that is available
QVERIFY(db.hasFamily(familyForHint));
f.exactMatch(); // loads engine
fontEngine = QFontPrivate::get(f)->engineForScript(QChar::Script_Common);
QVERIFY(fontEngine);
QVERIFY(QFontCache::instance()->engineCacheCount.value(fontEngine) > 0); // ensure it is cached
// acquire the engine to use it somewhere else:
// (e.g. like the we do in QFontSubset() or like QRawFont does in fromFont())
fontEngine->ref.ref();
// cache the engine once again; there is a special case when the engine is cached more than once
QFontCache::instance()->insertEngine(QFontCache::Key(QFontDef(), 0, 0), fontEngine);
}
// use it:
// e.g. fontEngine->stringToCMap(..);
// and whilst it is alive, don't hesitate to add/remove the app-local fonts:
// (QFontDatabase::{add,remove}ApplicationFont() clears the cache)
QFontCache::instance()->clear();
// release the acquired engine:
if (fontEngine) {
if (!fontEngine->ref.deref())
delete fontEngine;
fontEngine = 0;
}
// we may even exit the application now:
QFontCache::instance()->cleanup();
#ifdef QT_BUILD_INTERNAL
QList<QFontEngine *> leakedEngines = QFontEngine_stopCollectingEngines();
for (int i = 0; i < leakedEngines.size(); ++i) qWarning() << i << leakedEngines.at(i) << leakedEngines.at(i)->ref.load();
// and we are not leaking!
QCOMPARE(leakedEngines.size(), 0);
#endif
}
QTEST_MAIN(tst_QFontCache)
#include "tst_qfontcache.moc"

View File

@ -3,6 +3,7 @@ SUBDIRS=\
qabstracttextdocumentlayout \
qcssparser \
qfont \
qfontcache \
qfontdatabase \
qfontmetrics \
qglyphrun \
@ -27,6 +28,7 @@ contains(QT_CONFIG, OdfWriter):SUBDIRS += qzip qtextodfwriter
win32:SUBDIRS -= qtextpiecetable
!contains(QT_CONFIG, private_tests): SUBDIRS -= \
qfontcache \
qcssparser \
qstatictext \
qtextlayout \