Ensure that we never increase the strong reference count up from zero.

Also add some thread stress tests to try and detect doing it wrong.

Reviewed-By: Bradley T. Hughes

2 files changed, 138 insertions, 5 deletions

diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h
index b5daf5d816..9fa8df4c78 100644
--- a/src/corelib/tools/qsharedpointer_impl.h
+++ b/src/corelib/tools/qsharedpointer_impl.h
@@ -364,12 +364,22 @@ namespace QtSharedPointer {
         inline void internalSet(Data *o, T *actual)
         {
             if (d == o) return;
-            if (o && !o->strongref)
-                o = 0;
             if (o) {
                 verifyReconstruction(actual);
-                o->weakref.ref();
-                o->strongref.ref();
+
+                // increase the strongref, but never up from zero
+                register int tmp = o->strongref;
+                while (tmp > 0) {
+                    // try to increment from "tmp" to "tmp + 1"
+                    if (o->strongref.testAndSetRelaxed(tmp, tmp + 1))
+                        break;   // succeeded
+                    tmp = o->strongref;  // failed, try again
+                }
+
+                if (tmp)
+                    o->weakref.ref();
+                else
+                    o = 0;
             }
             if (d && !deref())
                 delete d;
diff --git a/tests/auto/qsharedpointer/tst_qsharedpointer.cpp b/tests/auto/qsharedpointer/tst_qsharedpointer.cpp
index e259e87855..210f3fae10 100644
--- a/tests/auto/qsharedpointer/tst_qsharedpointer.cpp
+++ b/tests/auto/qsharedpointer/tst_qsharedpointer.cpp
@@ -41,11 +41,16 @@
 
 #define QT_SHAREDPOINTER_TRACK_POINTERS
 #include "qsharedpointer.h"
-#include "externaltests.h"
 #include <QtTest/QtTest>
+#include <QtCore/QThread>
+#include <QtCore/QVector>
 
+#include "externaltests.h"
 #include "wrapper.h"
 
+#include <stdlib.h>
+#include <time.h>
+
 namespace QtSharedPointer {
     Q_CORE_EXPORT void internalSafetyCheckCleanCheck();
 }
@@ -75,6 +80,8 @@ private slots:
     void customDeleter();
     void creating();
     void mixTrackingPointerCode();
+    void threadStressTest_data();
+    void threadStressTest();
     void validConstructs();
     void invalidConstructs_data();
     void invalidConstructs();
@@ -1178,6 +1185,122 @@ void tst_QSharedPointer::mixTrackingPointerCode()
     }
 }
 
+class ThreadData
+{
+    QAtomicInt * volatile ptr;
+public:
+    ThreadData(QAtomicInt *p) : ptr(p) { }
+    ~ThreadData() { ++ptr; }
+    void ref()
+    {
+        // if we're called after the destructor, we'll crash
+        ptr->ref();
+    }
+};
+
+class StrongThread: public QThread
+{
+protected:
+    void run()
+    {
+        usleep(rand() % 2000);
+        ptr->ref();
+        ptr.clear();
+    }
+public:
+    QSharedPointer<ThreadData> ptr;
+};
+
+class WeakThread: public QThread
+{
+protected:
+    void run()
+    {
+        usleep(rand() % 2000);
+        QSharedPointer<ThreadData> ptr = weak;
+        if (ptr)
+            ptr->ref();
+        ptr.clear();
+    }
+public:
+    QWeakPointer<ThreadData> weak;
+};
+
+void tst_QSharedPointer::threadStressTest_data()
+{
+    QTest::addColumn<int>("strongThreadCount");
+    QTest::addColumn<int>("weakThreadCount");
+
+    QTest::newRow("0+0") << 0 << 0;
+    QTest::newRow("1+0") << 1 << 0;
+    QTest::newRow("2+0") << 2 << 0;
+    QTest::newRow("10+0") << 10 << 0;
+
+    QTest::newRow("0+1") << 0 << 1;
+    QTest::newRow("1+1") << 1 << 1;
+
+    QTest::newRow("2+10") << 2 << 10;
+    QTest::newRow("5+10") << 5 << 10;
+    QTest::newRow("5+30") << 5 << 30;
+
+    QTest::newRow("100+100") << 100 << 100;
+}
+
+void tst_QSharedPointer::threadStressTest()
+{
+    QFETCH(int, strongThreadCount);
+    QFETCH(int, weakThreadCount);
+
+    int guard1[128];
+    QAtomicInt counter;
+    int guard2[128];
+
+    memset(guard1, 0, sizeof guard1);
+    memset(guard2, 0, sizeof guard2);
+
+    for (int r = 0; r < 5; ++r) {
+        QVector<QThread*> allThreads(6 * qMax(strongThreadCount, weakThreadCount) + 3, 0);
+        QSharedPointer<ThreadData> base = QSharedPointer<ThreadData>(new ThreadData(&counter));
+        counter = 0;
+
+        // set the pointers
+        for (int i = 0; i < strongThreadCount; ++i) {
+            StrongThread *t = new StrongThread;
+            t->ptr = base;
+            allThreads[2 * i] = t;
+        }
+        for (int i = 0; i < weakThreadCount; ++i) {
+            WeakThread *t = new WeakThread;
+            t->weak = base;
+            allThreads[6 * i + 3] = t;
+        }
+
+        base.clear();
+
+        srand(time(NULL));
+        // start threads
+        for (int i = 0; i < allThreads.count(); ++i)
+            if (allThreads[i]) allThreads[i]->start();
+
+        // wait for them to finish
+        for (int i = 0; i < allThreads.count(); ++i)
+            if (allThreads[i]) allThreads[i]->wait();
+        qDeleteAll(allThreads);
+
+        // ensure the guards aren't touched
+        for (uint i = 0; i < sizeof guard1 / sizeof guard1[0]; ++i)
+            QVERIFY(!guard1[i]);
+        for (uint i = 0; i < sizeof guard2 / sizeof guard2[0]; ++i)
+            QVERIFY(!guard2[i]);
+
+        // verify that the count is the right range
+        int minValue = strongThreadCount;
+        int maxValue = strongThreadCount + weakThreadCount;
+        QVERIFY(counter >= minValue);
+        QVERIFY(counter <= maxValue);
+    }
+}
+
 void tst_QSharedPointer::validConstructs()
 {
     {