In skia_test.cc, atomics -> mutex.

These guys are not heavily contended nor speed critical.  No need for atomics,
plus this makes tsan stop complaining (correctly) about reading fNextIndex
unsafely in onEnd.

I took a look at failCount/fFailCount, which I think is safely atomic and quite
conveniently so:  It's never read until all the threads which could possibly
increment it have terminated (except for the one where it was created,
obviously).  We could guard it with a mutex too, but maybe we can let this one
slide.

BUG=
R=bungeman@google.com

Author: mtklein@google.com

Review URL: https://codereview.chromium.org/25357002

git-svn-id: http://skia.googlecode.com/svn/trunk@11561 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/tests/skia_test.cpp b/tests/skia_test.cpp
index a93b307..620cdac 100644 (file)
--- a/tests/skia_test.cpp
+++ b/tests/skia_test.cpp
@@ -84,20 +84,23 @@ public:
 
 protected:
     virtual void onStart(Test* test) {
-        const int index = sk_atomic_inc(&fNextIndex)+1;
-        const int pending = sk_atomic_inc(&fPending)+1;
-        SkDebugf("[%3d/%3d] (%d) %s\n", index, fTotal, pending, test->getName());
+        SkAutoMutexAcquire lock(fStartEndMutex);
+        fNextIndex++;
+        fPending++;
+        SkDebugf("[%3d/%3d] (%d) %s\n", fNextIndex, fTotal, fPending, test->getName());
     }
+
     virtual void onReportFailed(const SkString& desc) {
         SkDebugf("\tFAILED: %s\n", desc.c_str());
     }
 
     virtual void onEnd(Test* test) {
+        SkAutoMutexAcquire lock(fStartEndMutex);
         if (!test->passed()) {
             SkDebugf("---- %s FAILED\n", test->getName());
         }
 
-        sk_atomic_dec(&fPending);
+        fPending--;
         if (fNextIndex == fTotal) {
             // Just waiting on straggler tests.  Shame them by printing their name and runtime.
             SkDebugf("          (%d) %5.1fs %s\n",
@@ -106,8 +109,11 @@ protected:
     }
 
 private:
+    SkMutex fStartEndMutex;  // Guards fNextIndex and fPending.
     int32_t fNextIndex;
     int32_t fPending;
+
+    // Once the tests get going, these are logically const.
     int fTotal;
     bool fAllowExtendedTest;
     bool fAllowThreaded;