From 021a116d48a1df3624ad2221a418f1585f7ec4f8 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 3 Aug 2009 18:49:16 +0200 Subject: Add support for using QWeakPointer with QObject, replacing QPointer. The problem with QPointer is that it's a simple QObject*. So the only way for QPointer to do what it's supposed to do is if the object it's pointing to clears all QPointers when getting deleted. That means the QObject must know each and every QPointer pointing to it. To make matters worse, QPointers can be deleted while the object they're pointing to also gets deleted. So deleting QObjects must do locking. The solution to the QPointer problem is that both QObject and the "QPointer" reference something outside the QObject. This way, QObject doesn't have to lock anything to destroy itself: it's simply setting a volatile integer to zero when it gets deleted. Since the integer is outside the QObject, the integer is also refcounted. It's also O(1), so there's no problem having as many "QPointer". The two-atomic-ints structure is exactly what QSharedPointer and QWeakPointer use internally. We just abuse this structure for QObject needs, setting the strong reference count to -1 to indicate that it's a QObject that cannot be managed by a QSharedPointer. But QWeakPointer can still work and replace QPointer neatly. Reviewed-by: Bradley T. Hughes Reviewed-by: Jarek Kobus --- src/corelib/kernel/qobject.cpp | 8 ++ src/corelib/kernel/qobject_p.h | 2 + src/corelib/tools/qsharedpointer.cpp | 27 ++++++ src/corelib/tools/qsharedpointer_impl.h | 28 ++++-- tests/auto/qsharedpointer/tst_qsharedpointer.cpp | 110 +++++++++++++++++++++++ 5 files changed, 170 insertions(+), 5 deletions(-) diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index 7e7dcaf..3417232 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -56,6 +56,7 @@ #include #include #include +#include #include #include @@ -768,6 +769,13 @@ QObject::~QObject() QObjectPrivate::clearGuards(this); } + if (d->sharedRefcount) { + // indicate to all QWeakPointers that this QObject has now been deleted + d->sharedRefcount->strongref = 0; + if (!d->sharedRefcount->weakref.deref()) + delete d->sharedRefcount; + } + emit destroyed(this); if (d->declarativeData) d->declarativeData->destroyed(this); diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h index e58ee6c..056dee3 100644 --- a/src/corelib/kernel/qobject_p.h +++ b/src/corelib/kernel/qobject_p.h @@ -67,6 +67,7 @@ QT_BEGIN_NAMESPACE class QVariant; class QThreadData; class QObjectConnectionListVector; +namespace QtSharedPointer { class ExternalRefCountData; } /* mirrored in QtTestLib, DON'T CHANGE without prior warning */ struct QSignalSpyCallbackSet @@ -187,6 +188,7 @@ public: // plus QPointer, which keeps a separate list QDeclarativeData *declarativeData; QGuard *objectGuards; + QAtomicPointer sharedRefcount; int *deleteWatch; }; diff --git a/src/corelib/tools/qsharedpointer.cpp b/src/corelib/tools/qsharedpointer.cpp index ef4dfac..e2c5e95 100644 --- a/src/corelib/tools/qsharedpointer.cpp +++ b/src/corelib/tools/qsharedpointer.cpp @@ -864,6 +864,33 @@ #include #include +#if !defined(QT_NO_QOBJECT) +#include "../kernel/qobject_p.h" + +QtSharedPointer::ExternalRefCountData *QtSharedPointer::ExternalRefCountData::getAndRef(const QObject *obj) +{ + Q_ASSERT(obj); + QObjectPrivate *d = QObjectPrivate::get(const_cast(obj)); + ExternalRefCountData *that = d->sharedRefcount; + if (that) { + that->weakref.ref(); + return that; + } + + // we can create the refcount data because it doesn't exist + ExternalRefCountData *x = new ExternalRefCountData(Qt::Uninitialized); + x->strongref = -1; + x->weakref = 2; // the QWeakPointer that called us plus the QObject itself + if (!d->sharedRefcount.testAndSetRelease(0, x)) { + delete x; + d->sharedRefcount->weakref.ref(); + } + return d->sharedRefcount; +} +#endif + + + #if !defined(QT_NO_MEMBER_TEMPLATES) //# define QT_SHARED_POINTER_BACKTRACE_SUPPORT diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h index 3ee407c..1fed024 100644 --- a/src/corelib/tools/qsharedpointer_impl.h +++ b/src/corelib/tools/qsharedpointer_impl.h @@ -162,13 +162,22 @@ namespace QtSharedPointer { struct ExternalRefCountData { - QAtomicInt weakref; - QAtomicInt strongref; + QBasicAtomicInt weakref; + QBasicAtomicInt strongref; - inline ExternalRefCountData() : weakref(1), strongref(1) { } - virtual inline ~ExternalRefCountData() { Q_ASSERT(!weakref); Q_ASSERT(!strongref); } + inline ExternalRefCountData() + { + QBasicAtomicInt proto = Q_BASIC_ATOMIC_INITIALIZER(1); + weakref = strongref = proto; + } + inline ExternalRefCountData(Qt::Initialization) { } + virtual inline ~ExternalRefCountData() { Q_ASSERT(!weakref); Q_ASSERT(strongref <= 0); } virtual inline bool destroy() { return false; } + +#ifndef QT_NO_QOBJECT + Q_CORE_EXPORT static ExternalRefCountData *getAndRef(const QObject *); +#endif }; // sizeof(ExternalRefCount) = 12 (32-bit) / 16 (64-bit) @@ -376,7 +385,7 @@ namespace QtSharedPointer { tmp = o->strongref; // failed, try again } - if (tmp) + if (tmp > 0) o->weakref.ref(); else o = 0; @@ -495,6 +504,15 @@ public: inline QWeakPointer() : d(0), value(0) { } inline ~QWeakPointer() { if (d && !d->weakref.deref()) delete d; } + + // special constructor that is enabled only if X derives from QObject + template + inline QWeakPointer(X *ptr) : d(ptr ? d->getAndRef(ptr) : 0), value(ptr) + { } + template + inline QWeakPointer &operator=(X *ptr) + { return *this = QWeakPointer(ptr); } + inline QWeakPointer(const QWeakPointer &o) : d(o.d), value(o.value) { if (d) d->weakref.ref(); } inline QWeakPointer &operator=(const QWeakPointer &o) diff --git a/tests/auto/qsharedpointer/tst_qsharedpointer.cpp b/tests/auto/qsharedpointer/tst_qsharedpointer.cpp index 210f3fa..8fd2116 100644 --- a/tests/auto/qsharedpointer/tst_qsharedpointer.cpp +++ b/tests/auto/qsharedpointer/tst_qsharedpointer.cpp @@ -67,6 +67,8 @@ private slots: void memoryManagement(); void downCast(); void upCast(); + void qobjectWeakManagement(); + void noSharedPointerFromWeakQObject(); void objectCast(); void differentPointers(); void virtualBaseDifferentPointers(); @@ -503,6 +505,109 @@ class OtherObject: public QObject Q_OBJECT }; +void tst_QSharedPointer::qobjectWeakManagement() +{ + { + QObject *obj = new QObject; + QWeakPointer weak(obj); + QVERIFY(!weak.isNull()); + QVERIFY(weak.data() == obj); + + // now delete + delete obj; + QVERIFY(weak.isNull()); + } + check(); + + { + // same, bit with operator= + QObject *obj = new QObject; + QWeakPointer weak; + weak = obj; + QVERIFY(!weak.isNull()); + QVERIFY(weak.data() == obj); + + // now delete + delete obj; + QVERIFY(weak.isNull()); + } + check(); + + { + // delete triggered by parent + QObject *obj, *parent; + parent = new QObject; + obj = new QObject(parent); + QWeakPointer weak(obj); + + // now delete the parent + delete parent; + QVERIFY(weak.isNull()); + } + check(); + + { + // same as above, but set the parent after QWeakPointer is created + QObject *obj, *parent; + obj = new QObject; + QWeakPointer weak(obj); + + parent = new QObject; + obj->setParent(parent); + + // now delete the parent + delete parent; + QVERIFY(weak.isNull()); + } + check(); + + { + // with two QWeakPointers + QObject *obj = new QObject; + QWeakPointer weak(obj); + + { + QWeakPointer weak2(obj); + QVERIFY(!weak2.isNull()); + QVERIFY(weak == weak2); + } + QVERIFY(!weak.isNull()); + + delete obj; + QVERIFY(weak.isNull()); + } + check(); + + { + // same, but delete the pointer while two QWeakPointers exist + QObject *obj = new QObject; + QWeakPointer weak(obj); + + { + QWeakPointer weak2(obj); + QVERIFY(!weak2.isNull()); + + delete obj; + QVERIFY(weak.isNull()); + QVERIFY(weak2.isNull()); + } + QVERIFY(weak.isNull()); + } + check(); +} + +void tst_QSharedPointer::noSharedPointerFromWeakQObject() +{ + // you're not allowed to create a QSharedPointer from an unmanaged QObject + QObject obj; + QWeakPointer weak(&obj); + + QSharedPointer strong = weak.toStrongRef(); + QVERIFY(strong.isNull()); + + // is something went wrong, we'll probably crash here +} + void tst_QSharedPointer::objectCast() { { @@ -1446,6 +1551,11 @@ void tst_QSharedPointer::invalidConstructs_data() << &QTest::QExternalTest::tryCompileFail << "QSharedPointer ptr1;\n" "QSharedPointer ptr2 = qSharedPointerObjectCast(ptr1);"; + + QTest::newRow("weak-pointer-from-regular-pointer") + << &QTest::QExternalTest::tryCompileFail + << "Data *ptr = 0;\n" + "QWeakPointer weakptr(ptr);\n"; } void tst_QSharedPointer::invalidConstructs() -- cgit v0.12