[llvm] r265618 - Fix a race condition in support library ThreadPool.

[Kostya Serebryany via llvm-commits]

ouch!
Will it make sense to add a tsan bot to the sanitizer bot where we
currently run asan, ubsan, and msan, but not tsan?
If yes, which tests to run? (if not all of "check-llvm")?

--kcc

On Wed, Apr 6, 2016 at 4:46 PM, Justin Lebar via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: jlebar
> Date: Wed Apr  6 18:46:40 2016
> New Revision: 265618
>
> URL: http://llvm.org/viewvc/llvm-project?rev=265618&view=rev
> Log:
> Fix a race condition in support library ThreadPool.
>
> By running TSAN on the ThreadPool unit tests it was discovered that the
> threads in the pool can pop tasks off the queue at the same time the
> "wait" routine is trying to check if the task queue is empty. This patch
> fixes this problem by checking for active threads in the waiter before
> checking whether the queue is empty.
>
> Patch by Jason Henline.
>
> Differential Revision: http://reviews.llvm.org/D18811
>
> Reviewers: joker.eph, jlebar
>
> Modified:
>     llvm/trunk/lib/Support/ThreadPool.cpp
>
> Modified: llvm/trunk/lib/Support/ThreadPool.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/ThreadPool.cpp?rev=265618&r1=265617&r2=265618&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Support/ThreadPool.cpp (original)
> +++ llvm/trunk/lib/Support/ThreadPool.cpp Wed Apr  6 18:46:40 2016
> @@ -75,8 +75,11 @@ ThreadPool::ThreadPool(unsigned ThreadCo
>  void ThreadPool::wait() {
>    // Wait for all threads to complete and the queue to be empty
>    std::unique_lock<std::mutex> LockGuard(CompletionLock);
> +  // The order of the checks for ActiveThreads and Tasks.empty() matters
> because
> +  // any active threads might be modifying the Tasks queue, and this
> would be a
> +  // race.
>    CompletionCondition.wait(LockGuard,
> -                           [&] { return Tasks.empty() && !ActiveThreads;
> });
> +                           [&] { return !ActiveThreads && Tasks.empty();
> });
>  }
>
>  std::shared_future<ThreadPool::VoidTy> ThreadPool::asyncImpl(TaskTy Task)
> {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160415/43ff8a8f/attachment.html>