1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QFutureWatcher: Fix race for initial emission of resultReadyAt()

Browse Source 
When connecting a QFutureWatcher to the QFuture it will connect to the
output interface, which will queue up events to notify about the current
state. This happens in the thread of the QFutureWatcher.

Since 07d6d31a4c0c17d8c897d783a9b0841df6834b02 unfortunately the sending
of those events was done outside the lock, meaning the worker-thread
could _also_ send events at the same time, leading to a race on which
events would be sent first.

To fix this we move the emission of the events back into the lock
and because it is now inside the lock again anyway, we will revert
back to posting the callout events immediately, so this patch also
partially reverts 07d6d31a4c0c17d8c897d783a9b0841df6834b02

Fixes: QTBUG-119169
Pick-to: 6.6
Change-Id: If29ab6712a82e7948c0ea4866340b6fac5aba5ef
Reviewed-by: Arno Rehn <a.rehn@menlosystems.com>
Reviewed-by: Jarek Kobus <jaroslaw.kobus@qt.io>
(cherry picked from commit 63b2cf8a457f0eea861fa060c610a74b35450ba6)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Jarek Kobus 2023-11-15 15:04:01 +01:00 committed by Qt Cherry-pick Bot
parent e2e0bfd238
commit 933b606af5
2 changed files with 33 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
src/corelib/thread/qfutureinterface.cpp
View File

@ -806,23 +806,21 @@ void QFutureInterfaceBasePrivate::connectOutputInterface(QFutureCallOutInterface
{
QMutexLocker locker(&m_mutex);
QVarLengthArray<std::unique_ptr<QFutureCallOutEvent>, 3> events;
const auto currentState = state.loadRelaxed();
if (currentState & QFutureInterfaceBase::Started) {
events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::Started));
iface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::Started));
if (m_progress) {
events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::ProgressRange,
iface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::ProgressRange,
m_progress->minimum,
m_progress->maximum));
events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::Progress,
iface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::Progress,
m_progressValue,
m_progress->text));
} else {
events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::ProgressRange,
iface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::ProgressRange,
0,
0));
events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::Progress,
iface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::Progress,
m_progressValue,
QString()));
}
@ -833,7 +831,7 @@ void QFutureInterfaceBasePrivate::connectOutputInterface(QFutureCallOutInterface
while (it != data.m_results.end()) {
const int begin = it.resultIndex();
const int end = begin + it.batchSize();
events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::ResultsReady,
iface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::ResultsReady,
begin,
end));
it.batchedAdvance();
@ -841,21 +839,17 @@ void QFutureInterfaceBasePrivate::connectOutputInterface(QFutureCallOutInterface
}
if (currentState & QFutureInterfaceBase::Suspended)
events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::Suspended));
iface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::Suspended));
else if (currentState & QFutureInterfaceBase::Suspending)
events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::Suspending));
iface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::Suspending));
if (currentState & QFutureInterfaceBase::Canceled)
events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::Canceled));
iface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::Canceled));
if (currentState & QFutureInterfaceBase::Finished)
events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::Finished));
iface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::Finished));
outputConnections.append(iface);
locker.unlock();
for (auto &&event : events)
iface->postCallOutEvent(*event);
}
void QFutureInterfaceBasePrivate::disconnectOutputInterface(QFutureCallOutInterface *iface)

23
tests/auto/corelib/thread/qfuturewatcher/tst_qfuturewatcher.cpp
View File

@ -26,6 +26,7 @@ private slots:
void cancelAndFinish();
void resultAt();
void resultReadyAt();
void orderedResultReadyAt();
void futureSignals();
void watchFinishedFuture();
void watchCanceledFuture();
@ -280,6 +281,28 @@ void tst_QFutureWatcher::resultReadyAt()
QVERIFY(resultSpy.wait());
}
void tst_QFutureWatcher::orderedResultReadyAt()
{
for (int i = 0; i < 1000; ++i) {
QObject context;
QFuture<QString> f = run([](QPromise<QString> &fi) {
fi.addResult("First");
fi.addResult("Second");
});
QList<int> actualIndices;
QFutureWatcher<QString> watcher;
connect(&watcher, &QFutureWatcherBase::resultReadyAt, &context,
[&actualIndices](int index) { actualIndices.append(index); });
watcher.setFuture(f);
f.waitForFinished();
QCoreApplication::processEvents();
const QList<int> expectedIndices{0, 1};
QCOMPARE(actualIndices.size(), expectedIndices.size());
QCOMPARE(actualIndices, expectedIndices);
}
}
class SignalSlotObject : public QObject
{
Q_OBJECT