From 01f64e0eb22d855fc769b6976ba62dd0d94a071a Mon Sep 17 00:00:00 2001
From: Artem Titov <titovartem@webrtc.org>
Date: Thu, 31 Jan 2019 13:31:09 +0100
Subject: [PATCH] Add test to verify TaskQueue memory access order.

Bug: webrtc:10138
Change-Id: I53e8a3a612ad44feced8d63a4035d79b7e0f22a9
Reviewed-on: https://webrtc-review.googlesource.com/c/120601
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26497}
---
 api/task_queue/task_queue_test.cc | 32 ++++++++++++++++++++++++++++++
 rtc_base/task_queue_unittest.cc   | 33 +++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/api/task_queue/task_queue_test.cc b/api/task_queue/task_queue_test.cc
index 103581ae9d..ca7aee9b26 100644
--- a/api/task_queue/task_queue_test.cc
+++ b/api/task_queue/task_queue_test.cc
@@ -208,5 +208,37 @@ TEST_P(TaskQueueTest, PostALot) {
   EXPECT_EQ(tasks_cleaned_up, kTaskCount);
 }
 
+// Test posting two tasks that have shared state not protected by a
+// lock. The TaskQueue should guarantee memory read-write order and
+// FIFO task execution order, so the second task should always see the
+// changes that were made by the first task.
+//
+// If the TaskQueue doesn't properly synchronize the execution of
+// tasks, there will be a data race, which is undefined behavior. The
+// EXPECT calls may randomly catch this, but to make the most of this
+// unit test, run it under TSan or some other tool that is able to
+// directly detect data races.
+TEST_P(TaskQueueTest, PostTwoWithSharedUnprotectedState) {
+  struct SharedState {
+    // First task will set this value to 1 and second will assert it.
+    int state = 0;
+  } state;
+
+  auto queue = CreateTaskQueue(GetParam(), "PostTwoWithSharedUnprotectedState");
+  rtc::Event done;
+  queue->PostTask(rtc::NewClosure([&state, &queue, &done] {
+    // Post tasks from queue to guarantee, that 1st task won't be
+    // executed before the second one will be posted.
+    queue->PostTask(rtc::NewClosure([&state] { state.state = 1; }));
+    queue->PostTask(rtc::NewClosure([&state, &done] {
+      EXPECT_EQ(state.state, 1);
+      done.Set();
+    }));
+    // Check, that state changing tasks didn't start yet.
+    EXPECT_EQ(state.state, 0);
+  }));
+  EXPECT_TRUE(done.Wait(1000));
+}
+
 }  // namespace
 }  // namespace webrtc
diff --git a/rtc_base/task_queue_unittest.cc b/rtc_base/task_queue_unittest.cc
index d4ef105c8a..9086206214 100644
--- a/rtc_base/task_queue_unittest.cc
+++ b/rtc_base/task_queue_unittest.cc
@@ -367,4 +367,37 @@ TEST(TaskQueueTest, PostALot) {
   EXPECT_EQ(kTaskCount, tasks_cleaned_up);
 }
 
+// Test posting two tasks that have shared state not protected by a
+// lock. The TaskQueue should guarantee memory read-write order and
+// FIFO task execution order, so the second task should always see the
+// changes that were made by the first task.
+//
+// If the TaskQueue doesn't properly synchronize the execution of
+// tasks, there will be a data race, which is undefined behavior. The
+// EXPECT calls may randomly catch this, but to make the most of this
+// unit test, run it under TSan or some other tool that is able to
+// directly detect data races.
+TEST(TaskQueueTest, PostTwoWithSharedUnprotectedState) {
+  static const char kQueueName[] = "PostTwoWithSharedUnprotectedState";
+  struct SharedState {
+    // First task will set this value to 1 and second will assert it.
+    int state = 0;
+  } state;
+
+  TaskQueue queue(kQueueName);
+  rtc::Event done;
+  queue.PostTask([&state, &queue, &done] {
+    // Post tasks from queue to guarantee, that 1st task won't be
+    // executed before the second one will be posted.
+    queue.PostTask([&state] { state.state = 1; });
+    queue.PostTask([&state, &done] {
+      EXPECT_EQ(state.state, 1);
+      done.Set();
+    });
+    // Check, that state changing tasks didn't start yet.
+    EXPECT_EQ(state.state, 0);
+  });
+  EXPECT_TRUE(done.Wait(1000));
+}
+
 }  // namespace rtc