From 96a7596a904529b1ceabf3552aab2a280c19fbd3 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Fri, 18 Jan 2013 14:39:00 +0800 Subject: Clear the current thread data for the main thread This avoids crashes accessing deleted memory when creating a QObject after the last QObject had been deleted, like a qDebug() in global destructors. ==41000== Invalid read of size 4 ==41000== at 0x5F01ED5: bool QBasicAtomicOps<4>::ref(int&) (qatomic_x86.h:208) ==41000== by 0x5F01309: QBasicAtomicInteger::ref() (qbasicatomic.h:147) ==41000== by 0x5F24051: QThreadData::ref() (qthread.cpp:100) ==41000== by 0x614A984: QObject::QObject(QObject*) (qobject.cpp:681) ==41000== Address 0x6ee73f0 is 0 bytes inside a block of size 152 free'd ==41000== at 0x4A0736C: operator delete(void*) (vg_replace_malloc.c:480) ==41000== by 0x5F240BF: QThreadData::deref() (qthread.cpp:109) ==41000== by 0x6113F6B: QCoreApplicationData::~QCoreApplicationData() (qcoreapplication.cpp:268) The comment right above the change in qthread.cpp looks eerily similar to the problem I'm trying to fix. However, the actual change that introduced the change is not in the Qt public history, so we can't know for sure what the problem was then. Cherry-picked from qtbase/950b35cf97ad398f97883efd2a18ee97994a8a9c. Change-Id: Ic4072c15529e2ae94ea36fbd0340cf5ee61413d2 Reviewed-by: Thiago Macieira --- src/corelib/kernel/qcoreapplication.cpp | 7 ------- src/corelib/thread/qthread.cpp | 1 + src/corelib/thread/qthread_p.h | 1 + src/corelib/thread/qthread_unix.cpp | 5 +++++ src/corelib/thread/qthread_win.cpp | 5 +++++ tests/auto/qcoreapplication/tst_qcoreapplication.cpp | 13 +++++++++++++ 6 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp index b8b639d..8ed8f56 100644 --- a/src/corelib/kernel/qcoreapplication.cpp +++ b/src/corelib/kernel/qcoreapplication.cpp @@ -373,13 +373,6 @@ struct QCoreApplicationData { #ifndef QT_NO_LIBRARY delete app_libpaths; #endif - - // cleanup the QAdoptedThread created for the main() thread - if (QCoreApplicationPrivate::theMainThread) { - QThreadData *data = QThreadData::get2(QCoreApplicationPrivate::theMainThread); - QCoreApplicationPrivate::theMainThread = 0; - data->deref(); // deletes the data and the adopted thread - } } #ifdef Q_OS_BLACKBERRY diff --git a/src/corelib/thread/qthread.cpp b/src/corelib/thread/qthread.cpp index 74bbd4e..7ebbc83 100644 --- a/src/corelib/thread/qthread.cpp +++ b/src/corelib/thread/qthread.cpp @@ -95,6 +95,7 @@ QThreadData::~QThreadData() // the problem... if (this->thread == QCoreApplicationPrivate::theMainThread) { QCoreApplicationPrivate::theMainThread = 0; + QThreadData::clearCurrentThreadData(); } QThread *t = thread; diff --git a/src/corelib/thread/qthread_p.h b/src/corelib/thread/qthread_p.h index 49cb80e..d404b53 100644 --- a/src/corelib/thread/qthread_p.h +++ b/src/corelib/thread/qthread_p.h @@ -215,6 +215,7 @@ public: ~QThreadData(); static QThreadData *current(); + static void clearCurrentThreadData(); static QThreadData *get2(QThread *thread) { Q_ASSERT_X(thread != 0, "QThread", "internal error"); return thread->d_func()->data; } diff --git a/src/corelib/thread/qthread_unix.cpp b/src/corelib/thread/qthread_unix.cpp index d66d7ec..57127a2 100644 --- a/src/corelib/thread/qthread_unix.cpp +++ b/src/corelib/thread/qthread_unix.cpp @@ -203,6 +203,11 @@ static void clear_thread_data() pthread_setspecific(current_thread_data_key, 0); } +void QThreadData::clearCurrentThreadData() +{ + clear_thread_data(); +} + QThreadData *QThreadData::current() { QThreadData *data = get_thread_data(); diff --git a/src/corelib/thread/qthread_win.cpp b/src/corelib/thread/qthread_win.cpp index 3b78502..98a7f81 100644 --- a/src/corelib/thread/qthread_win.cpp +++ b/src/corelib/thread/qthread_win.cpp @@ -96,6 +96,11 @@ Q_DESTRUCTOR_FUNCTION(qt_free_tls) /* QThreadData */ +void QThreadData::clearCurrentThreadData() +{ + TlsSetValue(qt_current_thread_data_tls_index, 0); +} + QThreadData *QThreadData::current() { qt_create_tls(); diff --git a/tests/auto/qcoreapplication/tst_qcoreapplication.cpp b/tests/auto/qcoreapplication/tst_qcoreapplication.cpp index 472c216..9eb53ee 100644 --- a/tests/auto/qcoreapplication/tst_qcoreapplication.cpp +++ b/tests/auto/qcoreapplication/tst_qcoreapplication.cpp @@ -577,5 +577,18 @@ void tst_QCoreApplication::eventLoopExecAfterExit() QCOMPARE(loop.exec(), 0); } +static void createQObjectOnDestruction() +{ + // Make sure that we can create a QObject after the last QObject has been + // destroyed (especially after QCoreApplication has). + // + // Before the fixes, this would cause a dangling pointer dereference. If + // the problem comes back, it's possible that the following causes no + // effect. + QObject obj; + obj.thread()->setProperty("testing", 1); +} +Q_DESTRUCTOR_FUNCTION(createQObjectOnDestruction) + QTEST_APPLESS_MAIN(tst_QCoreApplication) #include "tst_qcoreapplication.moc" -- cgit v0.12