Fix a crash when recursing into QSharedPointer from QSharedPointer::clear()

We used to delete the tracked object before the new tracking was
properly set up in QSharedPointer. That means if the tracked object's
destructor or deletion function recursed into the QSharedPointer, it
would find itself in an inconsistent state.

So instead finish the setup and only then call out to user code. That
ensures the internal state is always valid.

Task-number: QTBUG-11730
Reviewed-by: ossi

1 files changed, 17 insertions, 14 deletions

diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h
index 964b279da5..d581751eae 100644
--- a/src/corelib/tools/qsharedpointer_impl.h
+++ b/src/corelib/tools/qsharedpointer_impl.h
@@ -323,12 +323,17 @@ namespace QtSharedPointer {
         typedef ExternalRefCountData Data;
 
         inline void ref() const { d->weakref.ref(); d->strongref.ref(); }
-        inline bool deref()
+        inline void deref()
+        { deref(d, this->value); }
+        static inline void deref(Data *d, T *value)
         {
+            if (!d) return;
             if (!d->strongref.deref()) {
-                internalDestroy();
+                if (!d->destroy())
+                    delete value;
             }
-            return d->weakref.deref();
+            if (!d->weakref.deref())
+                delete d;
         }
 
         inline void internalConstruct(T *ptr)
@@ -377,7 +382,7 @@ namespace QtSharedPointer {
         template <class X>
         inline ExternalRefCount(const ExternalRefCount<X> &other) : Basic<T>(other.value), d(other.d)
         { if (d) ref(); }
-        inline ~ExternalRefCount() { if (d && !deref()) delete d; }
+        inline ~ExternalRefCount() { deref(); }
 
         template <class X>
         inline void internalCopy(const ExternalRefCount<X> &other)
@@ -385,12 +390,6 @@ namespace QtSharedPointer {
             internalSet(other.d, other.data());
         }
 
-        inline void internalDestroy()
-        {
-            if (!d->destroy())
-                delete this->value;
-        }
-
         inline void internalSwap(ExternalRefCount &other)
         {
             qSwap(d, other.d);
@@ -423,10 +422,14 @@ namespace QtSharedPointer {
                 else
                     o = 0;
             }
-            if (d && !deref())
-                delete d;
-            d = o;
-            this->value = d && d->strongref ? actual : 0;
+
+            qSwap(d, o);
+            qSwap(this->value, actual);
+            if (!d || d->strongref == 0)
+                this->value = 0;
+
+            // dereference saved data
+            deref(o, actual);
         }
 
         Data *d;