Fix shutdown race in IPCSyncChannelTest and get rid of "suppressions"/annotations.

ipc/ipc_sync_channel_unittest.cc

Index: ipc/ipc_sync_channel_unittest.cc
diff --git a/ipc/ipc_sync_channel_unittest.cc b/ipc/ipc_sync_channel_unittest.cc
index c31565a63e247496ab721b354f9de12d9a7068b1..a4e9bd8b374b2d266137a59ab3593f280f0a596c 100644
--- a/ipc/ipc_sync_channel_unittest.cc
+++ b/ipc/ipc_sync_channel_unittest.cc
@@ -15,9 +15,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/process_util.h"
-#include "base/stl_util.h"
 #include "base/string_util.h"
-#include "base/third_party/dynamic_annotations/dynamic_annotations.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread.h"
 #include "base/synchronization/waitable_event.h"
@@ -45,12 +43,8 @@ class Worker : public Listener, public Sender {
         ipc_thread_((thread_name + "_ipc").c_str()),
         listener_thread_((thread_name + "_listener").c_str()),
         overrided_thread_(NULL),
-        shutdown_event_(true, false) {
-    // The data race on vfptr is real but is very hard
-    // to suppress using standard Valgrind mechanism (suppressions).
-    // We have to use ANNOTATE_BENIGN_RACE to hide the reports and
-    // make ThreadSanitizer bots green.
-    ANNOTATE_BENIGN_RACE(this, "Race on vfptr, http://crbug.com/25841");
+        shutdown_event_(true, false),
+        is_shutdown_(false) {
   }
 
   // Will create a named channel and use this name for the threads' name.
@@ -62,25 +56,13 @@ class Worker : public Listener, public Sender {
         ipc_thread_((channel_name + "_ipc").c_str()),
         listener_thread_((channel_name + "_listener").c_str()),
         overrided_thread_(NULL),
-        shutdown_event_(true, false) {
-    // The data race on vfptr is real but is very hard
-    // to suppress using standard Valgrind mechanism (suppressions).
-    // We have to use ANNOTATE_BENIGN_RACE to hide the reports and
-    // make ThreadSanitizer bots green.
-    ANNOTATE_BENIGN_RACE(this, "Race on vfptr, http://crbug.com/25841");
+        shutdown_event_(true, false),
+        is_shutdown_(false) {
   }
 
-  // The IPC thread needs to outlive SyncChannel, so force the correct order of
-  // destruction.
   virtual ~Worker() {
-    WaitableEvent listener_done(false, false), ipc_done(false, false);
-    ListenerThread()->message_loop()->PostTask(
-        FROM_HERE, base::Bind(&Worker::OnListenerThreadShutdown1, this,
-                              &listener_done, &ipc_done));
-    listener_done.Wait();
-    ipc_done.Wait();
-    ipc_thread_.Stop();
-    listener_thread_.Stop();
+    // Shutdown() must be called before destruction.
+    CHECK(is_shutdown_);
   }
   void AddRef() { }
   void Release() { }
@@ -98,6 +80,20 @@ class Worker : public Listener, public Sender {
     ListenerThread()->message_loop()->PostTask(
         FROM_HERE, base::Bind(&Worker::OnStart, this));
   }
+  void Shutdown() {
+    // The IPC thread needs to outlive SyncChannel. We can't do this in
+    // ~Worker(), since that'll reset the vtable pointer (to Worker's), which
+    // may result in a race conditions. See http://crbug.com/25841.
+    WaitableEvent listener_done(false, false), ipc_done(false, false);
+    ListenerThread()->message_loop()->PostTask(
+        FROM_HERE, base::Bind(&Worker::OnListenerThreadShutdown1, this,
+                              &listener_done, &ipc_done));
+    listener_done.Wait();
+    ipc_done.Wait();
+    ipc_thread_.Stop();
+    listener_thread_.Stop();
+    is_shutdown_ = true;
+  }
   void OverrideThread(base::Thread* overrided_thread) {
     DCHECK(overrided_thread_ == NULL);
     overrided_thread_ = overrided_thread;
@@ -236,6 +232,8 @@ class Worker : public Listener, public Sender {
 
   base::WaitableEvent shutdown_event_;
 
+  bool is_shutdown_;
+
   DISALLOW_COPY_AND_ASSIGN(Worker);
 };
 
@@ -262,7 +260,10 @@ void RunTest(std::vector<Worker*> workers) {
   for (size_t i = 0; i < workers.size(); ++i)
     workers[i]->done_event()->Wait();
 
-  STLDeleteContainerPointers(workers.begin(), workers.end());
+  for (size_t i = 0; i < workers.size(); ++i) {
+    workers[i]->Shutdown();
+    delete workers[i];
+  }
 }
 
 }  // namespace
@@ -1194,6 +1195,8 @@ TEST_F(IPCSyncChannelTest, SendAfterClose) {
   server.done_event()->Wait();
 
   EXPECT_FALSE(server.send_result());
+
+  server.Shutdown();
 }
 
 //-----------------------------------------------------------------------------