commit edf23673468775470d55d9af5103c41137a1c0a3
author commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> Tue Oct 01 18:44:18 2013 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> Tue Oct 01 18:44:18 2013 +0000
tree 55a3afc1fd196e6458646e30449925d73f1ba581
parent 57f035ecae1ff8f2b2e0614e0649f0b90c69adad

Fix race between ~SkThreadPool and SkThreadPool::Loop on fDone.

We're writing fDone without holding the mutex.  Bad form, says tsan.

In practice this is fairly innocuous, as fDone only ever goes from false to
true and only once.  Though, I wouldn't be surprised if there were some way
this could leak a thread that never got the signal to die.

BUG=
R=scroggo@google.com, reed@google.com

Author: mtklein@google.com

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

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

include/utils/SkThreadPool.h

diff --git a/include/utils/SkThreadPool.h b/include/utils/SkThreadPool.h
index 3c86158..9865703 100644
--- a/include/utils/SkThreadPool.h
+++ b/include/utils/SkThreadPool.h
@@ -43,7 +43,7 @@
     SkTInternalLList<LinkedRunnable>    fQueue;
     SkCondVar                           fReady;
     SkTDArray<SkThread*>                fThreads;
-    bool                            fDone;
+    bool                                fDone;
 
     static void Loop(void*);  // Static because we pass in this.
 };

src/utils/SkThreadPool.cpp

diff --git a/src/utils/SkThreadPool.cpp b/src/utils/SkThreadPool.cpp
index 5fe1025..3d19d1c 100644
--- a/src/utils/SkThreadPool.cpp
+++ b/src/utils/SkThreadPool.cpp
@@ -39,8 +39,8 @@
 }
 
 SkThreadPool::~SkThreadPool() {
-    fDone = true;
     fReady.lock();
+    fDone = true;
     fReady.broadcast();
     fReady.unlock();