QThreadStorage: fix memory leak if thread storage are added while destroying

The destructor(q) function could use itself and create new QThreadStorage.
This might be the case for example, when using qDebug in a destructor.

Task-number: QTBUG-14579
Reveiwed-by: Joao
Reviewed-by: Brad

2 files changed, 76 insertions, 3 deletions

diff --git a/src/corelib/thread/qthreadstorage.cpp b/src/corelib/thread/qthreadstorage.cpp
index 2fc04f5..2222427 100644
--- a/src/corelib/thread/qthreadstorage.cpp
+++ b/src/corelib/thread/qthreadstorage.cpp
@@ -178,11 +178,12 @@ void QThreadStorageData::finish(void **p)
         return; // nothing to do
 
     DEBUG_MSG("QThreadStorageData: Destroying storage for thread %p", QThread::currentThread());
-
-    for(int i = tls->size() - 1; i >= 0; i--) {
-        void *&value = (*tls)[i];
+    while (!tls->isEmpty()) {
+        void *&value = tls->last();
         void *q = value;
         value = 0;
+        int i = tls->size() - 1;
+        tls->resize(i);
 
         if (!q) {
             // data already deleted
diff --git a/tests/auto/qthreadstorage/tst_qthreadstorage.cpp b/tests/auto/qthreadstorage/tst_qthreadstorage.cpp
index ed86165..54f8bd9 100644
--- a/tests/auto/qthreadstorage/tst_qthreadstorage.cpp
+++ b/tests/auto/qthreadstorage/tst_qthreadstorage.cpp
@@ -77,6 +77,7 @@ private slots:
     void adoptedThreads();
     void ensureCleanupOrder();
     void QTBUG13877_crashOnExit();
+    void QTBUG14579_leakInDestructor();
 };
 
 class Pointer
@@ -310,5 +311,76 @@ void tst_QThreadStorage::QTBUG13877_crashOnExit()
     QVERIFY(process.exitStatus() != QProcess::CrashExit);
 }
 
+// S stands for thread Safe.
+class SPointer
+{
+public:
+    static QBasicAtomicInt count;
+    inline SPointer() { count.ref(); }
+    inline ~SPointer() { count.deref(); }
+};
+QBasicAtomicInt SPointer::count = Q_BASIC_ATOMIC_INITIALIZER(0);
+
+Q_GLOBAL_STATIC(QThreadStorage<SPointer *>, QTBUG14579_pointers1)
+Q_GLOBAL_STATIC(QThreadStorage<SPointer *>, QTBUG14579_pointers2)
+
+class QTBUG14579_class
+{
+public:
+    SPointer member;
+    inline ~QTBUG14579_class() {
+        QVERIFY(!QTBUG14579_pointers1()->hasLocalData());
+        QVERIFY(!QTBUG14579_pointers2()->hasLocalData());
+        QTBUG14579_pointers2()->setLocalData(new SPointer);
+        QTBUG14579_pointers1()->setLocalData(new SPointer);
+        QVERIFY(QTBUG14579_pointers1()->hasLocalData());
+        QVERIFY(QTBUG14579_pointers2()->hasLocalData());
+    }
+};
+
+
+void tst_QThreadStorage::QTBUG14579_leakInDestructor()
+{
+    class Thread : public QThread
+    {
+    public:
+        QThreadStorage<QTBUG14579_class *> &tls;
+
+        Thread(QThreadStorage<QTBUG14579_class *> &t) : tls(t) { }
+
+        void run()
+        {
+            QVERIFY(!tls.hasLocalData());
+            tls.setLocalData(new QTBUG14579_class);
+            QVERIFY(tls.hasLocalData());
+        }
+    };
+    int c = SPointer::count;
+
+    QThreadStorage<QTBUG14579_class *> tls;
+
+    QVERIFY(!QTBUG14579_pointers1()->hasLocalData());
+    QThreadStorage<int *> tls2; //add some more tls to make sure ids are not following each other too much
+    QThreadStorage<int *> tls3;
+    QVERIFY(!tls2.hasLocalData());
+    QVERIFY(!tls3.hasLocalData());
+    QVERIFY(!tls.hasLocalData());
+
+    Thread t1(tls);
+    Thread t2(tls);
+    Thread t3(tls);
+
+    t1.start();
+    t2.start();
+    t3.start();
+
+    QVERIFY(t1.wait());
+    QVERIFY(t2.wait());
+    QVERIFY(t3.wait());
+
+    //check all the constructed things have been destructed
+    QCOMPARE(int(SPointer::count), c);
+}
+
 QTEST_MAIN(tst_QThreadStorage)
 #include "tst_qthreadstorage.moc"