[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [kdelibs/frameworks] tier1/threadweaver/src/Weaver: Fix race: helgrind says d->elements is accessed
From:       David Faure <faure () kde ! org>
Date:       2013-10-27 17:56:35
Message-ID: E1VaUZn-0005xI-Pu () scm ! kde ! org
[Download RAW message or body]

Git commit a42c608f2fb7d13d6040afa2abb1cd8e28f199c6 by David Faure.
Committed on 27/10/2013 at 17:54.
Pushed by dfaure into branch 'frameworks'.

Fix race: helgrind says d->elements is accessed from multiple threads.

-> move mutex before std::find, and lock mutex before std::find in
the other method, in release mode.

M  +4    -1    tier1/threadweaver/src/Weaver/JobCollection.cpp

http://commits.kde.org/kdelibs/a42c608f2fb7d13d6040afa2abb1cd8e28f199c6

diff --git a/tier1/threadweaver/src/Weaver/JobCollection.cpp \
b/tier1/threadweaver/src/Weaver/JobCollection.cpp index 3dffb58..3bd3992 100644
--- a/tier1/threadweaver/src/Weaver/JobCollection.cpp
+++ b/tier1/threadweaver/src/Weaver/JobCollection.cpp
@@ -196,8 +196,11 @@ void JobCollection::run(JobPointer, Thread*)
 void JobCollection::elementStarted(JobPointer job, Thread* thread)
 {
     Q_UNUSED(job) // except in Q_ASSERT
+#ifndef NDEBUG // to avoid the mutex in release mode
     Q_ASSERT(!d->self.isNull());
+    QMutexLocker l(mutex()); Q_UNUSED(l);
     Q_ASSERT(job.data() == d->self || std::find(d->elements.begin(), \
d->elements.end(), job) != d->elements.end()); +#endif
     if (d->jobsStarted.fetchAndAddOrdered(1) == 0) {
         //emit started() signal on beginning of first job execution
         executor()->defaultBegin(d->self, thread);
@@ -208,8 +211,8 @@ void JobCollection::elementFinished(JobPointer job, Thread \
*thread)  {
     Q_UNUSED(job) // except in Q_ASSERT
     Q_ASSERT(!d->self.isNull());
-    Q_ASSERT(job.data() == d->self || std::find(d->elements.begin(), \
d->elements.end(), job) != d->elements.end());  QMutexLocker l(mutex()); Q_UNUSED(l);
+    Q_ASSERT(job.data() == d->self || std::find(d->elements.begin(), \
d->elements.end(), job) != d->elements.end());  const int jobsStarted = \
d->jobsStarted.loadAcquire();  Q_ASSERT(jobsStarted >=0); Q_UNUSED(jobsStarted);
     const int remainingJobs = d->jobCounter.fetchAndAddOrdered(-1) -1;


[prev in list] [next in list] [prev in thread] [next in thread]