From 4280a6451d406c38a65b37209c8f3896efb049e2 Mon Sep 17 00:00:00 2001
From: Sandy Carter <bwrsandman@gmail.com>
Date: Sun, 11 Mar 2018 16:22:53 -0400
Subject: [PATCH] [threading] Simplify and test Fence

Remove atomic boolean in fence. Variable signaled_ is already protected
by mutex.
Remove wait loop with single predicate wait protected with mutex.

Add Fence Signal and Wait tests
Test signaling without waiting.
Test signaling before waiting.
Test signaling twice before waiting.
Test synchronizing threads with fence.

Few REQUIRES were used to test as there are no return codes.
A failing test may hang indefinitely or cause a segfault which would still
register as a fail.
---
 src/xenia/base/testing/threading_test.cc | 53 ++++++++++++++++++++++--
 src/xenia/base/threading.h               | 10 ++---
 2 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/src/xenia/base/testing/threading_test.cc b/src/xenia/base/testing/threading_test.cc
index 37af92c80..5c391b9de 100644
--- a/src/xenia/base/testing/threading_test.cc
+++ b/src/xenia/base/testing/threading_test.cc
@@ -18,9 +18,56 @@ using namespace threading;
 using namespace std::chrono_literals;
 
 TEST_CASE("Fence") {
-  // TODO(bwrsandman):
-  REQUIRE(true);
-}
+  std::unique_ptr<threading::Fence> pFence;
+  std::unique_ptr<threading::HighResolutionTimer> pTimer;
+
+  // Signal without wait
+  pFence = std::make_unique<threading::Fence>();
+  pFence->Signal();
+
+  // Signal once and wait
+  pFence = std::make_unique<threading::Fence>();
+  pFence->Signal();
+  pFence->Wait();
+
+  // Signal twice and wait
+  pFence = std::make_unique<threading::Fence>();
+  pFence->Signal();
+  pFence->Signal();
+  pFence->Wait();
+
+  // Test to synchronize multiple threads
+  std::atomic<int> started(0);
+  std::atomic<int> finished(0);
+  pFence = std::make_unique<threading::Fence>();
+  auto func = [&pFence, &started, &finished] {
+    started.fetch_add(1);
+    pFence->Wait();
+    finished.fetch_add(1);
+  };
+
+  auto threads = std::array<std::thread, 5>({
+      std::thread(func),
+      std::thread(func),
+      std::thread(func),
+      std::thread(func),
+      std::thread(func),
+  });
+
+  Sleep(100ms);
+  REQUIRE(finished.load() == 0);
+
+  // TODO(bwrsandman): Check if this is correct behaviour: looping with Sleep
+  // is the only way to get fence to signal all threads on windows
+  for (int i = 0; i < threads.size(); ++i) {
+    Sleep(10ms);
+    pFence->Signal();
+  }
+  REQUIRE(started.load() == threads.size());
+
+  for (auto& t : threads) t.join();
+  REQUIRE(finished.load() == threads.size());
+}  // namespace test
 
 TEST_CASE("Get number of logical processors") {
   auto count = std::thread::hardware_concurrency();
diff --git a/src/xenia/base/threading.h b/src/xenia/base/threading.h
index fef37dd06..7c635fcea 100644
--- a/src/xenia/base/threading.h
+++ b/src/xenia/base/threading.h
@@ -32,21 +32,19 @@ class Fence {
   Fence() : signaled_(false) {}
   void Signal() {
     std::unique_lock<std::mutex> lock(mutex_);
-    signaled_.store(true);
+    signaled_ = true;
     cond_.notify_all();
   }
   void Wait() {
     std::unique_lock<std::mutex> lock(mutex_);
-    while (!signaled_.load()) {
-      cond_.wait(lock);
-    }
-    signaled_.store(false);
+    cond_.wait(lock, [this] { return signaled_; });
+    signaled_ = false;
   }
 
  private:
   std::mutex mutex_;
   std::condition_variable cond_;
-  std::atomic<bool> signaled_;
+  bool signaled_;
 };
 
 // Returns the total number of logical processors in the host system.