diff options
author | Thiago Macieira <thiago.macieira@intel.com> | 2018-10-25 16:32:13 -0700 |
---|---|---|
committer | Thiago Macieira <thiago.macieira@intel.com> | 2018-11-03 18:25:36 +0000 |
commit | 928e77fa14e18773f994be7cf9dcf0da52f39055 (patch) | |
tree | dbc54aa3f7750b215cf0370bef87c31368eb52a8 /src | |
parent | e73bd4a5be059dcef5f8afca567f9aca31866230 (diff) |
Fix deleting of QSharedPointer internals in case QPointer loses the race
QPointer uses QWeakPointer / QSharedPointer internals in QObject and has
the code to make sure two threads won't stomp on each other if both try
to create a QPointer for the same QObject at the same time. The
threading code was fine, but had a mistake in the clean up code for the
loser thread: the QtSharedPointer::ExternalRefCountData destructor has a
Q_ASSERT for the state of the reference counts. So we need to set the
state correctly before calling the destructor.
But we don't want to do it in case the Q_ASSERT compiled to nothing. So
we use a hack that violates the Second Rule of Q_ASSERTs: don't do
something with side-effects. This way, we can insert code that will only
be compiled if Q_ASSERTs do something, without having to duplicate the
preprocessor conditions from qglobal.h.
Fixes: QTBUG-71412
Change-Id: I1bd327aeaf73421a8ec5fffd1560fdfc8b73b70c
Reviewed-by: Romain Pokrzywka <romain.pokrzywka@gmail.com>
Reviewed-by: Jędrzej Nowacki <jedrzej.nowacki@qt.io>
(cherry picked from commit 3b8075de3b3c842311c157476a85d2cf9ddff403)
Diffstat (limited to 'src')
-rw-r--r-- | src/corelib/tools/qsharedpointer.cpp | 3 |
1 files changed, 3 insertions, 0 deletions
diff --git a/src/corelib/tools/qsharedpointer.cpp b/src/corelib/tools/qsharedpointer.cpp index af09ef6f40..abb60a44f9 100644 --- a/src/corelib/tools/qsharedpointer.cpp +++ b/src/corelib/tools/qsharedpointer.cpp @@ -1453,6 +1453,9 @@ QtSharedPointer::ExternalRefCountData *QtSharedPointer::ExternalRefCountData::ge x->strongref.store(-1); x->weakref.store(2); // the QWeakPointer that called us plus the QObject itself if (!d->sharedRefcount.testAndSetRelease(0, x)) { + // ~ExternalRefCountData has a Q_ASSERT, so we use this trick to + // only execute this if Q_ASSERTs are enabled + Q_ASSERT((x->weakref.store(0), true)); delete x; x = d->sharedRefcount.loadAcquire(); x->weakref.ref(); |