TSsan error when using AwaitWithTimeout with multiple waiters

[taekahn]

Summary

Creating two threads, both calling AwaitWithTimeout with a Condition, and then setting the condition to true produces a TSan error

Per #217 and the linked issue google/sanitizers#953 I tried running with report_atomic_races=0, but still see the error.

Code to reproduce

TEST(Concurrency, express_tsan_issue_with_multiple_waiters)
{
  std::queue<int> shared_queue;
  absl::Mutex shared_mutex;

  auto work = [&shared_queue, &shared_mutex]() {
      auto start = absl::Now();
      auto duration = absl::Seconds(3);
      while (absl::Now() - start < duration) {
        absl::MutexLock lock(&shared_mutex);
        shared_mutex.AwaitWithTimeout(
            absl::Condition(+[](std::queue<int>* queue) { return !queue->empty(); }, &shared_queue),
            absl::Seconds(1));
        if (!shared_queue.empty()) shared_queue.pop();
      }
  };

  auto t1 = std::async(std::launch::async, work);
  auto t2 = std::async(std::launch::async, work);

  {
    absl::MutexLock lock(&shared_mutex);
    shared_queue.push(5);
  }
}

Error produced (shortened)

==================
WARNING: ThreadSanitizer: data race (pid=14)
  Read of size 8 at 0x7f68b5aaf100 by thread T1:
    #0 absl::Mutex::Block(absl::base_internal::PerThreadSynch*) external/com_google_absl/absl/synchronization/mutex.cc:1092 
    #1 absl::Mutex::AwaitCommon(absl::Condition const&, absl::synchronization_internal::KernelTimeout) external/com_google_absl/absl/synchronization/mutex.cc:1550 
    #2 absl::Mutex::AwaitWithDeadline(absl::Condition const&, absl::Time) external/com_google_absl/absl/synchronization/mutex.cc:1531
    #3 absl::Mutex::AwaitWithTimeout(absl::Condition const&, absl::Duration) external/com_google_absl/absl/synchronization/mutex.cc:1519 

  Previous write of size 8 at 0x7f68b5aaf100 by thread T2:
    #0 Dequeue external/com_google_absl/absl/synchronization/mutex.cc:969 
    #1 absl::Mutex::TryRemove(absl::base_internal::PerThreadSynch*) external/com_google_absl/absl/synchronization/mutex.cc:1057
    #2 absl::Mutex::Block(absl::base_internal::PerThreadSynch*) external/com_google_absl/absl/synchronization/mutex.cc:1090
    #3 absl::Mutex::AwaitCommon(absl::Condition const&, absl::synchronization_internal::KernelTimeout) external/com_google_absl/absl/synchronization/mutex.cc:1550
    #4 absl::Mutex::AwaitWithDeadline(absl::Condition const&, absl::Time) external/com_google_absl/absl/synchronization/mutex.cc:1531
    #5 absl::Mutex::AwaitWithTimeout(absl::Condition const&, absl::Duration) external/com_google_absl/absl/synchronization/mutex.cc:1519

SUMMARY: ThreadSanitizer: data race external/com_google_absl/absl/synchronization/mutex.cc:1092 in absl::Mutex::Block(absl::base_internal::PerThreadSynch*)
==================

Environment

bazel 0.23
gcc 8.2
abseil commit 0dffca4

---

The test pretty reliably produces the TSan error, but it does spuriously pass on occasion.
Any help / points in the right direction appreciated.

[derekmauro]

I didn't try your example, but I looked into building Abseil with TSAN correctly and here is what I've determined:

• Abseil+TSAN probably isn't going to work with GCC at all. absl::Mutex relies on https://github.com/abseil/abseil-cpp/blob/master/absl/base/internal/tsan_mutex_interface.h to tell TSAN about its internals, and according to my testing only Clang seems to have <sanitizer/tsan_interface.h>. However, you don't seem to get any warning about this when you try to build with GCC.
• You must build with -DTHREAD_SANITIZER and -DDYNAMIC_ANNOTATIONS_ENABLED=1. With Bazel, you would pass --copt=-DTHREAD_SANITIZER and --copt=-DDYNAMIC_ANNOTATIONS_ENABLED=1 on the command line.
• It also seems like TSAN_OPTIONS=report_atomic_races=0 is necessary (--test_env="TSAN_OPTIONS=report_atomic_races=0" when running under bazel test)

None of this is ideal. We should be able to automatically detect when Abseil is being compiled with TSAN and set the appropriate defines accordingly, and error when the appropriate interfaces aren't available. I'll look into doing this.

[GregBowyer]

For what it is worth Gcc 8.3 & 9.1 for me does include the tsan interface file, so it might be worth making sure this doesnt get broken for newer versions of GCC?

[derekmauro]

This should be fixed by bb7be49.