[PATCH] D18811: Fix a race condition in support library ThreadPool

[Mehdi AMINI via llvm-commits]

joker.eph added inline comments.

================
Comment at: lib/Support/ThreadPool.cpp:79
@@ -78,3 +70,3 @@
   CompletionCondition.wait(LockGuard,
                            [&] { return Tasks.empty() && !ActiveThreads; });
 }
----------------
joker.eph wrote:
> jlebar wrote:
> > joker.eph wrote:
> > > Would just reversing the check solve the race?
> > > 
> > > ```
> > >  [&] { return !ActiveThreads && Tasks.empty(); });
> > > ```
> > > 
> > > If not can you elaborate a little?
> > > 
> > The race is that we touch Tasks here while holding only CompletionLock, while another thread in asyncImpl may touch Tasks while holding only QueueLock.
> Mmmm the description of the revision was not about *enqueuing* like you suggest, but about poping.
> I need another look then.
The use case you're mentioning, i.e. waiting for the pool to finish in one thread, while another thread would enqueue to the pool is explicitly forbidden by the API: see the `wait()` documentation: ` It is an error to try to add new tasks while blocking on this call.`


http://reviews.llvm.org/D18811