commit 8c80c6e389d235d259a39e0114316447aa228092
author tommi <tommi@webrtc.org> Thu Feb 23 00:34:52 2017 -0800
committer Commit bot <commit-bot@chromium.org> Thu Feb 23 08:34:52 2017 +0000
tree bae49bd333abd43f0c84f8fba6aedcd77aae94c7
parent 64b6d0781fbfce17c98e1dfaf54f31d8f776ba29

Fix potential deadlock in TaskQueue's libevent PostTaskAndReply implementation

BUG=webrtc:7188

Review-Url: https://codereview.webrtc.org/2709603002
Cr-Commit-Position: refs/heads/master@{#16786}

webrtc/base/task_queue_unittest.cc

diff --git a/webrtc/base/task_queue_unittest.cc b/webrtc/base/task_queue_unittest.cc
index 74433b9..4389d35 100644
--- a/webrtc/base/task_queue_unittest.cc
+++ b/webrtc/base/task_queue_unittest.cc
@@ -37,22 +37,21 @@
 
 TEST(TaskQueueTest, PostAndCheckCurrent) {
   static const char kQueueName[] = "PostAndCheckCurrent";
+  Event event(false, false);
   TaskQueue queue(kQueueName);
 
   // We're not running a task, so there shouldn't be a current queue.
   EXPECT_FALSE(queue.IsCurrent());
   EXPECT_FALSE(TaskQueue::Current());
 
-  Event event(false, false);
   queue.PostTask(Bind(&CheckCurrent, kQueueName, &event, &queue));
   EXPECT_TRUE(event.Wait(1000));
 }
 
 TEST(TaskQueueTest, PostCustomTask) {
   static const char kQueueName[] = "PostCustomImplementation";
-  TaskQueue queue(kQueueName);
-
   Event event(false, false);
+  TaskQueue queue(kQueueName);
 
   class CustomTask : public QueuedTask {
    public:
@@ -74,18 +73,18 @@
 
 TEST(TaskQueueTest, PostLambda) {
   static const char kQueueName[] = "PostLambda";
+  Event event(false, false);
   TaskQueue queue(kQueueName);
 
-  Event event(false, false);
   queue.PostTask([&event]() { event.Set(); });
   EXPECT_TRUE(event.Wait(1000));
 }
 
 TEST(TaskQueueTest, PostFromQueue) {
   static const char kQueueName[] = "PostFromQueue";
+  Event event(false, false);
   TaskQueue queue(kQueueName);
 
-  Event event(false, false);
   queue.PostTask(
       [&event, &queue]() { queue.PostTask([&event]() { event.Set(); }); });
   EXPECT_TRUE(event.Wait(1000));
@@ -93,9 +92,9 @@
 
 TEST(TaskQueueTest, PostDelayed) {
   static const char kQueueName[] = "PostDelayed";
+  Event event(false, false);
   TaskQueue queue(kQueueName);
 
-  Event event(false, false);
   uint32_t start = Time();
   queue.PostDelayedTask(Bind(&CheckCurrent, kQueueName, &event, &queue), 100);
   EXPECT_TRUE(event.Wait(1000));
@@ -135,10 +134,10 @@
 TEST(TaskQueueTest, PostAndReply) {
   static const char kPostQueue[] = "PostQueue";
   static const char kReplyQueue[] = "ReplyQueue";
+  Event event(false, false);
   TaskQueue post_queue(kPostQueue);
   TaskQueue reply_queue(kReplyQueue);
 
-  Event event(false, false);
   post_queue.PostTaskAndReply(
       Bind(&CheckCurrent, kPostQueue, nullptr, &post_queue),
       Bind(&CheckCurrent, kReplyQueue, &event, &reply_queue), &reply_queue);
@@ -148,6 +147,7 @@
 TEST(TaskQueueTest, PostAndReuse) {
   static const char kPostQueue[] = "PostQueue";
   static const char kReplyQueue[] = "ReplyQueue";
+  Event event(false, false);
   TaskQueue post_queue(kPostQueue);
   TaskQueue reply_queue(kReplyQueue);
 
@@ -184,7 +184,6 @@
     Event* const event_;
   };
 
-  Event event(false, false);
   std::unique_ptr<QueuedTask> task(
       new ReusedTask(&call_count, &reply_queue, &event));
 
@@ -195,10 +194,10 @@
 TEST(TaskQueueTest, PostAndReplyLambda) {
   static const char kPostQueue[] = "PostQueue";
   static const char kReplyQueue[] = "ReplyQueue";
+  Event event(false, false);
   TaskQueue post_queue(kPostQueue);
   TaskQueue reply_queue(kReplyQueue);
 
-  Event event(false, false);
   bool my_flag = false;
   post_queue.PostTaskAndReply([&my_flag]() { my_flag = true; },
                               [&event]() { event.Set(); }, &reply_queue);
@@ -206,6 +205,21 @@
   EXPECT_TRUE(my_flag);
 }
 
+// This test covers a particular bug that we had in the libevent implementation
+// where we could hit a deadlock while trying to post a reply task to a queue
+// that was being deleted.  The test isn't guaranteed to hit that case but it's
+// written in a way that makes it likely and by running with --gtest_repeat=1000
+// the bug would occur. Alas, now it should be fixed.
+TEST(TaskQueueTest, PostAndReplyDeadlock) {
+  Event event(false, false);
+  TaskQueue post_queue("PostQueue");
+  TaskQueue reply_queue("ReplyQueue");
+
+  post_queue.PostTaskAndReply([&event]() { event.Set(); }, []() {},
+                              &reply_queue);
+  EXPECT_TRUE(event.Wait(1000));
+}
+
 void TestPostTaskAndReply(TaskQueue* work_queue,
                           const char* work_queue_name,
                           Event* event) {
@@ -220,10 +234,10 @@
 TEST(TaskQueueTest, PostAndReply2) {
   static const char kQueueName[] = "PostAndReply2";
   static const char kWorkQueueName[] = "PostAndReply2_Worker";
+  Event event(false, false);
   TaskQueue queue(kQueueName);
   TaskQueue work_queue(kWorkQueueName);
 
-  Event event(false, false);
   queue.PostTask(
       Bind(&TestPostTaskAndReply, &work_queue, kWorkQueueName, &event));
   EXPECT_TRUE(event.Wait(1000));