diff options
3 files changed, 126 insertions, 16 deletions
diff --git a/src/declarative/util/qdeclarativepixmapcache.cpp b/src/declarative/util/qdeclarativepixmapcache.cpp index 3557425..b23ac73 100644 --- a/src/declarative/util/qdeclarativepixmapcache.cpp +++ b/src/declarative/util/qdeclarativepixmapcache.cpp @@ -90,8 +90,9 @@ public: ~QDeclarativePixmapReply(); QDeclarativePixmapData *data; - QDeclarativePixmapReader *reader; + QDeclarativeEngine *engineForReader; // always access reader inside readerMutex. QSize requestSize; + QUrl url; bool loading; int redirectCount; @@ -147,6 +148,7 @@ public: void cancel(QDeclarativePixmapReply *rep); static QDeclarativePixmapReader *instance(QDeclarativeEngine *engine); + static QDeclarativePixmapReader *existingInstance(QDeclarativeEngine *engine); protected: void run(); @@ -176,6 +178,7 @@ private: static int downloadProgress; static int threadNetworkRequestDone; static QHash<QDeclarativeEngine *,QDeclarativePixmapReader*> readers; +public: static QMutex readerMutex; }; @@ -326,6 +329,22 @@ QDeclarativePixmapReader::~QDeclarativePixmapReader() readers.remove(engine); readerMutex.unlock(); + mutex.lock(); + // manually cancel all outstanding jobs. + foreach (QDeclarativePixmapReply *reply, jobs) { + delete reply; + } + jobs.clear(); + QList<QDeclarativePixmapReply*> activeJobs = replies.values(); + foreach (QDeclarativePixmapReply *reply, activeJobs) { + if (reply->loading) { + cancelled.append(reply); + reply->data = 0; + } + } + if (threadObject) threadObject->processJobs(); + mutex.unlock(); + eventLoopQuitHack->deleteLater(); wait(); } @@ -433,9 +452,8 @@ void QDeclarativePixmapReader::processJobs() if (!jobs.isEmpty() && replies.count() < IMAGEREQUEST_MAX_REQUEST_COUNT) { QDeclarativePixmapReply *runningJob = jobs.takeLast(); runningJob->loading = true; - - QUrl url = runningJob->data->url; - QSize requestSize = runningJob->data->requestSize; + QUrl url = runningJob->url; + QSize requestSize = runningJob->requestSize; locker.unlock(); processJob(runningJob, url, requestSize); locker.relock(); @@ -459,7 +477,6 @@ void QDeclarativePixmapReader::processJob(QDeclarativePixmapReply *runningJob, c errorCode = QDeclarativePixmapReply::Loading; errorStr = QDeclarativePixmap::tr("Failed to get image from provider: %1").arg(url.toString()); } - mutex.lock(); if (!cancelled.contains(runningJob)) runningJob->postReply(errorCode, errorStr, readSize, image); mutex.unlock(); @@ -487,10 +504,8 @@ void QDeclarativePixmapReader::processJob(QDeclarativePixmapReply *runningJob, c QNetworkRequest req(url); req.setAttribute(QNetworkRequest::HttpPipeliningAllowedAttribute, true); QNetworkReply *reply = networkAccessManager()->get(req); - QMetaObject::connect(reply, replyDownloadProgress, runningJob, downloadProgress); QMetaObject::connect(reply, replyFinished, threadObject, threadNetworkRequestDone); - replies.insert(reply, runningJob); } } @@ -498,22 +513,27 @@ void QDeclarativePixmapReader::processJob(QDeclarativePixmapReply *runningJob, c QDeclarativePixmapReader *QDeclarativePixmapReader::instance(QDeclarativeEngine *engine) { - readerMutex.lock(); + // XXX NOTE: must be called within readerMutex locking. QDeclarativePixmapReader *reader = readers.value(engine); if (!reader) { reader = new QDeclarativePixmapReader(engine); readers.insert(engine, reader); } - readerMutex.unlock(); return reader; } +QDeclarativePixmapReader *QDeclarativePixmapReader::existingInstance(QDeclarativeEngine *engine) +{ + // XXX NOTE: must be called within readerMutex locking. + return readers.value(engine, 0); +} + QDeclarativePixmapReply *QDeclarativePixmapReader::getImage(QDeclarativePixmapData *data) { mutex.lock(); QDeclarativePixmapReply *reply = new QDeclarativePixmapReply(data); - reply->reader = this; + reply->engineForReader = engine; jobs.append(reply); // XXX if (threadObject) threadObject->processJobs(); @@ -692,7 +712,7 @@ void QDeclarativePixmapStore::flushCache() } QDeclarativePixmapReply::QDeclarativePixmapReply(QDeclarativePixmapData *d) -: data(d), reader(0), requestSize(d->requestSize), loading(false), redirectCount(0) +: data(d), engineForReader(0), requestSize(d->requestSize), url(d->url), loading(false), redirectCount(0) { if (finishedIndex == -1) { finishedIndex = QDeclarativePixmapReply::staticMetaObject.indexOfSignal("finished()"); @@ -750,8 +770,14 @@ void QDeclarativePixmapData::release() if (refCount == 0) { if (reply) { - reply->reader->cancel(reply); + QDeclarativePixmapReply *cancelReply = reply; + reply->data = 0; reply = 0; + QDeclarativePixmapReader::readerMutex.lock(); + QDeclarativePixmapReader *reader = QDeclarativePixmapReader::existingInstance(cancelReply->engineForReader); + if (reader) + reader->cancel(cancelReply); + QDeclarativePixmapReader::readerMutex.unlock(); } if (pixmapStatus == QDeclarativePixmap::Ready) { @@ -1013,13 +1039,12 @@ void QDeclarativePixmap::load(QDeclarativeEngine *engine, const QUrl &url, const if (!engine) return; - QDeclarativePixmapReader *reader = QDeclarativePixmapReader::instance(engine); - d = new QDeclarativePixmapData(url, requestSize); if (options & QDeclarativePixmap::Cache) d->addToCache(); - - d->reply = reader->getImage(d); + QDeclarativePixmapReader::readerMutex.lock(); + d->reply = QDeclarativePixmapReader::instance(engine)->getImage(d); + QDeclarativePixmapReader::readerMutex.unlock(); } else { d = *iter; d->addref(); diff --git a/tests/auto/declarative/qdeclarativeimage/data/qtbug_22125.qml b/tests/auto/declarative/qdeclarativeimage/data/qtbug_22125.qml new file mode 100644 index 0000000..8588028 --- /dev/null +++ b/tests/auto/declarative/qdeclarativeimage/data/qtbug_22125.qml @@ -0,0 +1,44 @@ +import QtQuick 1.1 + +Item { + id: root + width: 800 + height: 800 + + GridView { + anchors.fill: parent + delegate: Image { + source: imagePath; + asynchronous: true + smooth: true + width: 200 + height: 200 + } + model: ListModel { + ListElement { + imagePath: "http://127.0.0.1:14451/big256.png" + } + ListElement { + imagePath: "http://127.0.0.1:14451/big256.png" + } + ListElement { + imagePath: "http://127.0.0.1:14451/big256.png" + } + ListElement { + imagePath: "http://127.0.0.1:14451/colors.png" + } + ListElement { + imagePath: "http://127.0.0.1:14451/colors1.png" + } + ListElement { + imagePath: "http://127.0.0.1:14451/big.jpeg" + } + ListElement { + imagePath: "http://127.0.0.1:14451/heart.png" + } + ListElement { + imagePath: "http://127.0.0.1:14451/green.png" + } + } + } +} diff --git a/tests/auto/declarative/qdeclarativeimage/tst_qdeclarativeimage.cpp b/tests/auto/declarative/qdeclarativeimage/tst_qdeclarativeimage.cpp index a35d69a..f67c5b5 100644 --- a/tests/auto/declarative/qdeclarativeimage/tst_qdeclarativeimage.cpp +++ b/tests/auto/declarative/qdeclarativeimage/tst_qdeclarativeimage.cpp @@ -94,6 +94,7 @@ private slots: void resetSourceSize(); void testQtQuick11Attributes(); void testQtQuick11Attributes_data(); + void readerCrash_QTBUG_22125(); private: template<typename T> @@ -762,6 +763,46 @@ void tst_qdeclarativeimage::testQtQuick11Attributes_data() << ":1 \"Image.cache\" is not available in QtQuick 1.0.\n"; } +void tst_qdeclarativeimage::readerCrash_QTBUG_22125() +{ + { + TestHTTPServer server(SERVER_PORT); + QVERIFY(server.isValid()); + server.serveDirectory(SRCDIR "/data/", TestHTTPServer::Delay); + + { + QDeclarativeView view(QUrl::fromLocalFile(SRCDIR "/data/qtbug_22125.qml")); + view.show(); + qApp->processEvents(); + qApp->processEvents(); + // shouldn't crash when the view drops out of scope due to + // QDeclarativePixmapData attempting to dereference a pointer to + // the destroyed reader. + } + + // shouldn't crash when deleting cancelled QDeclarativePixmapReplys. + QTest::qWait(1000); + qApp->processEvents(QEventLoop::DeferredDeletion); + } + + { + TestHTTPServer server(SERVER_PORT); + QVERIFY(server.isValid()); + server.serveDirectory(SRCDIR "/data/"); + + { + QDeclarativeView view(QUrl::fromLocalFile(SRCDIR "/data/qtbug_22125.qml")); + view.show(); + qApp->processEvents(); + QTest::qWait(1000); + qApp->processEvents(); + // shouldn't crash when the view drops out of scope due to + // the reader thread accessing self-deleted QDeclarativePixmapReplys. + } + qApp->processEvents(); + } +} + /* Find an item with the specified objectName. If index is supplied then the item must also evaluate the {index} expression equal to index |