Change the pointer-tracking code to work everywhere.

Currently, if you create a QSharedPointer in code with
pointer-tracking, you must ensure it gets deleted in code with
pointer-tracking, otherwise the internal safety tracker will be
"leaking" objects. The pointers would never get removed.

And if any new pointer happened to have the same pointer address
(which happens quite often), the tracker code would promptly abort the
application.

With this change, the untracking of the pointer is scheduled by the
same code that creates the tracking. This is done by "abusing" the
custom deleter code:

- for the QSharedPointer that used ExternalRefCountWithDestroyFn
  already, we intercept the call to the destroy function and call the
  untracking function
- for a normal QSharedPointer, we use the "normalDeleter" function as
  custom deleter and chain up above

Note: the autotest only *really* works in release mode. Otherwise
functions don't get inlined and do get merged by the linker.

Reviewed-By: Bradley T. Hughes

5 files changed, 173 insertions, 12 deletions

diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h
index 55aeb10..b5daf5d 100644
--- a/src/corelib/tools/qsharedpointer_impl.h
+++ b/src/corelib/tools/qsharedpointer_impl.h
@@ -214,10 +214,19 @@ namespace QtSharedPointer {
             // delete the deleter too
             realself->extra.~Next();
         }
+        static void safetyCheckDeleter(ExternalRefCountData *self)
+        {
+            internalSafetyCheckRemove2(self);
+            deleter(self);
+        }
 
         static inline Self *create(T *ptr, Deleter userDeleter)
         {
+# ifdef QT_SHAREDPOINTER_TRACK_POINTERS
+            DestroyerFn destroy = &safetyCheckDeleter;
+# else
             DestroyerFn destroy = &deleter;
+# endif
             Self *d = static_cast<Self *>(::operator new(sizeof(Self)));
 
             // initialize the two sub-objects
@@ -244,10 +253,19 @@ namespace QtSharedPointer {
                     static_cast<ExternalRefCountWithContiguousData *>(self);
             that->data.~T();
         }
+        static void safetyCheckDeleter(ExternalRefCountData *self)
+        {
+            internalSafetyCheckRemove2(self);
+            deleter(self);
+        }
 
         static inline ExternalRefCountData *create(T **ptr)
         {
+# ifdef QT_SHAREDPOINTER_TRACK_POINTERS
+            DestroyerFn destroy = &safetyCheckDeleter;
+# else
             DestroyerFn destroy = &deleter;
+# endif
             ExternalRefCountWithContiguousData *d =
                 static_cast<ExternalRefCountWithContiguousData *>(::operator new(sizeof(ExternalRefCountWithContiguousData)));
 
@@ -282,32 +300,34 @@ namespace QtSharedPointer {
 
         inline void internalConstruct(T *ptr)
         {
-            Basic<T>::internalConstruct(ptr);
+#ifdef QT_SHAREDPOINTER_TRACK_POINTERS
+            internalConstruct<void (*)(T *)>(ptr, normalDeleter);
+#else
             Q_ASSERT(!d);
             if (ptr)
                 d = new Data;
-#ifdef QT_SHAREDPOINTER_TRACK_POINTERS
-            if (ptr) internalSafetyCheckAdd2(d, ptr);
+            internalFinishConstruction(ptr);
 #endif
         }
 
         template <typename Deleter>
         inline void internalConstruct(T *ptr, Deleter deleter)
         {
-            Basic<T>::internalConstruct(ptr);
             Q_ASSERT(!d);
             if (ptr)
                 d = ExternalRefCountWithCustomDeleter<T, Deleter>::create(ptr, deleter);
-#ifdef QT_SHAREDPOINTER_TRACK_POINTERS
-            if (ptr) internalSafetyCheckAdd2(d, ptr);
-#endif
+            internalFinishConstruction(ptr);
         }
 
         inline void internalCreate()
         {
             T *ptr;
             d = ExternalRefCountWithContiguousData<T>::create(&ptr);
+            Basic<T>::internalConstruct(ptr);
+        }
 
+        inline void internalFinishConstruction(T *ptr)
+        {
             Basic<T>::internalConstruct(ptr);
 #ifdef QT_SHAREDPOINTER_TRACK_POINTERS
             if (ptr) internalSafetyCheckAdd2(d, ptr);
@@ -327,9 +347,6 @@ namespace QtSharedPointer {
 
         inline void internalDestroy()
         {
-#ifdef QT_SHAREDPOINTER_TRACK_POINTERS
-            internalSafetyCheckRemove2(d);
-#endif
             if (!d->destroy())
                 delete this->value;
         }
@@ -450,6 +467,7 @@ public:
 
         // now initialize the data
         new (result.data()) T();
+        result.internalFinishConstruction(result.data());
         return result;
     }
 };
diff --git a/tests/auto/qsharedpointer/qsharedpointer.pro b/tests/auto/qsharedpointer/qsharedpointer.pro
index 30c81cb..1759323 100644
--- a/tests/auto/qsharedpointer/qsharedpointer.pro
+++ b/tests/auto/qsharedpointer/qsharedpointer.pro
@@ -1,8 +1,14 @@
 load(qttest_p4)
+
 SOURCES += tst_qsharedpointer.cpp \
     forwarddeclaration.cpp \
-    forwarddeclared.cpp
+    forwarddeclared.cpp \
+    wrapper.cpp
+
+HEADERS += forwarddeclared.h \
+    wrapper.h
+
 QT = core
 DEFINES += SRCDIR=\\\"$$PWD/\\\"
+
 include(externaltests.pri)
-HEADERS += forwarddeclared.h
diff --git a/tests/auto/qsharedpointer/tst_qsharedpointer.cpp b/tests/auto/qsharedpointer/tst_qsharedpointer.cpp
index 481377a..e259e87 100644
--- a/tests/auto/qsharedpointer/tst_qsharedpointer.cpp
+++ b/tests/auto/qsharedpointer/tst_qsharedpointer.cpp
@@ -44,6 +44,8 @@
 #include "externaltests.h"
 #include <QtTest/QtTest>
 
+#include "wrapper.h"
+
 namespace QtSharedPointer {
     Q_CORE_EXPORT void internalSafetyCheckCleanCheck();
 }
@@ -72,6 +74,7 @@ private slots:
     void constCorrectness();
     void customDeleter();
     void creating();
+    void mixTrackingPointerCode();
     void validConstructs();
     void invalidConstructs_data();
     void invalidConstructs();
@@ -1156,6 +1159,25 @@ void tst_QSharedPointer::creating()
     check();
 }
 
+void tst_QSharedPointer::mixTrackingPointerCode()
+{
+    {
+        // pointer created with tracking
+        // deleted in code without tracking
+        QSharedPointer<int> ptr = QSharedPointer<int>(new int(42));
+        Wrapper w(ptr);
+        ptr.clear();
+    }
+    check();
+
+    {
+        // pointer created without tracking
+        // deleted in code with tracking
+        Wrapper w = Wrapper::create();
+        w.ptr.clear();
+    }
+}
+
 void tst_QSharedPointer::validConstructs()
 {
     {
diff --git a/tests/auto/qsharedpointer/wrapper.cpp b/tests/auto/qsharedpointer/wrapper.cpp
new file mode 100644
index 0000000..7640e68
--- /dev/null
+++ b/tests/auto/qsharedpointer/wrapper.cpp
@@ -0,0 +1,60 @@
+/****************************************************************************
+**
+** Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies).
+** Contact: Nokia Corporation (qt-info@nokia.com)
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** No Commercial Usage
+** This file contains pre-release code and may not be distributed.
+** You may use this file in accordance with the terms and conditions
+** contained in the either Technology Preview License Agreement or the
+** Beta Release License Agreement.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU Lesser General Public License version 2.1 requirements
+** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, Nokia gives you certain
+** additional rights. These rights are described in the Nokia Qt LGPL
+** Exception version 1.0, included in the file LGPL_EXCEPTION.txt in this
+** package.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3.0 as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU General Public License version 3.0 requirements will be
+** met: http://www.gnu.org/copyleft/gpl.html.
+**
+** If you are unsure which license is appropriate for your use, please
+** contact the sales department at http://www.qtsoftware.com/contact.
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#ifdef QT_SHAREDPOINTER_TRACK_POINTERS
+# undef QT_SHAREDPOINTER_TRACK_POINTERS
+#endif
+#include <QtCore/qsharedpointer.h>
+#include "wrapper.h"
+
+Wrapper::Wrapper(const QSharedPointer<int> &value)
+        : ptr(value)
+{
+}
+
+Wrapper::~Wrapper()
+{
+}
+
+Wrapper Wrapper::create()
+{
+    return Wrapper(QSharedPointer<int>(new int(-47)));
+}
diff --git a/tests/auto/qsharedpointer/wrapper.h b/tests/auto/qsharedpointer/wrapper.h
new file mode 100644
index 0000000..a888063
--- /dev/null
+++ b/tests/auto/qsharedpointer/wrapper.h
@@ -0,0 +1,55 @@
+/****************************************************************************
+**
+** Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies).
+** Contact: Nokia Corporation (qt-info@nokia.com)
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** No Commercial Usage
+** This file contains pre-release code and may not be distributed.
+** You may use this file in accordance with the terms and conditions
+** contained in the either Technology Preview License Agreement or the
+** Beta Release License Agreement.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU Lesser General Public License version 2.1 requirements
+** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, Nokia gives you certain
+** additional rights. These rights are described in the Nokia Qt LGPL
+** Exception version 1.0, included in the file LGPL_EXCEPTION.txt in this
+** package.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3.0 as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU General Public License version 3.0 requirements will be
+** met: http://www.gnu.org/copyleft/gpl.html.
+**
+** If you are unsure which license is appropriate for your use, please
+** contact the sales department at http://www.qtsoftware.com/contact.
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+#ifndef WRAPPER_H
+#define WRAPPER_H
+
+template <class T> class QSharedPointer;
+class Wrapper
+{
+public:
+    QSharedPointer<int> ptr;
+    Wrapper(const QSharedPointer<int> &);
+    ~Wrapper();
+
+    static Wrapper create();
+};
+
+#endif // WRAPPER_H