From e6fb3b4ea00044f300249f777004aad10aaa6835 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@qt.io>
Date: Mon, 5 Jun 2023 22:15:14 +0200
Subject: [PATCH] QPixmapCache: fix leaking of QStrings and Keys on clear()

QPixmapCache maintains a mapping from QString to QPixmapCache::Key, in
the form of the cacheKeys QHash, but QPixmapCache::clear() didn't
touch it, leading to the string data (as well as the Keys) being
retained after any possible use. This can lead to memory slowly being
eaten up, as reported in QTBUG-112200, and prevents a periodic calling
of QPixmapCache::clear() from being a work-around for the issue in the
bug report.

Fix by clearing cacheKeys in QPixmapCache::clear().

This is designed as a low-risk enabler of a work-around, not a fix for
the issue. The work-around enabled by this is periodic calling of
QPixmapCache::clear().

[ChangeLog][QtGui][QPixmapCache] Fixed QString key data not being
freed on clear().

Task-number: QTBUG-112200
Change-Id: Ica6fa0e27e1b47b8df58d5e996378a2ececa5f9c
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
(cherry picked from commit 6ab0d25a09f5aeb7a5a062f7fd44e95ca761e21e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/gui/image/qpixmapcache.cpp                |  1 +
 .../image/qpixmapcache/tst_qpixmapcache.cpp   | 36 +++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/src/gui/image/qpixmapcache.cpp b/src/gui/image/qpixmapcache.cpp
index 016cd1a9873..82b7060d6f0 100644
--- a/src/gui/image/qpixmapcache.cpp
+++ b/src/gui/image/qpixmapcache.cpp
@@ -439,6 +439,7 @@ void QPMCache::clear()
         killTimer(theid);
         theid = 0;
     }
+    cacheKeys.clear();
 }
 
 QPixmapCache::KeyData* QPMCache::getKeyData(QPixmapCache::Key *key)
diff --git a/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp b/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp
index d4c39a2130b..de785fd160d 100644
--- a/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp
+++ b/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp
@@ -13,6 +13,8 @@ Q_AUTOTEST_EXPORT int qt_qpixmapcache_qpixmapcache_total_used();
 Q_AUTOTEST_EXPORT int q_QPixmapCache_keyHashSize();
 QT_END_NAMESPACE
 
+using namespace Qt::StringLiterals;
+
 class tst_QPixmapCache : public QObject
 {
     Q_OBJECT
@@ -34,6 +36,7 @@ private slots:
     void clear();
     void pixmapKey();
     void noLeak();
+    void clearDoesNotLeakStringKeys();
     void strictCacheLimit();
     void noCrashOnLargeInsert();
 };
@@ -478,6 +481,39 @@ void tst_QPixmapCache::noLeak()
     QCOMPARE(oldSize, newSize);
 }
 
+void tst_QPixmapCache::clearDoesNotLeakStringKeys()
+{
+    QPixmapCache::setCacheLimit(20); // 20KiB
+    //
+    // GIVEN: a QPixmap with QString key `key` in QPixmapCache
+    //
+    QString key;
+    {
+        QPixmap pm(64, 64);
+        QCOMPARE_LT(pm.width() * pm.height() * std::ceil(pm.depth() / 8.0),
+                    QPixmapCache::cacheLimit() * 1024);
+        pm.fill(Qt::transparent);
+        key = u"theKey"_s.repeated(20); // avoid eventual QString SSO
+        QVERIFY(key.isDetached());
+        QPixmapCache::insert(key, pm);
+    }
+    QVERIFY(!key.isDetached()); // was saved inside QPixmapCache
+
+    //
+    // WHEN: clearing the cache:
+    //
+    QPixmapCache::clear();
+
+    //
+    // THEN: `key` is no longer referenced by QPixmapCache:
+    //
+    QVERIFY(key.isDetached());
+    // verify that the pixmap is really gone from the cache
+    // (do it after the key check, because QPixmapCache cleans up `key` on a failed lookup)
+    QPixmap r;
+    QVERIFY(!QPixmapCache::find(key, &r));
+}
+
 
 void tst_QPixmapCache::strictCacheLimit()
 {