From: Bea Lam Date: Thu, 4 Aug 2011 05:02:37 +0000 (+1000) Subject: Rework threading internals in XmlListModel to avoid global static X-Git-Tag: qt-v5.0.0-alpha1~1976 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d6f9bdfa89e36578092a7b8e0cfd00a0a7f9bf88;p=profile%2Fivi%2Fqtdeclarative.git Rework threading internals in XmlListModel to avoid global static QTBUG-20629 reports a crash on destruction of XmlListModel when cleaning up the global static for the QDeclarativeXmlQuery object. The fix restructures the internals to be like the threading structure used for QDeclarativePixmapReader which doesn't use a global static. Task-number: QTBUG-20629 Reviewed-by: Martin Jones (cherry picked from commit 422f4e8ec53b917fad09a3e671fd93048dde72ed in qt-qml-staging:master) Change-Id: Ic26164947abab67ec9d8f1ae68c20961e7af8a2b Reviewed-on: http://codereview.qt.nokia.com/2595 Reviewed-by: Qt Sanity Bot Reviewed-by: Bea Lam --- diff --git a/src/declarative/util/qdeclarativexmllistmodel.cpp b/src/declarative/util/qdeclarativexmllistmodel.cpp index 5b978f4..6b0edc0 100644 --- a/src/declarative/util/qdeclarativexmllistmodel.cpp +++ b/src/declarative/util/qdeclarativexmllistmodel.cpp @@ -147,114 +147,226 @@ struct XmlQueryJob QString prefix; }; -class QDeclarativeXmlQuery : public QObject + +class QDeclarativeXmlQueryEngine; +class QDeclarativeXmlQueryThreadObject : public QObject { Q_OBJECT public: - QDeclarativeXmlQuery(QObject *parent=0) - : QObject(parent), m_queryIds(XMLLISTMODEL_CLEAR_ID + 1) { - qRegisterMetaType("QDeclarativeXmlQueryResult"); - moveToThread(&m_thread); - m_thread.start(QThread::IdlePriority); - } + QDeclarativeXmlQueryThreadObject(QDeclarativeXmlQueryEngine *); - ~QDeclarativeXmlQuery() { - if(m_thread.isRunning()) { - m_thread.quit(); - m_thread.wait(); - } - } - - void abort(int id) { - QMutexLocker ml(&m_mutex); - if (id != -1) { - m_jobs.remove(id); - } - } - - int doQuery(QString query, QString namespaces, QByteArray data, QList* roleObjects, QStringList keyRoleResultsCache) { - { - QMutexLocker m1(&m_mutex); - m_queryIds.ref(); - if (m_queryIds <= 0) - m_queryIds = 1; - } - - XmlQueryJob job; - job.queryId = m_queryIds; - job.data = data; - job.query = QLatin1String("doc($src)") + query; - job.namespaces = namespaces; - job.keyRoleResultsCache = keyRoleResultsCache; - - for (int i=0; icount(); i++) { - if (!roleObjects->at(i)->isValid()) { - job.roleQueries << QString(); - continue; - } - job.roleQueries << roleObjects->at(i)->query(); - job.roleQueryErrorId << static_cast(roleObjects->at(i)); - if (roleObjects->at(i)->isKey()) - job.keyRoleQueries << job.roleQueries.last(); - } + void processJobs(); + virtual bool event(QEvent *e); - { - QMutexLocker ml(&m_mutex); - m_jobs.insert(m_queryIds, job); - } +private: + QDeclarativeXmlQueryEngine *m_queryEngine; +}; - QMetaObject::invokeMethod(this, "processQuery", Qt::QueuedConnection, Q_ARG(int, job.queryId)); - return job.queryId; - } -private slots: - void processQuery(int queryId) { - XmlQueryJob job; +class QDeclarativeXmlQueryEngine : public QThread +{ + Q_OBJECT +public: + QDeclarativeXmlQueryEngine(QDeclarativeEngine *eng); + ~QDeclarativeXmlQueryEngine(); - { - QMutexLocker ml(&m_mutex); - if (!m_jobs.contains(queryId)) - return; - job = m_jobs.value(queryId); - } + int doQuery(QString query, QString namespaces, QByteArray data, QList* roleObjects, QStringList keyRoleResultsCache); + void abort(int id); - QDeclarativeXmlQueryResult result; - result.queryId = job.queryId; - doQueryJob(&job, &result); - doSubQueryJob(&job, &result); + void processJobs(); - { - QMutexLocker ml(&m_mutex); - if (m_jobs.contains(queryId)) { - emit queryCompleted(result); - m_jobs.remove(queryId); - } - } - } + static QDeclarativeXmlQueryEngine *instance(QDeclarativeEngine *engine); -Q_SIGNALS: +signals: void queryCompleted(const QDeclarativeXmlQueryResult &); void error(void*, const QString&); protected: - + void run(); private: + void processQuery(XmlQueryJob *job); void doQueryJob(XmlQueryJob *job, QDeclarativeXmlQueryResult *currentResult); void doSubQueryJob(XmlQueryJob *job, QDeclarativeXmlQueryResult *currentResult); void getValuesOfKeyRoles(const XmlQueryJob& currentJob, QStringList *values, QXmlQuery *query) const; void addIndexToRangeList(QList *ranges, int index) const; -private: QMutex m_mutex; - QThread m_thread; - QMap m_jobs; + QDeclarativeXmlQueryThreadObject *m_threadObject; + QList m_jobs; + QSet m_cancelledJobs; QAtomicInt m_queryIds; + + QDeclarativeEngine *m_engine; + QObject *m_eventLoopQuitHack; + + static QHash queryEngines; + static QMutex queryEnginesMutex; }; +QHash QDeclarativeXmlQueryEngine::queryEngines; +QMutex QDeclarativeXmlQueryEngine::queryEnginesMutex; + + +QDeclarativeXmlQueryThreadObject::QDeclarativeXmlQueryThreadObject(QDeclarativeXmlQueryEngine *e) + : m_queryEngine(e) +{ +} + +void QDeclarativeXmlQueryThreadObject::processJobs() +{ + QCoreApplication::postEvent(this, new QEvent(QEvent::User)); +} + +bool QDeclarativeXmlQueryThreadObject::event(QEvent *e) +{ + if (e->type() == QEvent::User) { + m_queryEngine->processJobs(); + return true; + } else { + return QObject::event(e); + } +} + + + +QDeclarativeXmlQueryEngine::QDeclarativeXmlQueryEngine(QDeclarativeEngine *eng) +: QThread(eng), m_threadObject(0), m_queryIds(XMLLISTMODEL_CLEAR_ID + 1), m_engine(eng), m_eventLoopQuitHack(0) +{ + qRegisterMetaType("QDeclarativeXmlQueryResult"); -Q_GLOBAL_STATIC(QDeclarativeXmlQuery, globalXmlQuery) + m_eventLoopQuitHack = new QObject; + m_eventLoopQuitHack->moveToThread(this); + connect(m_eventLoopQuitHack, SIGNAL(destroyed(QObject*)), SLOT(quit()), Qt::DirectConnection); + start(QThread::IdlePriority); +} -void QDeclarativeXmlQuery::doQueryJob(XmlQueryJob *currentJob, QDeclarativeXmlQueryResult *currentResult) +QDeclarativeXmlQueryEngine::~QDeclarativeXmlQueryEngine() +{ + queryEnginesMutex.lock(); + queryEngines.remove(m_engine); + queryEnginesMutex.unlock(); + + m_eventLoopQuitHack->deleteLater(); + wait(); +} + +int QDeclarativeXmlQueryEngine::doQuery(QString query, QString namespaces, QByteArray data, QList* roleObjects, QStringList keyRoleResultsCache) { + { + QMutexLocker m1(&m_mutex); + m_queryIds.ref(); + if (m_queryIds <= 0) + m_queryIds = 1; + } + + XmlQueryJob job; + job.queryId = m_queryIds; + job.data = data; + job.query = QLatin1String("doc($src)") + query; + job.namespaces = namespaces; + job.keyRoleResultsCache = keyRoleResultsCache; + + for (int i=0; icount(); i++) { + if (!roleObjects->at(i)->isValid()) { + job.roleQueries << QString(); + continue; + } + job.roleQueries << roleObjects->at(i)->query(); + job.roleQueryErrorId << static_cast(roleObjects->at(i)); + if (roleObjects->at(i)->isKey()) + job.keyRoleQueries << job.roleQueries.last(); + } + + { + QMutexLocker ml(&m_mutex); + m_jobs.append(job); + if (m_threadObject) + m_threadObject->processJobs(); + } + + return job.queryId; +} + +void QDeclarativeXmlQueryEngine::abort(int id) +{ + QMutexLocker ml(&m_mutex); + if (id != -1) { + m_cancelledJobs.insert(id); + if (m_threadObject) + m_threadObject->processJobs(); + } +} + +void QDeclarativeXmlQueryEngine::run() +{ + m_mutex.lock(); + m_threadObject = new QDeclarativeXmlQueryThreadObject(this); + m_mutex.unlock(); + + processJobs(); + exec(); + + delete m_threadObject; + m_threadObject = 0; +} + +void QDeclarativeXmlQueryEngine::processJobs() +{ + QMutexLocker locker(&m_mutex); + + while (true) { + if (m_cancelledJobs.isEmpty() && m_jobs.isEmpty()) + return; + + if (!m_cancelledJobs.isEmpty()) { + for (QList::Iterator it = m_jobs.begin(); it != m_jobs.end(); ++it) { + int queryId = (*it).queryId; + if (m_cancelledJobs.remove(queryId)) + it = m_jobs.erase(it); + } + m_cancelledJobs.clear(); + } + + if (!m_jobs.isEmpty()) { + XmlQueryJob currentJob = m_jobs.takeLast(); + + locker.unlock(); + processQuery(¤tJob); + locker.relock(); + } + } +} + +QDeclarativeXmlQueryEngine *QDeclarativeXmlQueryEngine::instance(QDeclarativeEngine *engine) +{ + queryEnginesMutex.lock(); + QDeclarativeXmlQueryEngine *queryEng = queryEngines.value(engine); + if (!queryEng) { + queryEng = new QDeclarativeXmlQueryEngine(engine); + queryEngines.insert(engine, queryEng); + } + queryEnginesMutex.unlock(); + + return queryEng; +} + +void QDeclarativeXmlQueryEngine::processQuery(XmlQueryJob *job) +{ + QDeclarativeXmlQueryResult result; + result.queryId = job->queryId; + doQueryJob(job, &result); + doSubQueryJob(job, &result); + + { + QMutexLocker ml(&m_mutex); + if (m_cancelledJobs.contains(job->queryId)) { + m_cancelledJobs.remove(job->queryId); + } else { + emit queryCompleted(result); + } + } +} + +void QDeclarativeXmlQueryEngine::doQueryJob(XmlQueryJob *currentJob, QDeclarativeXmlQueryResult *currentResult) { Q_ASSERT(currentJob->queryId != -1); @@ -293,7 +405,7 @@ void QDeclarativeXmlQuery::doQueryJob(XmlQueryJob *currentJob, QDeclarativeXmlQu currentResult->size = (count > 0 ? count : 0); } -void QDeclarativeXmlQuery::getValuesOfKeyRoles(const XmlQueryJob& currentJob, QStringList *values, QXmlQuery *query) const +void QDeclarativeXmlQueryEngine::getValuesOfKeyRoles(const XmlQueryJob& currentJob, QStringList *values, QXmlQuery *query) const { const QStringList &keysQueries = currentJob.keyRoleQueries; QString keysQuery; @@ -314,7 +426,7 @@ void QDeclarativeXmlQuery::getValuesOfKeyRoles(const XmlQueryJob& currentJob, QS } } -void QDeclarativeXmlQuery::addIndexToRangeList(QList *ranges, int index) const { +void QDeclarativeXmlQueryEngine::addIndexToRangeList(QList *ranges, int index) const { if (ranges->isEmpty()) ranges->append(qMakePair(index, 1)); else if (ranges->last().first + ranges->last().second == index) @@ -323,7 +435,7 @@ void QDeclarativeXmlQuery::addIndexToRangeList(QList * ranges->append(qMakePair(index, 1)); } -void QDeclarativeXmlQuery::doSubQueryJob(XmlQueryJob *currentJob, QDeclarativeXmlQueryResult *currentResult) +void QDeclarativeXmlQueryEngine::doSubQueryJob(XmlQueryJob *currentJob, QDeclarativeXmlQueryResult *currentResult) { Q_ASSERT(currentJob->queryId != -1); @@ -583,10 +695,6 @@ void QDeclarativeXmlListModelPrivate::clear_role(QDeclarativeListPropertyisComponentComplete = false; + + QDeclarativeXmlQueryEngine *queryEngine = QDeclarativeXmlQueryEngine::instance(qmlEngine(this)); + connect(queryEngine, SIGNAL(queryCompleted(QDeclarativeXmlQueryResult)), + SLOT(queryCompleted(QDeclarativeXmlQueryResult))); + connect(queryEngine, SIGNAL(error(void*,QString)), + SLOT(queryError(void*,QString))); } void QDeclarativeXmlListModel::componentComplete() @@ -885,7 +999,7 @@ void QDeclarativeXmlListModel::reload() if (!d->isComponentComplete) return; - globalXmlQuery()->abort(d->queryId); + QDeclarativeXmlQueryEngine::instance(qmlEngine(this))->abort(d->queryId); d->queryId = -1; if (d->size < 0) @@ -901,7 +1015,7 @@ void QDeclarativeXmlListModel::reload() } if (!d->xml.isEmpty()) { - d->queryId = globalXmlQuery()->doQuery(d->query, d->namespaces, d->xml.toUtf8(), &d->roleObjects, d->keyRoleResultsCache); + d->queryId = QDeclarativeXmlQueryEngine::instance(qmlEngine(this))->doQuery(d->query, d->namespaces, d->xml.toUtf8(), &d->roleObjects, d->keyRoleResultsCache); d->notifyQueryStarted(false); } else if (d->src.isEmpty()) { @@ -962,7 +1076,7 @@ void QDeclarativeXmlListModel::requestFinished() d->queryId = XMLLISTMODEL_CLEAR_ID; QTimer::singleShot(0, this, SLOT(dataCleared())); } else { - d->queryId = globalXmlQuery()->doQuery(d->query, d->namespaces, data, &d->roleObjects, d->keyRoleResultsCache); + d->queryId = QDeclarativeXmlQueryEngine::instance(qmlEngine(this))->doQuery(d->query, d->namespaces, data, &d->roleObjects, d->keyRoleResultsCache); } disconnect(d->reply, 0, this, 0); d->reply->deleteLater();