Fix race condition in processJobs()
authorBea Lam <bea.lam@nokia.com>
Thu, 11 Aug 2011 03:26:15 +0000 (13:26 +1000)
committerQt by Nokia <qt-info@nokia.com>
Fri, 12 Aug 2011 22:01:12 +0000 (00:01 +0200)
Don't modify list of running jobs when a job is aborted since the job
may have just started. Wait till the next time processJobs() is invoked
for a new job and discard the aborted job at that time.

(cherry picked from commit f55ecc080d0c5eca4e65a235c63ab13867c86874
in 4.8)

Task-number: QTBUG-20841
Change-Id: Icb4cd089505c0634d3fec023b52c61bbc878404f
Reviewed-on: http://codereview.qt.nokia.com/2837
Reviewed-by: Bea Lam <bea.lam@nokia.com>
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
src/declarative/util/qdeclarativexmllistmodel.cpp

index 7ac6c76..86e6e9e 100644 (file)
@@ -289,11 +289,8 @@ int QDeclarativeXmlQueryEngine::doQuery(QString query, QString namespaces, QByte
 void QDeclarativeXmlQueryEngine::abort(int id)
 {
     QMutexLocker ml(&m_mutex);
-    if (id != -1) {
+    if (id != -1)
         m_cancelledJobs.insert(id);
-        if (m_threadObject)
-            m_threadObject->processJobs();
-    }
 }
 
 void QDeclarativeXmlQueryEngine::run()
@@ -314,25 +311,19 @@ void QDeclarativeXmlQueryEngine::processJobs()
     QMutexLocker locker(&m_mutex);
 
     while (true) {
-        if (m_cancelledJobs.isEmpty() && m_jobs.isEmpty())
+        if (m_jobs.isEmpty())
             return;
 
-        if (!m_cancelledJobs.isEmpty()) {
-            for (QList<XmlQueryJob>::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();
+        XmlQueryJob currentJob = m_jobs.takeLast();
+        while (m_cancelledJobs.remove(currentJob.queryId)) {
+            if (m_jobs.isEmpty())
+              return;
+            currentJob = m_jobs.takeLast();
         }
 
-        if (!m_jobs.isEmpty()) {
-            XmlQueryJob currentJob = m_jobs.takeLast();
-
-            locker.unlock();
-            processQuery(&currentJob);
-            locker.relock();
-        }
+        locker.unlock();
+        processQuery(&currentJob);
+        locker.relock();
     }
 }