Skip to content

Commit

Permalink
Windows: fix DeferredDelete events processing on QThread::terminate()
Browse files Browse the repository at this point in the history
On finishing/terminating a thread, when processing posted events,
we need to consider QThread's own data instead of caller thread's data.
Otherwise we can get into unexpected situations such as double
destruction of an object, premature destruction, etc.

Fixes: QTBUG-103922
Pick-to: 6.4 6.3 6.3.1 6.2 5.15
Change-Id: Idf77221ebbaa0b150ee2d0c296b51829ae8dc30e
Reviewed-by: Thiago Macieira <[email protected]>
  • Loading branch information
VladimirBelyavsky authored and vohi committed Jun 10, 2022
1 parent e2f14e5 commit 8652120
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 1 deletion.
13 changes: 12 additions & 1 deletion src/corelib/thread/qthread_win.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,17 @@ unsigned int __stdcall QT_ENSURE_STACK_ALIGNED_FOR_SSE QThreadPrivate::start(voi
return 0;
}

/*
For regularly terminating threads, this will be called and executed by the thread as the
last code before the thread exits. In that case, \a arg is the current QThread.
However, this function will also be called by QThread::terminate (as well as wait() and
setTerminationEnabled) to give Qt a chance to update the terminated thread's state and
process pending DeleteLater events for objects that live in the terminated thread. And for
adopted thread, this method is called by the thread watcher.
In those cases, \a arg will not be the current thread.
*/
void QThreadPrivate::finish(void *arg, bool lockAnyway) noexcept
{
QThread *thr = reinterpret_cast<QThread *>(arg);
Expand All @@ -305,7 +316,7 @@ void QThreadPrivate::finish(void *arg, bool lockAnyway) noexcept
if (lockAnyway)
locker.unlock();
emit thr->finished(QThread::QPrivateSignal());
QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
QCoreApplicationPrivate::sendPostedEvents(nullptr, QEvent::DeferredDelete, d->data);
QThreadStorageData::finish(tls_data);
if (lockAnyway)
locker.relock();
Expand Down
78 changes: 78 additions & 0 deletions tests/auto/corelib/thread/qthread/tst_qthread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ private slots:
void create();
void createDestruction();
void threadIdReuse();

void terminateAndPrematureDestruction();
void terminateAndDoubleDestruction();
};

enum { one_minute = 60 * 1000, five_minutes = 5 * one_minute };
Expand Down Expand Up @@ -1719,5 +1722,80 @@ void tst_QThread::threadIdReuse()
}
}

class WaitToRun_Thread : public QThread
{
Q_OBJECT
public:
void run() override
{
emit running();
QThread::exec();
}

Q_SIGNALS:
void running();
};


void tst_QThread::terminateAndPrematureDestruction()
{
WaitToRun_Thread thread;
QSignalSpy spy(&thread, &WaitToRun_Thread::running);
thread.start();
QVERIFY(spy.wait(500));

QScopedPointer<QObject> obj(new QObject);
QPointer<QObject> pObj(obj.data());
obj->deleteLater();

thread.terminate();
QVERIFY2(pObj, "object was deleted prematurely!");
thread.wait(500);
}

void tst_QThread::terminateAndDoubleDestruction()
{
class ChildObject : public QObject
{
public:
ChildObject(QObject *parent)
: QObject(parent)
{
QSignalSpy spy(&thread, &WaitToRun_Thread::running);
thread.start();
spy.wait(500);
}

~ChildObject()
{
QVERIFY2(!inDestruction, "Double object destruction!");
inDestruction = true;
thread.terminate();
thread.wait(500);
}

bool inDestruction = false;
WaitToRun_Thread thread;
};

class TestObject : public QObject
{
public:
TestObject()
: child(new ChildObject(this))
{
}

~TestObject()
{
child->deleteLater();
}

ChildObject *child = nullptr;
};

TestObject obj;
}

QTEST_MAIN(tst_QThread)
#include "tst_qthread.moc"

0 comments on commit 8652120

Please sign in to comment.