commit 0495d0db8f7d8440cb563b8f4eea4468c532c27f
author Vardhan Mudunuru <vardhan@google.com> Fri May 17 16:27:25 2019 -0700
committer Vardhan Mudunuru <vardhan@google.com> Sat May 18 01:59:54 2019 +0000
tree 8eb06933acdc2d29f2a7a8a4208e1fda98d2f246
parent 9a8bc896a115c956ea7bb299ce66983f7e3ac7ef

[modular] Fix race condition in PseudoDirServer

Sometimes, the child thread was calling .signal() even before parent
thread kicked off a .wait().

The fix is to use condition variables the correct way -- have the child
use use the lock :)

Test: Verified that it doesnt flake by running:
fx run-test peridot_tests -- --gtest_filter='ModularConfigReaderTest.OverrideConfigDir' --gtest_repeat=2000

MF-419 #done

Change-Id: Ib74317da4478d9539070a74815b3b8ad5f08a45c

peridot/lib/util/pseudo_dir_server.cc

diff --git a/peridot/lib/util/pseudo_dir_server.cc b/peridot/lib/util/pseudo_dir_server.cc
index e9662be..65439d9 100644
--- a/peridot/lib/util/pseudo_dir_server.cc
+++ b/peridot/lib/util/pseudo_dir_server.cc
@@ -13,10 +13,10 @@
             StartThread(std::move(request));
           },
           dir_.NewRequest()) {
-  // The thread (owned by |serving_thread_|) kicked off, but lets wait until
-  // it's run loop is ready.
+  // Block until the run loop in |serving_thread_| is ready before returning
+  // control to the caller.
   std::unique_lock<std::mutex> lock(ready_mutex_);
-  thread_loop_ready_.wait(lock);
+  thread_loop_ready_.wait(lock, [this] { return thread_loop_ != nullptr; });
 }
 
 PseudoDirServer::~PseudoDirServer() {
@@ -43,14 +43,17 @@
 void PseudoDirServer::StartThread(
     fidl::InterfaceRequest<fuchsia::io::Directory> request) {
   async::Loop loop(&kAsyncLoopConfigAttachToThread);
-  thread_loop_ = &loop;
 
-  // The owner's thread is currently blocked waiting for this thread to
-  // initialize a valid |thread_loop_|. Notify the owner thread that it's now
-  // safe to manipulate this thread's run loop:
-  thread_loop_ready_.notify_all();
+  {
+    std::lock_guard<std::mutex> lock(ready_mutex_);
 
-  pseudo_dir_->Serve(fuchsia::io::OPEN_RIGHT_READABLE, request.TakeChannel());
+    // Setting this will let |thread_loop_ready_.wait()| proceed.
+    thread_loop_ = &loop;
+
+    pseudo_dir_->Serve(fuchsia::io::OPEN_RIGHT_READABLE, request.TakeChannel());
+    thread_loop_ready_.notify_one();
+  }
+
   thread_loop_->Run();
   // This thread exits when the owner thread calls thread_loop_->Quit().
 }