diff --git a/src/declarative/util/qdeclarativepixmapcache.cpp b/src/declarative/util/qdeclarativepixmapcache.cpp index 7ff84abcf1..e366e6b58b 100644 --- a/src/declarative/util/qdeclarativepixmapcache.cpp +++ b/src/declarative/util/qdeclarativepixmapcache.cpp @@ -189,43 +189,54 @@ public: class QDeclarativePixmapData { public: - QDeclarativePixmapData(const QUrl &u, const QSize &s, const QString &e) + QDeclarativePixmapData(QDeclarativePixmap *pixmap, const QUrl &u, const QSize &s, const QString &e) : refCount(1), inCache(false), pixmapStatus(QDeclarativePixmap::Error), - url(u), errorString(e), requestSize(s), texture(0), context(0), reply(0), prevUnreferenced(0), - prevUnreferencedPtr(0), nextUnreferenced(0) + url(u), errorString(e), requestSize(s), texture(0), context(0), + reply(0), prevUnreferenced(0), prevUnreferencedPtr(0), nextUnreferenced(0) { + declarativePixmaps.insert(pixmap); } - QDeclarativePixmapData(const QUrl &u, const QSize &r) + QDeclarativePixmapData(QDeclarativePixmap *pixmap, const QUrl &u, const QSize &r) : refCount(1), inCache(false), pixmapStatus(QDeclarativePixmap::Loading), - url(u), requestSize(r), texture(0), context(0), reply(0), prevUnreferenced(0), prevUnreferencedPtr(0), - nextUnreferenced(0) + url(u), requestSize(r), texture(0), context(0), + reply(0), prevUnreferenced(0), prevUnreferencedPtr(0), nextUnreferenced(0) { + declarativePixmaps.insert(pixmap); } - QDeclarativePixmapData(const QUrl &u, const QPixmap &p, const QSize &s, const QSize &r) + QDeclarativePixmapData(QDeclarativePixmap *pixmap, const QUrl &u, const QPixmap &p, const QSize &s, const QSize &r) : refCount(1), inCache(false), privatePixmap(false), pixmapStatus(QDeclarativePixmap::Ready), - url(u), pixmap(p), implicitSize(s), requestSize(r), texture(0), context(0), reply(0), prevUnreferenced(0), - prevUnreferencedPtr(0), nextUnreferenced(0) + url(u), pixmap(p), implicitSize(s), requestSize(r), texture(0), context(0), + reply(0), prevUnreferenced(0), prevUnreferencedPtr(0), nextUnreferenced(0) { + declarativePixmaps.insert(pixmap); } - QDeclarativePixmapData(const QUrl &u, QSGTexture *t, QSGContext *c, const QPixmap &p, const QSize &s, const QSize &r) + QDeclarativePixmapData(QDeclarativePixmap *pixmap, const QUrl &u, QSGTexture *t, QSGContext *c, const QPixmap &p, const QSize &s, const QSize &r) : refCount(1), inCache(false), privatePixmap(false), pixmapStatus(QDeclarativePixmap::Ready), - url(u), pixmap(p), implicitSize(s), requestSize(r), texture(t), context(c), reply(0), prevUnreferenced(0), - prevUnreferencedPtr(0), nextUnreferenced(0) + url(u), pixmap(p), implicitSize(s), requestSize(r), texture(t), context(c), + reply(0), prevUnreferenced(0), prevUnreferencedPtr(0), nextUnreferenced(0) { + declarativePixmaps.insert(pixmap); } - QDeclarativePixmapData(const QPixmap &p) + QDeclarativePixmapData(QDeclarativePixmap *pixmap, const QPixmap &p) : refCount(1), inCache(false), privatePixmap(true), pixmapStatus(QDeclarativePixmap::Ready), - pixmap(p), implicitSize(p.size()), requestSize(p.size()), texture(0), context(0), reply(0), prevUnreferenced(0), - prevUnreferencedPtr(0), nextUnreferenced(0) + pixmap(p), implicitSize(p.size()), requestSize(p.size()), texture(0), context(0), + reply(0), prevUnreferenced(0), prevUnreferencedPtr(0), nextUnreferenced(0) { + declarativePixmaps.insert(pixmap); } ~QDeclarativePixmapData() { + while (!declarativePixmaps.isEmpty()) { + QDeclarativePixmap *referencer = declarativePixmaps.first(); + declarativePixmaps.remove(referencer); + referencer->d = 0; + } + if (texture && context) { context->scheduleTextureForCleanup(texture); } @@ -252,6 +263,7 @@ public: QSGTexture *texture; QSGContext *context; + QIntrusiveList declarativePixmaps; QDeclarativePixmapReply *reply; QDeclarativePixmapData *prevUnreferenced; @@ -705,6 +717,7 @@ class QDeclarativePixmapStore : public QObject Q_OBJECT public: QDeclarativePixmapStore(); + ~QDeclarativePixmapStore(); void unreferencePixmap(QDeclarativePixmapData *); void referencePixmap(QDeclarativePixmapData *); @@ -741,6 +754,35 @@ QDeclarativePixmapStore::QDeclarativePixmapStore() { } +QDeclarativePixmapStore::~QDeclarativePixmapStore() +{ + int leakedPixmaps = 0; + int leakedTextures = 0; + QList cachedData = m_cache.values(); + + // unreference all (leaked) pixmaps + foreach (QDeclarativePixmapData* pixmap, cachedData) { + int currRefCount = pixmap->refCount; + if (currRefCount) { + leakedPixmaps++; + if (pixmap->texture) + leakedTextures++; + while (currRefCount > 0) { + pixmap->release(); + currRefCount--; + } + } + } + + // free all unreferenced pixmaps + while (m_lastUnreferencedPixmap) { + shrinkCache(20); + } + + if (leakedPixmaps) + qDebug("Number of leaked pixmaps: %i (of which %i are leaked textures)", leakedPixmaps, leakedTextures); +} + void QDeclarativePixmapStore::cleanTextureForContext(QDeclarativePixmapData *data) { if (data->context) { @@ -953,7 +995,7 @@ void QDeclarativePixmapData::removeFromCache() } } -static QDeclarativePixmapData* createPixmapDataSync(QDeclarativeEngine *engine, const QUrl &url, const QSize &requestSize, bool *ok) +static QDeclarativePixmapData* createPixmapDataSync(QDeclarativePixmap *declarativePixmap, QDeclarativeEngine *engine, const QUrl &url, const QSize &requestSize, bool *ok) { QSGContext *sgContext = QDeclarativeEnginePrivate::get(engine)->sgContext; @@ -964,14 +1006,14 @@ static QDeclarativePixmapData* createPixmapDataSync(QDeclarativeEngine *engine, switch (imageType) { case QDeclarativeImageProvider::Invalid: - return new QDeclarativePixmapData(url, requestSize, + return new QDeclarativePixmapData(declarativePixmap, url, requestSize, QDeclarativePixmap::tr("Invalid image provider: %1").arg(url.toString())); case QDeclarativeImageProvider::Texture: { QSGTexture *texture = ep->getTextureFromProvider(url, &readSize, requestSize); if (texture) { *ok = true; - return new QDeclarativePixmapData(url, texture, sgContext, QPixmap(), readSize, requestSize); + return new QDeclarativePixmapData(declarativePixmap, url, texture, sgContext, QPixmap(), readSize, requestSize); } } @@ -982,9 +1024,9 @@ static QDeclarativePixmapData* createPixmapDataSync(QDeclarativeEngine *engine, *ok = true; if (sgContext) { QSGTexture *t = sgContext->createTexture(image); - return new QDeclarativePixmapData(url, t, sgContext, QPixmap::fromImage(image), readSize, requestSize); + return new QDeclarativePixmapData(declarativePixmap, url, t, sgContext, QPixmap::fromImage(image), readSize, requestSize); } - return new QDeclarativePixmapData(url, QPixmap::fromImage(image), readSize, requestSize); + return new QDeclarativePixmapData(declarativePixmap, url, QPixmap::fromImage(image), readSize, requestSize); } } case QDeclarativeImageProvider::Pixmap: @@ -994,15 +1036,15 @@ static QDeclarativePixmapData* createPixmapDataSync(QDeclarativeEngine *engine, *ok = true; if (sgContext) { QSGTexture *t = sgContext->createTexture(pixmap.toImage()); - return new QDeclarativePixmapData(url, t, sgContext, pixmap, readSize, requestSize); + return new QDeclarativePixmapData(declarativePixmap, url, t, sgContext, pixmap, readSize, requestSize); } - return new QDeclarativePixmapData(url, pixmap, readSize, requestSize); + return new QDeclarativePixmapData(declarativePixmap, url, pixmap, readSize, requestSize); } } } // provider has bad image type, or provider returned null image - return new QDeclarativePixmapData(url, requestSize, + return new QDeclarativePixmapData(declarativePixmap, url, requestSize, QDeclarativePixmap::tr("Failed to get image from provider: %1").arg(url.toString())); } @@ -1034,14 +1076,14 @@ static QDeclarativePixmapData* createPixmapDataSync(QDeclarativeEngine *engine, } if (texture) - return new QDeclarativePixmapData(url, texture, ctx, QPixmap::fromImage(image), readSize, requestSize); + return new QDeclarativePixmapData(declarativePixmap, url, texture, ctx, QPixmap::fromImage(image), readSize, requestSize); else - return new QDeclarativePixmapData(url, QPixmap::fromImage(image), readSize, requestSize); + return new QDeclarativePixmapData(declarativePixmap, url, QPixmap::fromImage(image), readSize, requestSize); } else { errorString = QDeclarativePixmap::tr("Cannot open: %1").arg(url.toString()); } - return new QDeclarativePixmapData(url, requestSize, errorString); + return new QDeclarativePixmapData(declarativePixmap, url, requestSize, errorString); } @@ -1072,6 +1114,7 @@ QDeclarativePixmap::QDeclarativePixmap(QDeclarativeEngine *engine, const QUrl &u QDeclarativePixmap::~QDeclarativePixmap() { if (d) { + d->declarativePixmaps.remove(this); d->release(); d = 0; } @@ -1164,7 +1207,7 @@ void QDeclarativePixmap::setPixmap(const QPixmap &p) clear(); if (!p.isNull()) - d = new QDeclarativePixmapData(p); + d = new QDeclarativePixmapData(this, p); } int QDeclarativePixmap::width() const @@ -1208,7 +1251,11 @@ void QDeclarativePixmap::load(QDeclarativeEngine *engine, const QUrl &url, const void QDeclarativePixmap::load(QDeclarativeEngine *engine, const QUrl &url, const QSize &requestSize, QDeclarativePixmap::Options options) { - if (d) { d->release(); d = 0; } + if (d) { + d->declarativePixmaps.remove(this); + d->release(); + d = 0; + } QDeclarativePixmapKey key = { &url, &requestSize }; QDeclarativePixmapStore *store = pixmapStore(); @@ -1226,7 +1273,7 @@ void QDeclarativePixmap::load(QDeclarativeEngine *engine, const QUrl &url, const if (!(options & QDeclarativePixmap::Asynchronous)) { bool ok = false; - d = createPixmapDataSync(engine, url, requestSize, &ok); + d = createPixmapDataSync(this, engine, url, requestSize, &ok); if (ok) { if (options & QDeclarativePixmap::Cache) d->addToCache(); @@ -1239,7 +1286,7 @@ void QDeclarativePixmap::load(QDeclarativeEngine *engine, const QUrl &url, const if (!engine) return; - d = new QDeclarativePixmapData(url, requestSize); + d = new QDeclarativePixmapData(this, url, requestSize); if (options & QDeclarativePixmap::Cache) d->addToCache(); @@ -1249,12 +1296,14 @@ void QDeclarativePixmap::load(QDeclarativeEngine *engine, const QUrl &url, const } else { d = *iter; d->addref(); + d->declarativePixmaps.insert(this); } } void QDeclarativePixmap::clear() { if (d) { + d->declarativePixmaps.remove(this); d->release(); d = 0; } @@ -1265,6 +1314,7 @@ void QDeclarativePixmap::clear(QObject *obj) if (d) { if (d->reply) QObject::disconnect(d->reply, 0, obj, 0); + d->declarativePixmaps.remove(this); d->release(); d = 0; } diff --git a/src/declarative/util/qdeclarativepixmapcache_p.h b/src/declarative/util/qdeclarativepixmapcache_p.h index c1256d78f5..542ac3e3a4 100644 --- a/src/declarative/util/qdeclarativepixmapcache_p.h +++ b/src/declarative/util/qdeclarativepixmapcache_p.h @@ -47,6 +47,8 @@ #include #include +#include + QT_BEGIN_HEADER QT_BEGIN_NAMESPACE @@ -111,6 +113,8 @@ public: private: Q_DISABLE_COPY(QDeclarativePixmap) QDeclarativePixmapData *d; + QIntrusiveListNode dataListNode; + friend class QDeclarativePixmapData; }; inline QDeclarativePixmap::operator const QPixmap &() const diff --git a/tests/auto/declarative/qdeclarativepixmapcache/data/dataLeak.qml b/tests/auto/declarative/qdeclarativepixmapcache/data/dataLeak.qml new file mode 100644 index 0000000000..724ce5d816 --- /dev/null +++ b/tests/auto/declarative/qdeclarativepixmapcache/data/dataLeak.qml @@ -0,0 +1,18 @@ +import QtQuick 2.0 + +Item { + id: root + width: 800 + height: 800 + + Image { + id: i1 + source: "exists1.png"; + anchors.top: parent.top; + } + Image { + id: i2 + source: "exists2.png" + anchors.top: i1.bottom; + } +} diff --git a/tests/auto/declarative/qdeclarativepixmapcache/tst_qdeclarativepixmapcache.cpp b/tests/auto/declarative/qdeclarativepixmapcache/tst_qdeclarativepixmapcache.cpp index 5224de3e56..1bd0e70ddb 100644 --- a/tests/auto/declarative/qdeclarativepixmapcache/tst_qdeclarativepixmapcache.cpp +++ b/tests/auto/declarative/qdeclarativepixmapcache/tst_qdeclarativepixmapcache.cpp @@ -44,6 +44,7 @@ #include #include #include +#include "../shared/util.h" #include "testhttpserver.h" #ifndef QT_NO_CONCURRENT @@ -51,15 +52,19 @@ #include #endif +inline QUrl TEST_FILE(const QString &filename) +{ + return QUrl::fromLocalFile(TESTDATA(filename)); +} + class tst_qdeclarativepixmapcache : public QObject { Q_OBJECT public: tst_qdeclarativepixmapcache() : - thisfile(QUrl::fromLocalFile(QCoreApplication::applicationFilePath())), server(14452) { - server.serveDirectory(QCoreApplication::applicationDirPath() + "/data/http"); + server.serveDirectory(TESTDATA("http")); } private slots: @@ -74,13 +79,12 @@ private slots: void networkCrash(); #endif void lockingCrash(); + void dataLeak(); private: QDeclarativeEngine engine; - QUrl thisfile; TestHTTPServer server; }; - static int slotters=0; class Slotter : public QObject @@ -121,8 +125,8 @@ void tst_qdeclarativepixmapcache::single_data() QTest::addColumn("neterror"); // File URLs are optimized - QTest::newRow("local") << thisfile.resolved(QUrl("data/exists.png")) << localfile_optimized << true << false; - QTest::newRow("local") << thisfile.resolved(QUrl("data/notexists.png")) << localfile_optimized << false << false; + QTest::newRow("local") << TEST_FILE("exists.png") << localfile_optimized << true << false; + QTest::newRow("local") << TEST_FILE("notexists.png") << localfile_optimized << false << false; QTest::newRow("remote") << QUrl("http://127.0.0.1:14452/exists.png") << false << true << false; QTest::newRow("remote") << QUrl("http://127.0.0.1:14452/notexists.png") << false << false << true; } @@ -185,8 +189,8 @@ void tst_qdeclarativepixmapcache::parallel_data() QTest::addColumn("cancel"); // which one to cancel QTest::newRow("local") - << thisfile.resolved(QUrl("data/exists1.png")) - << thisfile.resolved(QUrl("data/exists2.png")) + << TEST_FILE("exists1.png") + << TEST_FILE("exists2.png") << (localfile_optimized ? 2 : 0) << -1; @@ -282,7 +286,7 @@ void tst_qdeclarativepixmapcache::parallel() void tst_qdeclarativepixmapcache::massive() { QDeclarativeEngine engine; - QUrl url = thisfile.resolved(QUrl("data/massive.png")); + QUrl url = TEST_FILE("massive.png"); // Confirm that massive images remain in the cache while they are // in use by the application. @@ -360,7 +364,7 @@ void createNetworkServer() { QEventLoop eventLoop; TestHTTPServer server(14453); - server.serveDirectory(QCoreApplication::applicationDirPath() + "/data/http"); + server.serveDirectory(TESTDATA("http")); QTimer::singleShot(100, &eventLoop, SLOT(quit())); eventLoop.exec(); } @@ -388,7 +392,7 @@ void tst_qdeclarativepixmapcache::networkCrash() void tst_qdeclarativepixmapcache::lockingCrash() { TestHTTPServer server(14453); - server.serveDirectory(QCoreApplication::applicationDirPath() + "/data/http", TestHTTPServer::Delay); + server.serveDirectory(TESTDATA("http"), TestHTTPServer::Delay); { QDeclarativePixmap* p = new QDeclarativePixmap; @@ -402,6 +406,53 @@ void tst_qdeclarativepixmapcache::lockingCrash() } } +#include +class DataLeakView : public QQuickView +{ + Q_OBJECT + +public: + explicit DataLeakView() : QQuickView() + { + setSource(TEST_FILE("dataLeak.qml")); + } + + void showFor2Seconds() + { + showFullScreen(); + QTimer::singleShot(2000, this, SIGNAL(ready())); + } + +signals: + void ready(); +}; + +// QTBUG-22742 +Q_GLOBAL_STATIC(QDeclarativePixmap, dataLeakPixmap) +void tst_qdeclarativepixmapcache::dataLeak() +{ + // Should not leak cached QDeclarativePixmapData. + // Unfortunately, since the QDeclarativePixmapStore + // is a global static, and it releases the cache + // entries on dtor (application exit), we must use + // valgrind to determine whether it leaks or not. + QDeclarativePixmap *p1 = new QDeclarativePixmap; + QDeclarativePixmap *p2 = new QDeclarativePixmap; + { + QScopedPointer test(new DataLeakView); + test->showFor2Seconds(); + dataLeakPixmap()->load(test->engine(), TEST_FILE("exists.png")); + p1->load(test->engine(), TEST_FILE("exists.png")); + p2->load(test->engine(), TEST_FILE("exists2.png")); + QTest::qWait(2005); // 2 seconds + a few more millis. + } + + // When the (global static) dataLeakPixmap is deleted, it + // shouldn't attempt to dereference a QDeclarativePixmapData + // which has been deleted by the QDeclarativePixmapStore + // destructor. +} + QTEST_MAIN(tst_qdeclarativepixmapcache) #include "tst_qdeclarativepixmapcache.moc"