From c5d49725779292a04fed599eb7f508d334ffc5c3 Mon Sep 17 00:00:00 2001 From: Paolo Angelelli Date: Thu, 3 Dec 2015 10:57:55 +0100 Subject: Fix for deferredDelete() bug when calling the glib loop directly This patch makes sure that all events posted using Qt on top of the GLib event loop have the loopLevel counter incremented. This is done since Qt depends on the fact that all deleteLater() calls are issued within the scope of some signal handler (in other words, triggered by the chain sendEvent() -> notifyInternal2()). There is a side effect though: in the conditions affected by this patch, that is deleteLater()s issued within a glib event handler for example, manually calling processEvents() or sendPostedEvents() with or without the QEvent::DeferredDelete flag has the same effect, and deferred deleted events are always processed. While this is not a currently working feature which the patch breaks, this side effect seems to be difficult to avoid without separating sendPostedEvents() and processEvents() into a public and a private method, in order to detect when they are manually called. Such change could perhaps be done for Qt6. An autotest for QTBUG-36434 is also included. Autotesting for QTBUG-32859 seems to be more challenging in this respect, due to its dependency on GLib. Task-number: QTBUG-18434 Task-number: QTBUG-32859 Task-number: QTBUG-36434 Change-Id: Ib89175aa27c9e38bca68ae254d182b2cd21cf7e9 Reviewed-by: Olivier Goffart (Woboq GmbH) --- src/corelib/kernel/qabstracteventdispatcher.cpp | 2 +- src/corelib/kernel/qcoreapplication.cpp | 46 +++++++++++++++++++------ 2 files changed, 37 insertions(+), 11 deletions(-) (limited to 'src/corelib/kernel') diff --git a/src/corelib/kernel/qabstracteventdispatcher.cpp b/src/corelib/kernel/qabstracteventdispatcher.cpp index 31369f9a09..907b3ccf1f 100644 --- a/src/corelib/kernel/qabstracteventdispatcher.cpp +++ b/src/corelib/kernel/qabstracteventdispatcher.cpp @@ -458,7 +458,7 @@ bool QAbstractEventDispatcher::filterNativeEvent(const QByteArray &eventType, vo if (!d->eventFilters.isEmpty()) { // Raise the loopLevel so that deleteLater() calls in or triggered // by event_filter() will be processed from the main event loop. - QScopedLoopLevelCounter loopLevelCounter(d->threadData); + QScopedScopeLevelCounter scopeLevelCounter(d->threadData); for (int i = 0; i < d->eventFilters.size(); ++i) { QAbstractNativeEventFilter *filter = d->eventFilters.at(i); if (!filter) diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp index 60f3dc0db0..42bda25be5 100644 --- a/src/corelib/kernel/qcoreapplication.cpp +++ b/src/corelib/kernel/qcoreapplication.cpp @@ -980,7 +980,7 @@ bool QCoreApplication::notifyInternal2(QObject *receiver, QEvent *event) // call overhead. QObjectPrivate *d = receiver->d_func(); QThreadData *threadData = d->threadData; - QScopedLoopLevelCounter loopLevelCounter(threadData); + QScopedScopeLevelCounter scopeLevelCounter(threadData); if (!selfRequired) return doNotify(receiver, event); return self->notify(receiver, event); @@ -1193,6 +1193,9 @@ void QCoreApplication::processEvents(QEventLoop::ProcessEventsFlags flags) */ void QCoreApplication::processEvents(QEventLoop::ProcessEventsFlags flags, int maxtime) { + // ### Qt 6: consider splitting this method into a public and a private + // one, so that a user-invoked processEvents can be detected + // and handled properly. QThreadData *data = QThreadData::current(); if (!data->hasEventDispatcher()) return; @@ -1396,8 +1399,24 @@ void QCoreApplication::postEvent(QObject *receiver, QEvent *event, int priority) if (event->type() == QEvent::DeferredDelete && data == QThreadData::current()) { // remember the current running eventloop for DeferredDelete - // events posted in the receiver's thread - static_cast(event)->level = data->loopLevel; + // events posted in the receiver's thread. + + // Events sent by non-Qt event handlers (such as glib) may not + // have the scopeLevel set correctly. The scope level makes sure that + // code like this: + // foo->deleteLater(); + // qApp->processEvents(); // without passing QEvent::DeferredDelete + // will not cause "foo" to be deleted before returning to the event loop. + + // If the scope level is 0 while loopLevel != 0, we are called from a + // non-conformant code path, and our best guess is that the scope level + // should be 1. (Loop level 0 is special: it means that no event loops + // are running.) + int loopLevel = data->loopLevel; + int scopeLevel = data->scopeLevel; + if (scopeLevel == 0 && loopLevel != 0) + scopeLevel = 1; + static_cast(event)->level = loopLevel + scopeLevel; } // delete the event on exceptions to protect against memory leaks till the event is @@ -1474,6 +1493,9 @@ bool QCoreApplication::compressEvent(QEvent *event, QObject *receiver, QPostEven */ void QCoreApplication::sendPostedEvents(QObject *receiver, int event_type) { + // ### Qt 6: consider splitting this method into a public and a private + // one, so that a user-invoked sendPostedEvents can be detected + // and handled properly. QThreadData *data = QThreadData::current(); QCoreApplicationPrivate::sendPostedEvents(receiver, event_type, data); @@ -1565,15 +1587,19 @@ void QCoreApplicationPrivate::sendPostedEvents(QObject *receiver, int event_type } if (pe.event->type() == QEvent::DeferredDelete) { - // DeferredDelete events are only sent when we are explicitly asked to - // (s.a. QEvent::DeferredDelete), and then only if the event loop that - // posted the event has returned. - int loopLevel = static_cast(pe.event)->loopLevel(); + // DeferredDelete events are sent either + // 1) when the event loop that posted the event has returned; or + // 2) if explicitly requested (with QEvent::DeferredDelete) for + // events posted by the current event loop; or + // 3) if the event was posted before the outermost event loop. + + int eventLevel = static_cast(pe.event)->loopLevel(); + int loopLevel = data->loopLevel + data->scopeLevel; const bool allowDeferredDelete = - (loopLevel > data->loopLevel - || (!loopLevel && data->loopLevel > 0) + (eventLevel > loopLevel + || (!eventLevel && loopLevel > 0) || (event_type == QEvent::DeferredDelete - && loopLevel == data->loopLevel)); + && eventLevel == loopLevel)); if (!allowDeferredDelete) { // cannot send deferred delete if (!event_type && !receiver) { -- cgit v1.2.3 From ab8cc8387f1891ccf99721bfe5a6182c507e332f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20K=C3=BCmmel?= Date: Tue, 10 Nov 2015 18:57:40 +0100 Subject: Add qOverload to select overloaded functions [ChangeLog][QtCore][Global] qOverload added to select overloaded functions. Change-Id: I7c9b1b054e6631eca0b5594db59e1202ef552c33 Reviewed-by: Olivier Goffart (Woboq GmbH) --- src/corelib/kernel/qobject.cpp | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/corelib/kernel') diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index dea8c200ef..6702f78a04 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -4538,6 +4538,8 @@ void qDeleteInEventHandler(QObject *o) make sure to declare the argument type with Q_DECLARE_METATYPE + Overloaded functions can be resolved with help of \l qOverload. + \note The number of arguments in the signal or slot are limited to 6 if the compiler does not support C++11 variadic templates. */ @@ -4573,6 +4575,8 @@ void qDeleteInEventHandler(QObject *o) However, you should take care that any objects used within the functor are still alive when the signal is emitted. + Overloaded functions can be resolved with help of \l qOverload. + \note If the compiler does not support C++11 variadic templates, the number of arguments in the signal or slot are limited to 6, and the functor object must not have an overloaded or templated operator(). @@ -4612,6 +4616,8 @@ void qDeleteInEventHandler(QObject *o) However, you should take care that any objects used within the functor are still alive when the signal is emitted. + Overloaded functions can be resolved with help of \l qOverload. + \note If the compiler does not support C++11 variadic templates, the number of arguments in the signal or slot are limited to 6, and the functor object must not have an overloaded or templated operator(). -- cgit v1.2.3 From e1538b39bb7f18b55e50f36a95bf0232cc43b328 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Sun, 14 Feb 2016 15:35:27 +0100 Subject: Share code between const and non-const QMetaObject::cast variants. Change-Id: I7aad8e5060bb17ebc04fdb137dad3b6d167895a8 Reviewed-by: Marc Mutz Reviewed-by: Olivier Goffart (Woboq GmbH) --- src/corelib/kernel/qmetaobject.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to 'src/corelib/kernel') diff --git a/src/corelib/kernel/qmetaobject.cpp b/src/corelib/kernel/qmetaobject.cpp index c5a6875a77..1c426225a5 100644 --- a/src/corelib/kernel/qmetaobject.cpp +++ b/src/corelib/kernel/qmetaobject.cpp @@ -333,14 +333,8 @@ const char *QMetaObject::className() const */ QObject *QMetaObject::cast(QObject *obj) const { - if (obj) { - const QMetaObject *m = obj->metaObject(); - do { - if (m == this) - return obj; - } while ((m = m->d.superdata)); - } - return 0; + // ### Qt 6: inline + return const_cast(cast(const_cast(obj))); } /*! -- cgit v1.2.3 From bd352604153543d8527929c7671fcfd2a22acc4a Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Sun, 14 Feb 2016 16:24:13 +0100 Subject: Microoptimize QObject::activate. This patch removes temporary variables that are not used in all cases. This reduces the instruction count per iteration for the corresponding benchmark by 1 or 2. Before: ********* Start testing of QObjectBenchmark ********* Config: Using QtTest library 5.7.0, Qt 5.7.0 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 5.3.0) PASS : QObjectBenchmark::initTestCase() PASS : QObjectBenchmark::signal_slot_benchmark(simple function) RESULT : QObjectBenchmark::signal_slot_benchmark():"simple function": 21.0065466 instructions per iteration (total: 210,065,466, iterations: 10000000) PASS : QObjectBenchmark::signal_slot_benchmark(single signal/slot) RESULT : QObjectBenchmark::signal_slot_benchmark():"single signal/slot": 405.0829559 instructions per iteration (total: 4,050,829,559, iterations: 10000000) PASS : QObjectBenchmark::signal_slot_benchmark(multi signal/slot) RESULT : QObjectBenchmark::signal_slot_benchmark():"multi signal/slot": 405.0812465 instructions per iteration (total: 4,050,812,465, iterations: 10000000) PASS : QObjectBenchmark::signal_slot_benchmark(unconnected signal) RESULT : QObjectBenchmark::signal_slot_benchmark():"unconnected signal": 104.0147348 instructions per iteration (total: 1,040,147,349, iterations: 10000000) PASS : QObjectBenchmark::signal_slot_benchmark(single signal/ptr) RESULT : QObjectBenchmark::signal_slot_benchmark():"single signal/ptr": 369.1021099 instructions per iteration (total: 3,691,021,100, iterations: 10000000) PASS : QObjectBenchmark::signal_slot_benchmark(functor) RESULT : QObjectBenchmark::signal_slot_benchmark():"functor": 370.0982862 instructions per iteration (total: 3,700,982,862, iterations: 10000000) PASS : QObjectBenchmark::cleanupTestCase() Totals: 8 passed, 0 failed, 0 skipped, 0 blacklisted, 6036ms ********* Finished testing of QObjectBenchmark ********* After: ********* Start testing of QObjectBenchmark ********* Config: Using QtTest library 5.7.0, Qt 5.7.0 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 5.3.0) PASS : QObjectBenchmark::initTestCase() PASS : QObjectBenchmark::signal_slot_benchmark(simple function) RESULT : QObjectBenchmark::signal_slot_benchmark():"simple function": 21.0061664 instructions per iteration (total: 210,061,664, iterations: 10000000) PASS : QObjectBenchmark::signal_slot_benchmark(single signal/slot) RESULT : QObjectBenchmark::signal_slot_benchmark():"single signal/slot": 403.0829161 instructions per iteration (total: 4,030,829,162, iterations: 10000000) PASS : QObjectBenchmark::signal_slot_benchmark(multi signal/slot) RESULT : QObjectBenchmark::signal_slot_benchmark():"multi signal/slot": 403.0836305 instructions per iteration (total: 4,030,836,305, iterations: 10000000) PASS : QObjectBenchmark::signal_slot_benchmark(unconnected signal) RESULT : QObjectBenchmark::signal_slot_benchmark():"unconnected signal": 104.0150038 instructions per iteration (total: 1,040,150,039, iterations: 10000000) PASS : QObjectBenchmark::signal_slot_benchmark(single signal/ptr) RESULT : QObjectBenchmark::signal_slot_benchmark():"single signal/ptr": 368.0981987 instructions per iteration (total: 3,680,981,988, iterations: 10000000) PASS : QObjectBenchmark::signal_slot_benchmark(functor) RESULT : QObjectBenchmark::signal_slot_benchmark():"functor": 369.1179429 instructions per iteration (total: 3,691,179,429, iterations: 10000000) PASS : QObjectBenchmark::cleanupTestCase() Totals: 8 passed, 0 failed, 0 skipped, 0 blacklisted, 5975ms ********* Finished testing of QObjectBenchmark ********* Change-Id: Iba3bffbca5b58109816c8b1a7dea0796b18c8785 Reviewed-by: Olivier Goffart (Woboq GmbH) --- src/corelib/kernel/qobject.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'src/corelib/kernel') diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index 6702f78a04..e3e536d7e1 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -3713,8 +3713,6 @@ void QMetaObject::activate(QObject *sender, int signalOffset, int local_signal_i if (receiverInSameThread) { sw.switchSender(receiver, sender, signal_index); } - const QObjectPrivate::StaticMetaCallFunction callFunction = c->callFunction; - const int method_relative = c->method_relative; if (c->isSlotObject) { c->slotObj->ref(); QScopedPointer obj(c->slotObj); @@ -3727,10 +3725,12 @@ void QMetaObject::activate(QObject *sender, int signalOffset, int local_signal_i obj.reset(); locker.relock(); - } else if (callFunction && c->method_offset <= receiver->metaObject()->methodOffset()) { + } else if (c->callFunction && c->method_offset <= receiver->metaObject()->methodOffset()) { //we compare the vtable to make sure we are not in the destructor of the object. - locker.unlock(); const int methodIndex = c->method(); + const int method_relative = c->method_relative; + const auto callFunction = c->callFunction; + locker.unlock(); if (qt_signal_spy_callback_set.slot_begin_callback != 0) qt_signal_spy_callback_set.slot_begin_callback(receiver, methodIndex, argv ? argv : empty_argv); @@ -3740,7 +3740,7 @@ void QMetaObject::activate(QObject *sender, int signalOffset, int local_signal_i qt_signal_spy_callback_set.slot_end_callback(receiver, methodIndex); locker.relock(); } else { - const int method = method_relative + c->method_offset; + const int method = c->method_relative + c->method_offset; locker.unlock(); if (qt_signal_spy_callback_set.slot_begin_callback != 0) { -- cgit v1.2.3