Improve checking for event/socket notifiers and timers

Starting/stopping timers from another thread may result in errors that
may not appear until hours, days or weeks after if a release build of
Qt is used with the GLib/UNIX event dispatchers. Such errors may
manifest as warnings such as "QObject::killTimer(): Error: timer id 7
is not valid for object 0x2a51b488 (), timer has not been killed" and
application crashes (e.g. crashes in malloc, realloc and
malloc_consolidate).

Initial-patch-by: Eike Ziller <eike.ziller@digia.com>
Task-number: QTBUG-40636
Change-Id: I2de50d50eb1fc7467fcebb9c73b74d2f85137933
Reviewed-by: Jocelyn Turcotte <jocelyn.turcotte@digia.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Jonathan Liu 2014-09-18 19:54:46 +10:00
parent a6316e6e74
commit 7b7ad02681
9 changed files with 88 additions and 33 deletions

View File

@ -118,13 +118,19 @@ QT_BEGIN_NAMESPACE
void QBasicTimer::start(int msec, QObject *obj)
{
QAbstractEventDispatcher *eventDispatcher = QAbstractEventDispatcher::instance();
if (!eventDispatcher) {
if (Q_UNLIKELY(!eventDispatcher)) {
qWarning("QBasicTimer::start: QBasicTimer can only be used with threads started with QThread");
return;
}
if (Q_UNLIKELY(obj && obj->thread() != eventDispatcher->thread())) {
qWarning("QBasicTimer::start: Timers cannot be started from another thread");
return;
}
if (id) {
eventDispatcher->unregisterTimer(id);
QAbstractEventDispatcherPrivate::releaseTimerId(id);
if (Q_LIKELY(eventDispatcher->unregisterTimer(id)))
QAbstractEventDispatcherPrivate::releaseTimerId(id);
else
qWarning("QBasicTimer::start: Stopping previous timer failed. Possibly trying to stop from a different thread");
}
id = 0;
if (obj)
@ -145,13 +151,23 @@ void QBasicTimer::start(int msec, QObject *obj)
void QBasicTimer::start(int msec, Qt::TimerType timerType, QObject *obj)
{
QAbstractEventDispatcher *eventDispatcher = QAbstractEventDispatcher::instance();
if (!eventDispatcher) {
if (Q_UNLIKELY(msec < 0)) {
qWarning("QBasicTimer::start: Timers cannot have negative timeouts");
return;
}
if (Q_UNLIKELY(!eventDispatcher)) {
qWarning("QBasicTimer::start: QBasicTimer can only be used with threads started with QThread");
return;
}
if (Q_UNLIKELY(obj && obj->thread() != eventDispatcher->thread())) {
qWarning("QBasicTimer::start: Timers cannot be started from another thread");
return;
}
if (id) {
eventDispatcher->unregisterTimer(id);
QAbstractEventDispatcherPrivate::releaseTimerId(id);
if (Q_LIKELY(eventDispatcher->unregisterTimer(id)))
QAbstractEventDispatcherPrivate::releaseTimerId(id);
else
qWarning("QBasicTimer::start: Stopping previous timer failed. Possibly trying to stop from a different thread");
}
id = 0;
if (obj)
@ -167,9 +183,13 @@ void QBasicTimer::stop()
{
if (id) {
QAbstractEventDispatcher *eventDispatcher = QAbstractEventDispatcher::instance();
if (eventDispatcher)
eventDispatcher->unregisterTimer(id);
QAbstractEventDispatcherPrivate::releaseTimerId(id);
if (eventDispatcher) {
if (Q_UNLIKELY(!eventDispatcher->unregisterTimer(id))) {
qWarning("QBasicTimer::stop: Failed. Possibly trying to stop from a different thread");
return;
}
QAbstractEventDispatcherPrivate::releaseTimerId(id);
}
}
id = 0;
}

View File

@ -518,7 +518,7 @@ void QEventDispatcherGlib::registerTimer(int timerId, int interval, Qt::TimerTyp
qWarning("QEventDispatcherGlib::registerTimer: invalid arguments");
return;
} else if (object->thread() != thread() || thread() != QThread::currentThread()) {
qWarning("QObject::startTimer: timers cannot be started from another thread");
qWarning("QEventDispatcherGlib::registerTimer: timers cannot be started from another thread");
return;
}
#endif
@ -534,7 +534,7 @@ bool QEventDispatcherGlib::unregisterTimer(int timerId)
qWarning("QEventDispatcherGlib::unregisterTimer: invalid argument");
return false;
} else if (thread() != QThread::currentThread()) {
qWarning("QObject::killTimer: timers cannot be stopped from another thread");
qWarning("QEventDispatcherGlib::unregisterTimer: timers cannot be stopped from another thread");
return false;
}
#endif
@ -550,7 +550,7 @@ bool QEventDispatcherGlib::unregisterTimers(QObject *object)
qWarning("QEventDispatcherGlib::unregisterTimers: invalid argument");
return false;
} else if (object->thread() != thread() || thread() != QThread::currentThread()) {
qWarning("QObject::killTimers: timers cannot be stopped from another thread");
qWarning("QEventDispatcherGlib::unregisterTimers: timers cannot be stopped from another thread");
return false;
}
#endif

View File

@ -338,7 +338,7 @@ void QEventDispatcherUNIX::registerTimer(int timerId, int interval, Qt::TimerTyp
qWarning("QEventDispatcherUNIX::registerTimer: invalid arguments");
return;
} else if (obj->thread() != thread() || thread() != QThread::currentThread()) {
qWarning("QObject::startTimer: timers cannot be started from another thread");
qWarning("QEventDispatcherUNIX::registerTimer: timers cannot be started from another thread");
return;
}
#endif
@ -357,7 +357,7 @@ bool QEventDispatcherUNIX::unregisterTimer(int timerId)
qWarning("QEventDispatcherUNIX::unregisterTimer: invalid argument");
return false;
} else if (thread() != QThread::currentThread()) {
qWarning("QObject::killTimer: timers cannot be stopped from another thread");
qWarning("QEventDispatcherUNIX::unregisterTimer: timers cannot be stopped from another thread");
return false;
}
#endif
@ -376,7 +376,7 @@ bool QEventDispatcherUNIX::unregisterTimers(QObject *object)
qWarning("QEventDispatcherUNIX::unregisterTimers: invalid argument");
return false;
} else if (object->thread() != thread() || thread() != QThread::currentThread()) {
qWarning("QObject::killTimers: timers cannot be stopped from another thread");
qWarning("QEventDispatcherUNIX::unregisterTimers: timers cannot be stopped from another thread");
return false;
}
#endif

View File

@ -908,13 +908,15 @@ void QEventDispatcherWin32::unregisterSocketNotifier(QSocketNotifier *notifier)
void QEventDispatcherWin32::registerTimer(int timerId, int interval, Qt::TimerType timerType, QObject *object)
{
#ifndef QT_NO_DEBUG
if (timerId < 1 || interval < 0 || !object) {
qWarning("QEventDispatcherWin32::registerTimer: invalid arguments");
return;
} else if (object->thread() != thread() || thread() != QThread::currentThread()) {
qWarning("QObject::startTimer: timers cannot be started from another thread");
qWarning("QEventDispatcherWin32::registerTimer: timers cannot be started from another thread");
return;
}
#endif
Q_D(QEventDispatcherWin32);
@ -936,15 +938,17 @@ void QEventDispatcherWin32::registerTimer(int timerId, int interval, Qt::TimerTy
bool QEventDispatcherWin32::unregisterTimer(int timerId)
{
#ifndef QT_NO_DEBUG
if (timerId < 1) {
qWarning("QEventDispatcherWin32::unregisterTimer: invalid argument");
return false;
}
QThread *currentThread = QThread::currentThread();
if (thread() != currentThread) {
qWarning("QObject::killTimer: timers cannot be stopped from another thread");
qWarning("QEventDispatcherWin32::unregisterTimer: timers cannot be stopped from another thread");
return false;
}
#endif
Q_D(QEventDispatcherWin32);
if (d->timerVec.isEmpty() || timerId <= 0)
@ -962,15 +966,17 @@ bool QEventDispatcherWin32::unregisterTimer(int timerId)
bool QEventDispatcherWin32::unregisterTimers(QObject *object)
{
#ifndef QT_NO_DEBUG
if (!object) {
qWarning("QEventDispatcherWin32::unregisterTimers: invalid argument");
return false;
}
QThread *currentThread = QThread::currentThread();
if (object->thread() != thread() || thread() != currentThread) {
qWarning("QObject::killTimers: timers cannot be stopped from another thread");
qWarning("QEventDispatcherWin32::unregisterTimers: timers cannot be stopped from another thread");
return false;
}
#endif
Q_D(QEventDispatcherWin32);
if (d->timerVec.isEmpty())

View File

@ -254,13 +254,15 @@ void QEventDispatcherWinRT::registerTimer(int timerId, int interval, Qt::TimerTy
{
Q_UNUSED(timerType);
#ifndef QT_NO_DEBUG
if (timerId < 1 || interval < 0 || !object) {
qWarning("QEventDispatcherWinRT::registerTimer: invalid arguments");
return;
} else if (object->thread() != thread() || thread() != QThread::currentThread()) {
qWarning("QObject::startTimer: timers cannot be started from another thread");
qWarning("QEventDispatcherWinRT::registerTimer: timers cannot be started from another thread");
return;
}
#endif
Q_D(QEventDispatcherWinRT);
@ -289,14 +291,16 @@ void QEventDispatcherWinRT::registerTimer(int timerId, int interval, Qt::TimerTy
bool QEventDispatcherWinRT::unregisterTimer(int timerId)
{
#ifndef QT_NO_DEBUG
if (timerId < 1) {
qWarning("QEventDispatcherWinRT::unregisterTimer: invalid argument");
return false;
}
if (thread() != QThread::currentThread()) {
qWarning("QObject::killTimer: timers cannot be stopped from another thread");
qWarning("QEventDispatcherWinRT::unregisterTimer: timers cannot be stopped from another thread");
return false;
}
#endif
Q_D(QEventDispatcherWinRT);
@ -310,15 +314,17 @@ bool QEventDispatcherWinRT::unregisterTimer(int timerId)
bool QEventDispatcherWinRT::unregisterTimers(QObject *object)
{
#ifndef QT_NO_DEBUG
if (!object) {
qWarning("QEventDispatcherWinRT::unregisterTimers: invalid argument");
return false;
}
QThread *currentThread = QThread::currentThread();
if (object->thread() != thread() || thread() != currentThread) {
qWarning("QObject::killTimers: timers cannot be stopped from another thread");
qWarning("QEventDispatcherWinRT::unregisterTimers: timers cannot be stopped from another thread");
return false;
}
#endif
Q_D(QEventDispatcherWinRT);
for (QHash<int, WinRTTimerInfo *>::iterator it = d->timerDict.begin(); it != d->timerDict.end();) {

View File

@ -225,13 +225,17 @@ QObjectPrivate::QObjectPrivate(int version)
QObjectPrivate::~QObjectPrivate()
{
if (extraData && !extraData->runningTimers.isEmpty()) {
// unregister pending timers
if (threadData->eventDispatcher.load())
threadData->eventDispatcher.load()->unregisterTimers(q_ptr);
if (Q_LIKELY(threadData->thread == QThread::currentThread())) {
// unregister pending timers
if (threadData->eventDispatcher.load())
threadData->eventDispatcher.load()->unregisterTimers(q_ptr);
// release the timer ids back to the pool
for (int i = 0; i < extraData->runningTimers.size(); ++i)
QAbstractEventDispatcherPrivate::releaseTimerId(extraData->runningTimers.at(i));
// release the timer ids back to the pool
for (int i = 0; i < extraData->runningTimers.size(); ++i)
QAbstractEventDispatcherPrivate::releaseTimerId(extraData->runningTimers.at(i));
} else {
qWarning("QObject::~QObject: Timers cannot be stopped from another thread");
}
}
if (postedEvents)
@ -1612,15 +1616,18 @@ int QObject::startTimer(int interval, Qt::TimerType timerType)
{
Q_D(QObject);
if (interval < 0) {
if (Q_UNLIKELY(interval < 0)) {
qWarning("QObject::startTimer: Timers cannot have negative intervals");
return 0;
}
if (!d->threadData->eventDispatcher.load()) {
if (Q_UNLIKELY(!d->threadData->eventDispatcher.load())) {
qWarning("QObject::startTimer: Timers can only be used with threads started with QThread");
return 0;
}
if (Q_UNLIKELY(thread() != QThread::currentThread())) {
qWarning("QObject::startTimer: Timers cannot be started from another thread");
return 0;
}
int timerId = d->threadData->eventDispatcher.load()->registerTimer(interval, timerType, this);
if (!d->extraData)
d->extraData = new QObjectPrivate::ExtraData;
@ -1640,6 +1647,10 @@ int QObject::startTimer(int interval, Qt::TimerType timerType)
void QObject::killTimer(int id)
{
Q_D(QObject);
if (Q_UNLIKELY(thread() != QThread::currentThread())) {
qWarning("QObject::killTimer: Timers cannot be stopped from another thread");
return;
}
if (id) {
int at = d->extraData ? d->extraData->runningTimers.indexOf(id) : -1;
if (at == -1) {

View File

@ -274,6 +274,10 @@ void QSocketNotifier::setEnabled(bool enable)
if (!d->threadData->eventDispatcher.load()) // perhaps application/thread is shutting down
return;
if (Q_UNLIKELY(thread() != QThread::currentThread())) {
qWarning("QSocketNotifier: Socket notifiers cannot be enabled or disabled from another thread");
return;
}
if (d->snenabled)
d->threadData->eventDispatcher.load()->registerSocketNotifier(this);
else

View File

@ -395,6 +395,10 @@ void QTimer::singleShot(int msec, const QObject *receiver, const char *member)
*/
void QTimer::singleShot(int msec, Qt::TimerType timerType, const QObject *receiver, const char *member)
{
if (Q_UNLIKELY(msec < 0)) {
qWarning("QTimer::singleShot: Timers cannot have negative timeouts");
return;
}
if (receiver && member) {
if (msec == 0) {
// special code shortpath for 0-timers

View File

@ -140,11 +140,11 @@ QWinEventNotifier::QWinEventNotifier(HANDLE hEvent, QObject *parent)
{
Q_D(QWinEventNotifier);
QAbstractEventDispatcher *eventDispatcher = d->threadData->eventDispatcher.load();
if (!eventDispatcher) {
if (Q_UNLIKELY(!eventDispatcher)) {
qWarning("QWinEventNotifier: Can only be used with threads started with QThread");
} else {
eventDispatcher->registerEventNotifier(this);
return;
}
eventDispatcher->registerEventNotifier(this);
d->enabled = true;
}
@ -215,6 +215,10 @@ void QWinEventNotifier::setEnabled(bool enable)
QAbstractEventDispatcher *eventDispatcher = d->threadData->eventDispatcher.load();
if (!eventDispatcher) // perhaps application is shutting down
return;
if (Q_UNLIKELY(thread() != QThread::currentThread())) {
qWarning("QWinEventNotifier: Event notifiers cannot be enabled or disabled from another thread");
return;
}
if (enable)
eventDispatcher->registerEventNotifier(this);