Browse Source

Fix incorrect use of QFutureWatcher

To avoid a race condition, it is important to call setFuture() after doing the connections.

See: https://doc.qt.io/qt-6/qfuturewatcher.html
pull/7056/head 1.4.0rc1-683-g320a1b81c
Jonas Kvinge 11 months ago
committed by John Maguire
parent
commit
320a1b81c9
  1. 4
      ext/libclementine-common/core/closure.h
  2. 2
      src/musicbrainz/tagfetcher.cpp
  3. 16
      tests/concurrentrun_test.cpp

4
ext/libclementine-common/core/closure.h

@ -202,8 +202,8 @@ template <typename T, typename... Args>
_detail::ClosureBase* NewClosure(QFuture<T> future, QObject* receiver,
const char* slot, const Args&... args) {
QFutureWatcher<T>* watcher = new QFutureWatcher<T>;
watcher->setFuture(future);
QObject::connect(watcher, SIGNAL(finished()), watcher, SLOT(deleteLater()));
watcher->setFuture(future);
return NewClosure(watcher, SIGNAL(finished()), receiver, slot, args...);
}
@ -211,8 +211,8 @@ template <typename T, typename F, typename... Args>
_detail::ClosureBase* NewClosure(QFuture<T> future, const F& callback,
const Args&... args) {
QFutureWatcher<T>* watcher = new QFutureWatcher<T>;
watcher->setFuture(future);
QObject::connect(watcher, SIGNAL(finished()), watcher, SLOT(deleteLater()));
watcher->setFuture(future);
return NewClosure(watcher, SIGNAL(finished()), callback, args...);
}

2
src/musicbrainz/tagfetcher.cpp

@ -50,9 +50,9 @@ void TagFetcher::StartFetch(const SongList& songs) {
QFuture<QString> future = QtConcurrent::mapped(songs_, GetFingerprint);
fingerprint_watcher_ = new QFutureWatcher<QString>(this);
fingerprint_watcher_->setFuture(future);
connect(fingerprint_watcher_, SIGNAL(resultReadyAt(int)),
SLOT(FingerprintFound(int)));
fingerprint_watcher_->setFuture(future);
for (const Song& song : songs) {
emit Progress(song, tr("Fingerprinting song"));

16
tests/concurrentrun_test.cpp

@ -17,9 +17,9 @@ TEST(ConcurrentRunTest, ConcurrentRun0StartAndWait) {
QThreadPool threadpool;
QFuture<int> future = ConcurrentRun::Run<int>(&threadpool, &f);
QFutureWatcher<int> watcher;
watcher.setFuture(future);
QEventLoop loop;
QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit()));
watcher.setFuture(future);
loop.exec();
EXPECT_EQ(1337, watcher.result());
}
@ -33,9 +33,9 @@ TEST(ConcurrentRunTest, ConcurrentRun1StartAndWait) {
int i = 1336;
QFuture<int> future = ConcurrentRun::Run<int, int>(&threadpool, &g, i);
QFutureWatcher<int> watcher;
watcher.setFuture(future);
QEventLoop loop;
QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit()));
watcher.setFuture(future);
loop.exec();
EXPECT_EQ(1337, watcher.result());
}
@ -50,9 +50,9 @@ TEST(ConcurrentRunTest, ConcurrentRun2StartAndWait) {
QThreadPool threadpool;
QFuture<int> future = ConcurrentRun::Run<int, int, int>(&threadpool, &max, i, j);
QFutureWatcher<int> watcher;
watcher.setFuture(future);
QEventLoop loop;
QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit()));
watcher.setFuture(future);
loop.exec();
EXPECT_EQ(42, watcher.result());
}
@ -68,9 +68,9 @@ TEST(ConcurrentRunTest, ConcurrentRun3StartAndWait) {
QThreadPool threadpool;
QFuture<int> future = ConcurrentRun::Run<int, int, int, int>(&threadpool, &sum, i, j, k);
QFutureWatcher<int> watcher;
watcher.setFuture(future);
QEventLoop loop;
QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit()));
watcher.setFuture(future);
loop.exec();
EXPECT_EQ(102, watcher.result());
}
@ -95,9 +95,9 @@ TEST(ConcurrentRunTest, ConcurrentRunVoidFunction1Start) {
int n = 10;
QFuture<void> future = ConcurrentRun::Run<void, int*>(&threadpool, &aFunction, &n);
QFutureWatcher<void> watcher;
watcher.setFuture(future);
QEventLoop loop;
QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit()));
watcher.setFuture(future);
loop.exec();
EXPECT_EQ(1337, n);
}
@ -108,9 +108,9 @@ TEST(ConcurrentRunTest, ConcurrentRunVoidFunction2Start) {
int n = 10, m = 11;
QFuture<void> future = ConcurrentRun::Run<void, int*, int*>(&threadpool, &bFunction, &n, &m);
QFutureWatcher<void> watcher;
watcher.setFuture(future);
QEventLoop loop;
QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit()));
watcher.setFuture(future);
loop.exec();
EXPECT_EQ(1337, n);
EXPECT_EQ(1338, m);
@ -122,9 +122,9 @@ TEST(ConcurrentRunTest, ConcurrentRunVoidFunction3Start) {
int n = 10, m = 11, o = 12;
QFuture<void> future = ConcurrentRun::Run<void, int*, int*, int*>(&threadpool, &cFunction, &n, &m, &o);
QFutureWatcher<void> watcher;
watcher.setFuture(future);
QEventLoop loop;
QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit()));
watcher.setFuture(future);
loop.exec();
EXPECT_EQ(1337, n);
EXPECT_EQ(1338, m);
@ -145,9 +145,9 @@ TEST(ConcurrentRunTest, ConcurrentRunVoidBindFunctionStart) {
int nb = 10;
QFuture<void> future = ConcurrentRun::Run<void>(&threadpool, std::bind(&A::f, &a, &nb));
QFutureWatcher<void> watcher;
watcher.setFuture(future);
QEventLoop loop;
QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit()));
watcher.setFuture(future);
loop.exec();
EXPECT_EQ(11, nb);
}

Loading…
Cancel
Save