From d1150c5bbf6be125d496ce71570140ea28836ba5 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 5 Aug 2009 11:26:34 +0200 Subject: Restore symmetry between QSharedPointer and QWeakPointer on QObjects. With the previous commit, you could create a QWeakPointer from any QObject-derived object. It's possible because QObject now has a pointer to the QWeakPointer's d-pointer. However, if you did: QSharedPointer obj(new QObject); QWeakPointer weak1(obj); QWeakPointer weak2(obj.data()); Then weak1 would shared d-pointers with QSharedPointer, but weak2 wouldn't. Also, weak1.toStrongRef() would work, but weak2.toStrongRef() wouldn't. This change makes QObject know where the d-pointer created by QSharedPointer is, so weak2 would get the same d-pointer. As a nice side-effect, you can check if a given QObject is shared by trying to promote its QWeakPointer to QSharedPointer. Reviewed-by: Bradley T. Hughes --- src/corelib/kernel/qobject.cpp | 5 ++++ src/corelib/tools/qsharedpointer.cpp | 23 ++++++++++++++++++ src/corelib/tools/qsharedpointer_impl.h | 3 +++ tests/auto/qsharedpointer/tst_qsharedpointer.cpp | 31 ++++++++++++++++++++++++ 4 files changed, 62 insertions(+) diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index 3417232..e37b6d3 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -770,6 +770,11 @@ QObject::~QObject() } if (d->sharedRefcount) { + if (d->sharedRefcount->strongref > 0) { + qWarning("QObject: shared QObject was deleted directly. The program is malformed and may crash."); + // but continue deleting, it's too late to stop anyway + } + // indicate to all QWeakPointers that this QObject has now been deleted d->sharedRefcount->strongref = 0; if (!d->sharedRefcount->weakref.deref()) diff --git a/src/corelib/tools/qsharedpointer.cpp b/src/corelib/tools/qsharedpointer.cpp index e2c5e95..10f35c7 100644 --- a/src/corelib/tools/qsharedpointer.cpp +++ b/src/corelib/tools/qsharedpointer.cpp @@ -867,6 +867,29 @@ #if !defined(QT_NO_QOBJECT) #include "../kernel/qobject_p.h" +/*! + \internal + This function is called for a just-created QObject \a obj, to enable + the use of QSharedPointer and QWeakPointer. + + When QSharedPointer is active in a QObject, the object must not be deleted + directly: the lifetime is managed by the QSharedPointer object. In that case, + the deleteLater() and parent-child relationship in QObject only decrease + the strong reference count, instead of deleting the object. +*/ +void QtSharedPointer::ExternalRefCountData::setQObjectShared(const QObject *obj, bool) +{ + Q_ASSERT(obj); + QObjectPrivate *d = QObjectPrivate::get(const_cast(obj)); + + if (d->sharedRefcount) + qFatal("QSharedPointer: pointer %p already has reference counting", obj); + d->sharedRefcount = this; + + // QObject decreases the refcount too, so increase it up + weakref.ref(); +} + QtSharedPointer::ExternalRefCountData *QtSharedPointer::ExternalRefCountData::getAndRef(const QObject *obj) { Q_ASSERT(obj); diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h index 421db34..8a34362 100644 --- a/src/corelib/tools/qsharedpointer_impl.h +++ b/src/corelib/tools/qsharedpointer_impl.h @@ -165,7 +165,9 @@ namespace QtSharedPointer { #ifndef QT_NO_QOBJECT Q_CORE_EXPORT static ExternalRefCountData *getAndRef(const QObject *); + Q_CORE_EXPORT void setQObjectShared(const QObject *, bool enable); #endif + inline void setQObjectShared(...) { } }; // sizeof(ExternalRefCount) = 12 (32-bit) / 16 (64-bit) @@ -328,6 +330,7 @@ namespace QtSharedPointer { inline void internalFinishConstruction(T *ptr) { Basic::internalConstruct(ptr); + if (ptr) d->setQObjectShared(ptr, true); #ifdef QT_SHAREDPOINTER_TRACK_POINTERS if (ptr) internalSafetyCheckAdd2(d, ptr); #endif diff --git a/tests/auto/qsharedpointer/tst_qsharedpointer.cpp b/tests/auto/qsharedpointer/tst_qsharedpointer.cpp index 8fd2116..5214edb 100644 --- a/tests/auto/qsharedpointer/tst_qsharedpointer.cpp +++ b/tests/auto/qsharedpointer/tst_qsharedpointer.cpp @@ -69,6 +69,7 @@ private slots: void upCast(); void qobjectWeakManagement(); void noSharedPointerFromWeakQObject(); + void weakQObjectFromSharedPointer(); void objectCast(); void differentPointers(); void virtualBaseDifferentPointers(); @@ -81,6 +82,7 @@ private slots: void constCorrectness(); void customDeleter(); void creating(); + void creatingQObject(); void mixTrackingPointerCode(); void threadStressTest_data(); void threadStressTest(); @@ -608,6 +610,19 @@ void tst_QSharedPointer::noSharedPointerFromWeakQObject() // is something went wrong, we'll probably crash here } +void tst_QSharedPointer::weakQObjectFromSharedPointer() +{ + // this is the inverse of the above: you're allowed to create a QWeakPointer + // from a managed QObject + QSharedPointer shared(new QObject); + QWeakPointer weak = shared.data(); + QVERIFY(!weak.isNull()); + + // delete: + shared.clear(); + QVERIFY(weak.isNull()); +} + void tst_QSharedPointer::objectCast() { { @@ -1186,6 +1201,13 @@ void tst_QSharedPointer::customDeleter() QCOMPARE(dataDeleter.callCount, 0); QCOMPARE(derivedDataDeleter.callCount, 1); QCOMPARE(refcount, 2); + check(); +} + +void customQObjectDeleterFn(QObject *obj) +{ + ++customDeleterFnCallCount; + delete obj; } void tst_QSharedPointer::creating() @@ -1252,7 +1274,10 @@ void tst_QSharedPointer::creating() QCOMPARE(baseptr->classLevel(), 4); } check(); +} +void tst_QSharedPointer::creatingQObject() +{ { QSharedPointer ptr = QSharedPointer::create(); QCOMPARE(ptr->metaObject(), &QObject::staticMetaObject); @@ -1556,6 +1581,12 @@ void tst_QSharedPointer::invalidConstructs_data() << &QTest::QExternalTest::tryCompileFail << "Data *ptr = 0;\n" "QWeakPointer weakptr(ptr);\n"; + + QTest::newRow("shared-pointer-from-unmanaged-qobject") + << &QTest::QExternalTest::tryRunFail + << "QObject *ptr = new QObject;\n" + "QWeakPointer weak = ptr;\n" // this makes the object unmanaged + "QSharedPointer shared(ptr);\n"; } void tst_QSharedPointer::invalidConstructs() -- cgit v0.12