aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/qml/qml/qqmlengine.cpp6
-rw-r--r--src/qml/qml/qqmlengine_p.h2
-rw-r--r--src/quick/util/qquickimageprovider_p.h2
-rw-r--r--src/quick/util/qquickpixmapcache.cpp34
-rw-r--r--tests/auto/quick/qquickimageprovider/tst_qquickimageprovider.cpp181
5 files changed, 192 insertions, 33 deletions
diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp
index b94f449a39..5e06ff283e 100644
--- a/src/qml/qml/qqmlengine.cpp
+++ b/src/qml/qml/qqmlengine.cpp
@@ -1169,6 +1169,12 @@ void QQmlEnginePrivate::registerFinalizeCallback(QObject *obj, int index)
}
}
+QSharedPointer<QQmlImageProviderBase> QQmlEnginePrivate::imageProvider(const QString &providerId) const
+{
+ QMutexLocker locker(&mutex);
+ return imageProviders.value(providerId.toLower());
+}
+
#if QT_CONFIG(qml_network)
/*!
Sets the \a factory to use for creating QNetworkAccessManager(s).
diff --git a/src/qml/qml/qqmlengine_p.h b/src/qml/qml/qqmlengine_p.h
index 3b716683fd..e58ff554a7 100644
--- a/src/qml/qml/qqmlengine_p.h
+++ b/src/qml/qml/qqmlengine_p.h
@@ -167,6 +167,8 @@ public:
mutable QQmlNetworkAccessManagerFactory *networkAccessManagerFactory;
#endif
QHash<QString,QSharedPointer<QQmlImageProviderBase> > imageProviders;
+ QSharedPointer<QQmlImageProviderBase> imageProvider(const QString &providerId) const;
+
QQmlAbstractUrlInterceptor* urlInterceptor;
diff --git a/src/quick/util/qquickimageprovider_p.h b/src/quick/util/qquickimageprovider_p.h
index b5baf79319..67e17010d4 100644
--- a/src/quick/util/qquickimageprovider_p.h
+++ b/src/quick/util/qquickimageprovider_p.h
@@ -61,7 +61,7 @@ class QQuickImageResponsePrivate : public QObjectPrivate
{
Q_DECLARE_PUBLIC(QQuickImageResponse)
public:
- bool finished = false;
+ QAtomicInteger<qint32> finished = false;
void _q_finished() { finished = true; }
};
diff --git a/src/quick/util/qquickpixmapcache.cpp b/src/quick/util/qquickpixmapcache.cpp
index ced0acd9ab..5f31376599 100644
--- a/src/quick/util/qquickpixmapcache.cpp
+++ b/src/quick/util/qquickpixmapcache.cpp
@@ -207,7 +207,7 @@ protected:
private:
friend class QQuickPixmapReaderThreadObject;
void processJobs();
- void processJob(QQuickPixmapReply *, const QUrl &, const QString &, QQuickImageProvider::ImageType, QQuickImageProvider *);
+ void processJob(QQuickPixmapReply *, const QUrl &, const QString &, QQuickImageProvider::ImageType, const QSharedPointer<QQuickImageProvider> &);
#if QT_CONFIG(qml_network)
void networkRequestDone(QNetworkReply *);
#endif
@@ -683,10 +683,11 @@ void QQuickPixmapReader::processJobs()
const QUrl url = job->url;
QString localFile;
QQuickImageProvider::ImageType imageType = QQuickImageProvider::Invalid;
- QQuickImageProvider *provider = nullptr;
+ QSharedPointer<QQuickImageProvider> provider;
if (url.scheme() == QLatin1String("image")) {
- provider = static_cast<QQuickImageProvider *>(engine->imageProvider(imageProviderId(url)));
+ QQmlEnginePrivate *enginePrivate = QQmlEnginePrivate::get(engine);
+ provider = enginePrivate->imageProvider(imageProviderId(url)).staticCast<QQuickImageProvider>();
if (provider)
imageType = provider->imageType();
@@ -721,7 +722,7 @@ void QQuickPixmapReader::processJobs()
}
void QQuickPixmapReader::processJob(QQuickPixmapReply *runningJob, const QUrl &url, const QString &localFile,
- QQuickImageProvider::ImageType imageType, QQuickImageProvider *provider)
+ QQuickImageProvider::ImageType imageType, const QSharedPointer<QQuickImageProvider> &provider)
{
// fetch
if (url.scheme() == QLatin1String("image")) {
@@ -737,7 +738,8 @@ void QQuickPixmapReader::processJob(QQuickPixmapReply *runningJob, const QUrl &u
return;
}
- QQuickImageProviderWithOptions *providerV2 = QQuickImageProviderWithOptions::checkedCast(provider);
+ // This is safe because we ensure that provider does outlive providerV2 and it does not escape the function
+ QQuickImageProviderWithOptions *providerV2 = QQuickImageProviderWithOptions::checkedCast(provider.get());
switch (imageType) {
case QQuickImageProvider::Invalid:
@@ -817,17 +819,24 @@ void QQuickPixmapReader::processJob(QQuickPixmapReply *runningJob, const QUrl &u
if (providerV2) {
response = providerV2->requestImageResponse(imageId(url), runningJob->requestSize, runningJob->providerOptions);
} else {
- QQuickAsyncImageProvider *asyncProvider = static_cast<QQuickAsyncImageProvider*>(provider);
+ QQuickAsyncImageProvider *asyncProvider = static_cast<QQuickAsyncImageProvider*>(provider.get());
response = asyncProvider->requestImageResponse(imageId(url), runningJob->requestSize);
}
+ {
+ QObject::connect(response, SIGNAL(finished()), threadObject, SLOT(asyncResponseFinished()));
+ // as the response object can outlive the provider QSharedPointer, we have to extend the pointee's lifetime by that of the response
+ // we do this by capturing a copy of the QSharedPointer in a lambda, and dropping it once the lambda has been called
+ auto provider_copy = provider; // capturing provider would capture it as a const reference, and copy capture with initializer is only available in C++14
+ QObject::connect(response, &QQuickImageResponse::destroyed, response, [provider_copy]() {
+ // provider_copy will be deleted when the connection gets deleted
+ });
+ }
// Might be that the async provider was so quick it emitted the signal before we
// could connect to it.
if (static_cast<QQuickImageResponsePrivate*>(QObjectPrivate::get(response))->finished) {
QMetaObject::invokeMethod(threadObject, "asyncResponseFinished",
Qt::QueuedConnection, Q_ARG(QQuickImageResponse*, response));
- } else {
- QObject::connect(response, SIGNAL(finished()), threadObject, SLOT(asyncResponseFinished()));
}
asyncResponses.insert(response, runningJob);
@@ -1270,8 +1279,10 @@ static QQuickPixmapData* createPixmapDataSync(QQuickPixmap *declarativePixmap, Q
QSize readSize;
QQuickImageProvider::ImageType imageType = QQuickImageProvider::Invalid;
- QQuickImageProvider *provider = static_cast<QQuickImageProvider *>(engine->imageProvider(imageProviderId(url)));
- QQuickImageProviderWithOptions *providerV2 = QQuickImageProviderWithOptions::checkedCast(provider);
+ QQmlEnginePrivate *enginePrivate = QQmlEnginePrivate::get(engine);
+ QSharedPointer<QQuickImageProvider> provider = enginePrivate->imageProvider(imageProviderId(url)).dynamicCast<QQuickImageProvider>();
+ // it is safe to use get() as providerV2 does not escape and is outlived by provider
+ QQuickImageProviderWithOptions *providerV2 = QQuickImageProviderWithOptions::checkedCast(provider.get());
if (provider)
imageType = provider->imageType();
@@ -1570,7 +1581,8 @@ void QQuickPixmap::load(QQmlEngine *engine, const QUrl &url, const QSize &reques
if (iter == store->m_cache.end()) {
if (url.scheme() == QLatin1String("image")) {
- if (QQuickImageProvider *provider = static_cast<QQuickImageProvider *>(engine->imageProvider(imageProviderId(url)))) {
+ QQmlEnginePrivate *enginePrivate = QQmlEnginePrivate::get(engine);
+ if (auto provider = enginePrivate->imageProvider(imageProviderId(url)).staticCast<QQuickImageProvider>()) {
const bool threadedPixmaps = QGuiApplicationPrivate::platformIntegration()->hasCapability(QPlatformIntegration::ThreadedPixmaps);
if (!threadedPixmaps && provider->imageType() == QQuickImageProvider::Pixmap) {
// pixmaps can only be loaded synchronously
diff --git a/tests/auto/quick/qquickimageprovider/tst_qquickimageprovider.cpp b/tests/auto/quick/qquickimageprovider/tst_qquickimageprovider.cpp
index 4b75a7e008..c79e665d94 100644
--- a/tests/auto/quick/qquickimageprovider/tst_qquickimageprovider.cpp
+++ b/tests/auto/quick/qquickimageprovider/tst_qquickimageprovider.cpp
@@ -33,6 +33,7 @@
#include <QImageReader>
#include <QWaitCondition>
#include <QThreadPool>
+#include <private/qqmlengine_p.h>
Q_DECLARE_METATYPE(QQuickImageProvider*);
@@ -67,6 +68,8 @@ private slots:
void asyncTextureTest();
void instantAsyncTextureTest();
+ void asyncImageThreadSafety();
+
private:
QString newImageFileName() const;
void fillRequestTestsData(const QString &id);
@@ -448,21 +451,56 @@ void tst_qquickimageprovider::threadTest()
foreach (QQuickImage *img, images) {
QCOMPARE(img->status(), QQuickImage::Loading);
}
- provider->ok = true;
- provider->cond.wakeAll();
+ {
+ QMutexLocker lock(&provider->mutex);
+ provider->ok = true;
+ provider->cond.wakeAll();
+ }
QTest::qWait(250);
foreach (QQuickImage *img, images) {
QTRY_COMPARE(img->status(), QQuickImage::Ready);
}
}
-class TestImageResponse : public QQuickImageResponse, public QRunnable
+class TestImageResponseRunner : public QObject, public QRunnable {
+
+ Q_OBJECT
+
+public:
+ Q_SIGNAL void finished(QQuickTextureFactory *texture);
+ TestImageResponseRunner(QMutex *lock, QWaitCondition *condition, bool *ok, const QString &id, const QSize &requestedSize)
+ : m_lock(lock), m_condition(condition), m_ok(ok), m_id(id), m_requestedSize(requestedSize) {}
+ void run()
+ {
+ m_lock->lock();
+ if (!(*m_ok)) {
+ m_condition->wait(m_lock);
+ }
+ m_lock->unlock();
+ QImage image(50, 50, QImage::Format_RGB32);
+ image.fill(QColor(m_id).rgb());
+ if (m_requestedSize.isValid())
+ image = image.scaled(m_requestedSize);
+ emit finished(QQuickTextureFactory::textureFactoryForImage(image));
+ }
+
+private:
+ QMutex *m_lock;
+ QWaitCondition *m_condition;
+ bool *m_ok;
+ QString m_id;
+ QSize m_requestedSize;
+};
+
+class TestImageResponse : public QQuickImageResponse
{
public:
- TestImageResponse(QMutex *lock, QWaitCondition *condition, bool *ok, const QString &id, const QSize &requestedSize)
+ TestImageResponse(QMutex *lock, QWaitCondition *condition, bool *ok, const QString &id, const QSize &requestedSize, QThreadPool *pool)
: m_lock(lock), m_condition(condition), m_ok(ok), m_id(id), m_requestedSize(requestedSize), m_texture(nullptr)
{
- setAutoDelete(false);
+ auto runnable = new TestImageResponseRunner(m_lock, m_condition, m_ok, m_id, m_requestedSize);
+ QObject::connect(runnable, &TestImageResponseRunner::finished, this, &TestImageResponse::handleResponse);
+ pool->start(runnable);
}
QQuickTextureFactory *textureFactory() const
@@ -470,18 +508,8 @@ class TestImageResponse : public QQuickImageResponse, public QRunnable
return m_texture;
}
- void run()
- {
- m_lock->lock();
- if (!(*m_ok)) {
- m_condition->wait(m_lock);
- }
- m_lock->unlock();
- QImage image(50, 50, QImage::Format_RGB32);
- image.fill(QColor(m_id).rgb());
- if (m_requestedSize.isValid())
- image = image.scaled(m_requestedSize);
- m_texture = QQuickTextureFactory::textureFactoryForImage(image);
+ void handleResponse(QQuickTextureFactory *factory) {
+ this->m_texture = factory;
emit finished();
}
@@ -505,8 +533,7 @@ class TestAsyncProvider : public QQuickAsyncImageProvider
QQuickImageResponse *requestImageResponse(const QString &id, const QSize &requestedSize)
{
- TestImageResponse *response = new TestImageResponse(&lock, &condition, &ok, id, requestedSize);
- pool.start(response);
+ TestImageResponse *response = new TestImageResponse(&lock, &condition, &ok, id, requestedSize, &pool);
return response;
}
@@ -544,8 +571,11 @@ void tst_qquickimageprovider::asyncTextureTest()
foreach (QQuickImage *img, images) {
QTRY_COMPARE(img->status(), QQuickImage::Loading);
}
- provider->ok = true;
- provider->condition.wakeAll();
+ {
+ QMutexLocker lock(&provider->lock);
+ provider->ok = true;
+ provider->condition.wakeAll();
+ }
foreach (QQuickImage *img, images) {
QTRY_COMPARE(img->status(), QQuickImage::Ready);
}
@@ -616,6 +646,115 @@ void tst_qquickimageprovider::instantAsyncTextureTest()
}
+class WaitingAsyncImageResponse : public QQuickImageResponse, public QRunnable
+{
+public:
+ WaitingAsyncImageResponse(QMutex *providerRemovedMutex, QWaitCondition *providerRemovedCond, bool *providerRemoved, QMutex *imageRequestedMutex, QWaitCondition *imageRequestedCond, bool *imageRequested)
+ : m_providerRemovedMutex(providerRemovedMutex), m_providerRemovedCond(providerRemovedCond), m_providerRemoved(providerRemoved),
+ m_imageRequestedMutex(imageRequestedMutex), m_imageRequestedCondition(imageRequestedCond), m_imageRequested(imageRequested)
+ {
+ setAutoDelete(false);
+ }
+
+ void run() override
+ {
+ m_imageRequestedMutex->lock();
+ *m_imageRequested = true;
+ m_imageRequestedCondition->wakeAll();
+ m_imageRequestedMutex->unlock();
+ m_providerRemovedMutex->lock();
+ while (!*m_providerRemoved)
+ m_providerRemovedCond->wait(m_providerRemovedMutex);
+ m_providerRemovedMutex->unlock();
+ emit finished();
+ }
+
+ QQuickTextureFactory *textureFactory() const
+ {
+ QImage image(50, 50, QImage::Format_RGB32);
+ auto texture = QQuickTextureFactory::textureFactoryForImage(image);
+ return texture;
+ }
+
+ QMutex *m_providerRemovedMutex;
+ QWaitCondition *m_providerRemovedCond;
+ bool *m_providerRemoved;
+ QMutex *m_imageRequestedMutex;
+ QWaitCondition *m_imageRequestedCondition;
+ bool *m_imageRequested;
+
+};
+
+class WaitingAsyncProvider : public QQuickAsyncImageProvider
+{
+public:
+ WaitingAsyncProvider(QMutex *providerRemovedMutex, QWaitCondition *providerRemovedCond, bool *providerRemoved, QMutex *imageRequestedMutex, QWaitCondition *imageRequestedCond, bool *imageRequested)
+ : m_providerRemovedMutex(providerRemovedMutex), m_providerRemovedCond(providerRemovedCond), m_providerRemoved(providerRemoved),
+ m_imageRequestedMutex(imageRequestedMutex), m_imageRequestedCondition(imageRequestedCond), m_imageRequested(imageRequested)
+ {
+ }
+
+ ~WaitingAsyncProvider() {}
+
+ QQuickImageResponse *requestImageResponse(const QString &id, const QSize &requestedSize)
+ {
+ auto response = new WaitingAsyncImageResponse(m_providerRemovedMutex, m_providerRemovedCond, m_providerRemoved, m_imageRequestedMutex, m_imageRequestedCondition, m_imageRequested);
+ pool.start(response);
+ return response;
+ }
+
+ QMutex *m_providerRemovedMutex;
+ QWaitCondition *m_providerRemovedCond;
+ bool *m_providerRemoved;
+ QMutex *m_imageRequestedMutex;
+ QWaitCondition *m_imageRequestedCondition;
+ bool *m_imageRequested;
+ QThreadPool pool;
+};
+
+
+// QTBUG-76527
+void tst_qquickimageprovider::asyncImageThreadSafety()
+{
+ QQmlEngine engine;
+ QMutex providerRemovedMutex;
+ bool providerRemoved = false;
+ QWaitCondition providerRemovedCond;
+ QMutex imageRequestedMutex;
+ bool imageRequested = false;
+ QWaitCondition imageRequestedCond;
+ auto imageProvider = new WaitingAsyncProvider(&providerRemovedMutex, &providerRemovedCond, &providerRemoved, &imageRequestedMutex, &imageRequestedCond, &imageRequested);
+ engine.addImageProvider("test_waiting", imageProvider);
+ QVERIFY(engine.imageProvider("test_waiting") != nullptr);
+ auto privateEngine = QQmlEnginePrivate::get(&engine);
+
+ QString componentStr = "import QtQuick 2.0\nItem { \n"
+ "Image { source: \"image://test_waiting/blue\"; }\n"
+ " }";
+ QQmlComponent component(&engine);
+ component.setData(componentStr.toLatin1(), QUrl::fromLocalFile(""));
+ QScopedPointer<QObject> obj(component.create());
+ QVERIFY(!obj.isNull());
+ QWeakPointer<QQmlImageProviderBase> observer = privateEngine->imageProvider("test_waiting").toWeakRef();
+ QVERIFY(!observer.isNull()); // engine still own the object
+ imageRequestedMutex.lock();
+ while (!imageRequested)
+ imageRequestedCond.wait(&imageRequestedMutex);
+ imageRequestedMutex.unlock();
+ engine.removeImageProvider("test_waiting");
+
+ QVERIFY(engine.imageProvider("test_waiting") == nullptr);
+ QVERIFY(!observer.isNull()); // lifetime has been extended
+
+ providerRemovedMutex.lock();
+ providerRemoved = true;
+ providerRemovedCond.wakeAll();
+ providerRemovedMutex.unlock();
+
+ QTRY_VERIFY(observer.isNull()); // once the reply has finished, the imageprovider gets deleted
+}
+
+
QTEST_MAIN(tst_qquickimageprovider)
#include "tst_qquickimageprovider.moc"
generated by cgit v1.2.3 (git 2.25.1) at 2024-06-08 15:13:40 +0000