Prevent QPixmapCache potentially growing indefinitely.

QPixmapCache has until now refused to throw out shared pixmaps, i.e.
ones that still have shallow copies lying around. This leads to problems
when someone inserts two shallow copies using different keys, causing
the cache itself containing multiple shallow copies and thus forever
refusing to throw out those entries.

It's rather easy for this to accidentally happen in a user application
since QPixmap::load() or QPixmap(const QString &fileName, ...)
automatically cache the pixmap in the QPixmapCache, thus if the user
then calls QPixmapCache::insert() on the same pixmap or a shallow copy
it is locked in the QPixmapCache forever.

The only reason for not throwing out a pixmap that's shared would be to
prevent re-loading a pixmap from file when a user has a direct reference
to it in his application, but in that case the user is unlikely to
re-load the pixmap from file in any case. Therefore it seems the best
fix is to get rid of this limitation.

Task-number: QTBUG-21359
Reviewed-by: John Brooks
Reviewed-by: Olivier Goffart

2 files changed, 26 insertions, 2 deletions

diff --git a/src/corelib/tools/qcache.h b/src/corelib/tools/qcache.h
index 16861c9..2408cd3 100644
--- a/src/corelib/tools/qcache.h
+++ b/src/corelib/tools/qcache.h
@@ -205,8 +205,7 @@ void QCache<Key,T>::trim(int m)
     while (n && total > m) {
         Node *u = n;
         n = n->p;
-        if (qIsDetached(*u->t))
-            unlink(*u);
+        unlink(*u);
     }
 }
 
diff --git a/tests/auto/qpixmapcache/tst_qpixmapcache.cpp b/tests/auto/qpixmapcache/tst_qpixmapcache.cpp
index 9f7192d..b36cf98 100644
--- a/tests/auto/qpixmapcache/tst_qpixmapcache.cpp
+++ b/tests/auto/qpixmapcache/tst_qpixmapcache.cpp
@@ -72,6 +72,7 @@ private slots:
     void clear();
     void pixmapKey();
     void noLeak();
+    void strictCacheLimit();
 };
 
 static QPixmapCache::KeyData* getPrivate(QPixmapCache::Key &key)
@@ -517,5 +518,29 @@ void tst_QPixmapCache::noLeak()
     QCOMPARE(oldSize, newSize);
 }
 
+
+void tst_QPixmapCache::strictCacheLimit()
+{
+    const int limit = 1024; // 1024 KB
+
+    QPixmapCache::clear();
+    QPixmapCache::setCacheLimit(limit);
+
+    // insert 200 64x64 pixmaps
+    // 3200 KB for 32-bit depths
+    // 1600 KB for 16-bit depths
+    // not counting the duplicate entries
+    for (int i = 0; i < 200; ++i) {
+        QPixmap pixmap(64, 64);
+        pixmap.fill(Qt::transparent);
+
+        QString id = QString::number(i);
+        QPixmapCache::insert(id + "-a", pixmap);
+        QPixmapCache::insert(id + "-b", pixmap);
+    }
+
+    QVERIFY(QPixmapCache::totalUsed() <= limit);
+}
+
 QTEST_MAIN(tst_QPixmapCache)
 #include "tst_qpixmapcache.moc"