From dacc222d5a3327fb27d69e57d99111cdf9084304 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C4=99drzej=20Nowacki?= <jedrzej.nowacki@digia.com>
Date: Tue, 12 Feb 2013 14:08:48 +0100
Subject: [PATCH] Fix QVector detaching in one thread while another destroys
 it.

The patch adds handling for a case when a QVector is shared between two
threads. In such scenario detaching in one thread could collide with
destruction in the other one, causing a memory leak or assert in debug
mode.

Task-number: QTBUG-29134
Change-Id: Idbff250d9cfc6cf83174954ea91dbf41f8ea4aa4
Reviewed-by: Olivier Goffart <ogoffart@woboq.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/corelib/tools/qvector.h                   |  3 +-
 .../corelib/tools/qvector/tst_qvector.cpp     | 78 ++++++++++++++++++-
 2 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/src/corelib/tools/qvector.h b/src/corelib/tools/qvector.h
index 191be6292c..94f3008274 100644
--- a/src/corelib/tools/qvector.h
+++ b/src/corelib/tools/qvector.h
@@ -506,8 +506,7 @@ void QVector<T>::reallocData(const int asize, const int aalloc, QArrayData::Allo
     }
     if (d != x) {
         if (!d->ref.deref()) {
-            Q_ASSERT(!isShared);
-            if (QTypeInfo<T>::isStatic || !aalloc) {
+            if (QTypeInfo<T>::isStatic || !aalloc || (isShared && QTypeInfo<T>::isComplex)) {
                 // data was copy constructed, we need to call destructors
                 // or if !alloc we did nothing to the old 'd'.
                 freeData(d);
diff --git a/tests/auto/corelib/tools/qvector/tst_qvector.cpp b/tests/auto/corelib/tools/qvector/tst_qvector.cpp
index c5ed44b952..7738a2c797 100644
--- a/tests/auto/corelib/tools/qvector/tst_qvector.cpp
+++ b/tests/auto/corelib/tools/qvector/tst_qvector.cpp
@@ -41,6 +41,8 @@
 
 #include <QtTest/QtTest>
 #include <QAtomicInt>
+#include <QThread>
+#include <QSemaphore>
 #include <qvector.h>
 
 struct Movable {
@@ -272,6 +274,9 @@ private slots:
     void detachInt() const;
     void detachMovable() const;
     void detachCustom() const;
+    void detachThreadSafetyInt() const;
+    void detachThreadSafetyMovable() const;
+    void detachThreadSafetyCustom() const;
 
 private:
     template<typename T> void copyConstructor() const;
@@ -295,6 +300,7 @@ private:
     template<typename T> void setSharable_data() const;
     template<typename T> void setSharable() const;
     template<typename T> void detach() const;
+    template<typename T> void detachThreadSafety() const;
 };
 
 
@@ -2213,5 +2219,75 @@ void tst_QVector::detachCustom() const
     QCOMPARE(instancesCount, Custom::counter.loadAcquire());
 }
 
-QTEST_APPLESS_MAIN(tst_QVector)
+static QAtomicPointer<QVector<int> > detachThreadSafetyDataInt;
+static QAtomicPointer<QVector<Movable> > detachThreadSafetyDataMovable;
+static QAtomicPointer<QVector<Custom> > detachThreadSafetyDataCustom;
+
+template<typename T> QAtomicPointer<QVector<T> > *detachThreadSafetyData();
+template<> QAtomicPointer<QVector<int> > *detachThreadSafetyData() { return &detachThreadSafetyDataInt; }
+template<> QAtomicPointer<QVector<Movable> > *detachThreadSafetyData() { return &detachThreadSafetyDataMovable; }
+template<> QAtomicPointer<QVector<Custom> > *detachThreadSafetyData() { return &detachThreadSafetyDataCustom; }
+
+static QSemaphore detachThreadSafetyLock;
+
+template<typename T>
+void tst_QVector::detachThreadSafety() const
+{
+    delete detachThreadSafetyData<T>()->fetchAndStoreOrdered(new QVector<T>(SimpleValue<T>::vector(400)));
+
+    static const uint threadsCount = 5;
+
+    struct : QThread {
+        void run() Q_DECL_OVERRIDE
+        {
+            QVector<T> copy(*detachThreadSafetyData<T>()->load());
+            QVERIFY(!copy.isDetached());
+            detachThreadSafetyLock.release();
+            detachThreadSafetyLock.acquire(100);
+            copy.detach();
+        }
+    } threads[threadsCount];
+
+    for (uint i = 0; i < threadsCount; ++i)
+        threads[i].start();
+    QThread::yieldCurrentThread();
+    detachThreadSafetyLock.acquire(threadsCount);
+
+    // destroy static original data
+    delete detachThreadSafetyData<T>()->fetchAndStoreOrdered(0);
+
+    QVERIFY(threadsCount < 100);
+    detachThreadSafetyLock.release(threadsCount * 100);
+    QThread::yieldCurrentThread();
+
+    for (uint i = 0; i < threadsCount; ++i)
+        threads[i].wait();
+}
+
+void tst_QVector::detachThreadSafetyInt() const
+{
+    for (uint i = 0; i < 128; ++i)
+        detachThreadSafety<int>();
+}
+
+void tst_QVector::detachThreadSafetyMovable() const
+{
+    const int instancesCount = Movable::counter.loadAcquire();
+    for (uint i = 0; i < 128; ++i) {
+        detachThreadSafety<Movable>();
+        QCOMPARE(Movable::counter.loadAcquire(), instancesCount);
+    }
+}
+
+void tst_QVector::detachThreadSafetyCustom() const
+{
+    const int instancesCount = Custom::counter.loadAcquire();
+    for (uint i = 0; i < 128; ++i) {
+        detachThreadSafety<Custom>();
+        QCOMPARE(Custom::counter.loadAcquire(), instancesCount);
+    }
+}
+
+
+QTEST_MAIN(tst_QVector)
 #include "tst_qvector.moc"