diff options
author | Tor Arne Vestbø <tor.arne.vestbo@qt.io> | 2023-12-10 15:25:44 +0100 |
---|---|---|
committer | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2024-03-05 20:14:37 +0000 |
commit | d50746ef3aec72d339f639395a2291009f186680 (patch) | |
tree | 42d07fc9148ddeca39e465590b6f456257ddb7c7 /src/corelib/kernel/qobject.cpp | |
parent | 997f86279f384c0b5c0956bbfbffff9d8507ea5f (diff) |
Debounce QDeferredDeleteEvents in QObject::deleteLater()
We used to look through the event queue in QCoreApplication::postEvent,
and if we found an existing DeferredDelete event for the receiver we
would compress the two events into one.
This was changed in 99b89d30fa5484c5d1f3cbda828648c28af4fb7d, as the
logic was causing O(n^2) for deleteLater, by using one of the bits
in QObjectData to track whether the object had already been deleted.
But it kept the logic for tracking this in QCoreApplication::postEvent,
and QCoreApplication::compressEvent would still do the work of deleting
the additional QDeferredDeleteEvents.
To avoid the unnecessary heap allocation of the QDeferredDeleteEvents
we can move the debouncing/compression to QObject::deleteLater().
We use the same mutex as in QCoreApplication::postEvent to guard
concurrent access to deleteLaterCalled.
A note has been added about the (preexisting) issue that the mutex
is not sufficient to prevent data races, as the deleteLaterCalled
flag is part of a bit-field, and we're not guarding any of our
other accesses to other bits.
As QDeferredDeleteEvents is private API, we can rely on no-one else
posting it than QObject::deleteLater(), which should be the case now
that tst_QApplication::sendPostedEvents() was fixed.
The documentation has been clarified as well. It's safe to call
deleteLater() more than once, but that's not _because_ other
pending events for the object are cleared. The latter behavior
is normal ~QObject() behavior. The documentation was probably
written at a point we didn't do any event compression at all
for QDeferredDeleteEvents.
Task-number: QTBUG-120124
Task-number: QTBUG-119918
Change-Id: I2a733095b7cb066ba494b1335aa40200c749cb0c
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 13074a967f18ed348ab744f7ff831965607a6421)
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Diffstat (limited to 'src/corelib/kernel/qobject.cpp')
-rw-r--r-- | src/corelib/kernel/qobject.cpp | 18 |
1 files changed, 18 insertions, 0 deletions
diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index faf6293d8f..54bbe3085a 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -2442,6 +2442,24 @@ void QObject::deleteLater() if (qApp == this) qWarning("You are deferring the delete of QCoreApplication, this may not work as expected."); #endif + + { + // De-bounce QDeferredDeleteEvents. Use the post event list mutex + // to guard access to deleteLaterCalled, so we don't need a separate + // mutex in QObjectData. + auto locker = QCoreApplicationPrivate::lockThreadPostEventList(this); + + // FIXME: The deleteLaterCalled flag is part of a bit field, + // so we likely have data races here, even with the mutex above, + // as long as we're not guarding every access to the bit field. + + Q_D(QObject); + if (d->deleteLaterCalled) + return; + + d->deleteLaterCalled = true; + } + QCoreApplication::postEvent(this, new QDeferredDeleteEvent()); } |