From 9b3b29997f91892266fc34083eef269d7d7cc006 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Thu, 3 Nov 2011 10:22:32 +0100 Subject: qmlplugindump: Add flush to fix output redirection on windows. Task-number: QTCREATORBUG-5825 --- tools/qmlplugindump/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/qmlplugindump/main.cpp b/tools/qmlplugindump/main.cpp index 6bef8d4..e9547af 100644 --- a/tools/qmlplugindump/main.cpp +++ b/tools/qmlplugindump/main.cpp @@ -641,7 +641,7 @@ int main(int argc, char *argv[]) qml.writeEndObject(); qml.writeEndDocument(); - std::cout << bytes.constData(); + std::cout << bytes.constData() << std::flush; // workaround to avoid crashes on exit QTimer timer; -- cgit v0.12 From b029874615f0da72a44dab5b6d1a398a71f13100 Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Mon, 14 Nov 2011 11:57:21 +1000 Subject: Properly protect access to pixmap reader thread with mutex Previously, access to the data from the reader thread wasn't guarded properly, causing a crash when the reader thread was deleted prior to QDeclarativePixmapData (which then attempted to dereference the thread pointer to cancel the request), or in the case where a QDeclarativePixmapData was deleted after its QDeclarativePixmapReply was removed from the jobs queue but prior to processing. Reviewed-by: Martin Jones Task-number: QTBUG-22125 --- src/declarative/util/qdeclarativepixmapcache.cpp | 57 ++++++++++++++++------ .../qdeclarativeimage/data/qtbug_22125.qml | 44 +++++++++++++++++ .../qdeclarativeimage/tst_qdeclarativeimage.cpp | 41 ++++++++++++++++ 3 files changed, 126 insertions(+), 16 deletions(-) create mode 100644 tests/auto/declarative/qdeclarativeimage/data/qtbug_22125.qml 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 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 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 @@ -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 -- cgit v0.12 From 7c5d13df7ec850196a6f9bb739deafbee32c77ee Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Wed, 16 Nov 2011 12:31:51 +0100 Subject: qmlplugindump: Fix dumping empty names for generated QMetaObjects. Task-number: QTCREATORBUG-6543 Reviewed-by: Roberto Raggi --- tools/qmlplugindump/main.cpp | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/tools/qmlplugindump/main.cpp b/tools/qmlplugindump/main.cpp index e9547af..4f523b9 100644 --- a/tools/qmlplugindump/main.cpp +++ b/tools/qmlplugindump/main.cpp @@ -147,6 +147,31 @@ QByteArray convertToId(const QByteArray &cppName) return cppToId.value(cppName, cppName); } +QByteArray convertToId(const QMetaObject *mo) +{ + QByteArray className(mo->className()); + if (!className.isEmpty()) + return convertToId(className); + + // likely a metaobject generated for an extended qml object + if (mo->superClass()) { + className = convertToId(mo->superClass()); + className.append("_extended"); + return className; + } + + static QHash generatedNames; + className = generatedNames.value(mo); + if (!className.isEmpty()) + return className; + + qWarning() << "Found a QMetaObject without a className, generating a random name"; + className = QByteArray("error-unknown-name-"); + className.append(QByteArray::number(generatedNames.size())); + generatedNames.insert(mo, className); + return className; +} + QSet collectReachableMetaObjects(const QList &skip = QList()) { QSet metas; @@ -241,7 +266,7 @@ public: { qml->writeStartObject("Component"); - QByteArray id = convertToId(meta->className()); + QByteArray id = convertToId(meta); qml->writeScriptBinding(QLatin1String("name"), enquote(id)); for (int index = meta->classInfoCount() - 1 ; index >= 0 ; --index) { @@ -253,7 +278,7 @@ public: } if (meta->superClass()) - qml->writeScriptBinding(QLatin1String("prototype"), enquote(convertToId(meta->superClass()->className()))); + qml->writeScriptBinding(QLatin1String("prototype"), enquote(convertToId(meta->superClass()))); QSet qmlTypes = qmlTypesByCppName.value(meta->className()); if (!qmlTypes.isEmpty()) { @@ -284,7 +309,7 @@ public: if (const QMetaObject *attachedType = (*qmlTypes.begin())->attachedPropertiesType()) { qml->writeScriptBinding(QLatin1String("attachedType"), enquote( - convertToId(attachedType->className()))); + convertToId(attachedType))); } } @@ -624,7 +649,7 @@ int main(int argc, char *argv[]) // put the metaobjects into a map so they are always dumped in the same order QMap nameToMeta; foreach (const QMetaObject *meta, metas) - nameToMeta.insert(convertToId(meta->className()), meta); + nameToMeta.insert(convertToId(meta), meta); Dumper dumper(&qml); if (relocatable) -- cgit v0.12 From 8c898d681d12f3c67e396e7e2f2aa5522288aa63 Mon Sep 17 00:00:00 2001 From: Aurindam Jana Date: Thu, 17 Nov 2011 15:42:48 +0100 Subject: DeclarativeDebugServer: Instantiate QPluginLoader on heap The pluginloader is instantiated on heap with server as its parent so that the connection plugin instance remains loaded in memory. This prevents the plugin loader in QDeclarativeInspectorService to accidentally un-load the plugin while it's in use. Change-Id: I11c82ce595a336ba0fd1b243d23cb497fe355147 Reviewed-by: Kai Koehne --- src/declarative/debugger/qdeclarativedebugserver.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/declarative/debugger/qdeclarativedebugserver.cpp b/src/declarative/debugger/qdeclarativedebugserver.cpp index 12691b2..fc908f3 100644 --- a/src/declarative/debugger/qdeclarativedebugserver.cpp +++ b/src/declarative/debugger/qdeclarativedebugserver.cpp @@ -95,7 +95,7 @@ public: private: // private slot void _q_deliverMessage(const QString &serviceName, const QByteArray &message); - static QDeclarativeDebugServerConnection *loadConnectionPlugin(const QString &pluginName); + static QDeclarativeDebugServerConnection *loadConnectionPlugin(QPluginLoader *loader, const QString &pluginName); }; QDeclarativeDebugServerPrivate::QDeclarativeDebugServerPrivate() : @@ -118,7 +118,7 @@ void QDeclarativeDebugServerPrivate::advertisePlugins() } QDeclarativeDebugServerConnection *QDeclarativeDebugServerPrivate::loadConnectionPlugin( - const QString &pluginName) + QPluginLoader *loader, const QString &pluginName) { #ifndef QT_NO_LIBRARY QStringList pluginCandidates; @@ -135,17 +135,17 @@ QDeclarativeDebugServerConnection *QDeclarativeDebugServerPrivate::loadConnectio } foreach (const QString &pluginPath, pluginCandidates) { - QPluginLoader loader(pluginPath); - if (!loader.load()) { + loader->setFileName(pluginPath); + if (!loader->load()) { continue; } QDeclarativeDebugServerConnection *connection = 0; - if (QObject *instance = loader.instance()) + if (QObject *instance = loader->instance()) connection = qobject_cast(instance); if (connection) return connection; - loader.unload(); + loader->unload(); } #endif return 0; @@ -200,8 +200,9 @@ QDeclarativeDebugServer *QDeclarativeDebugServer::instance() if (ok) { server = new QDeclarativeDebugServer(); + QPluginLoader *loader = new QPluginLoader(server); QDeclarativeDebugServerConnection *connection - = QDeclarativeDebugServerPrivate::loadConnectionPlugin(pluginName); + = QDeclarativeDebugServerPrivate::loadConnectionPlugin(loader, pluginName); if (connection) { server->d_func()->connection = connection; -- cgit v0.12