From fe608e32abc166de10671af71ed95e701cd103a8 Mon Sep 17 00:00:00 2001
From: Dmitri Smirnov <dmitrism@microsoft.com>
Date: Thu, 7 Dec 2017 13:41:35 -0800
Subject: [PATCH] Fix a race condition in WindowsThread (port::Thread)

Summary:
Fix a race condition when we create a thread and immediately destroy
 This case should be supported.
  What happens is that the thread function needs the Data instance
  to actually run but has no shared ownership and must rely on the
  WindowsThread instance to continue existing.
  To address this we change unique_ptr to shared_ptr and then
  acquire an additional refcount for the threadproc which destroys it
  just before the thread exit.
  We choose to allocate shared_ptr instance on the heap as this allows
  the original thread to continue w/o waiting for the new thread to start
  running.
Closes https://github.com/facebook/rocksdb/pull/3240

Differential Revision: D6511324

Pulled By: yiwu-arbug

fbshipit-source-id: 4633ff7996daf4d287a9fe34f60c1dd28cf4ff36
---
 port/win/win_thread.cc | 17 ++++++++++++-----
 port/win/win_thread.h  |  2 +-
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/port/win/win_thread.cc b/port/win/win_thread.cc
index e55ca7450..749332032 100644
--- a/port/win/win_thread.cc
+++ b/port/win/win_thread.cc
@@ -39,12 +39,17 @@ struct WindowsThread::Data {
 
 void WindowsThread::Init(std::function<void()>&& func) {
 
-  data_.reset(new Data(std::move(func)));
+  data_ = std::make_shared<Data>(std::move(func));
+  // We create another instance of shared_ptr to get an additional ref
+  // since we may detach and destroy this instance before the threadproc
+  // may start to run. We choose to allocate this additional ref on the heap
+  // so we do not need to synchronize and allow this thread to proceed
+  std::unique_ptr<std::shared_ptr<Data>> th_data(new std::shared_ptr<Data>(data_));
 
   data_->handle_ = _beginthreadex(NULL,
     0,    // stack size
     &Data::ThreadProc,
-    data_.get(),
+    th_data.get(),
     0,   // init flag
     &th_id_);
 
@@ -53,6 +58,7 @@ void WindowsThread::Init(std::function<void()>&& func) {
       std::errc::resource_unavailable_try_again),
       "Unable to create a thread");
   }
+  th_data.release();
 }
 
 WindowsThread::WindowsThread() :
@@ -129,7 +135,7 @@ void WindowsThread::join() {
     assert(false);
     throw std::system_error(static_cast<int>(lastError),
       std::system_category(),
-      "WaitForSingleObjectFailed");
+      "WaitForSingleObjectFailed: thread join");
   }
 
   CloseHandle(reinterpret_cast<HANDLE>(data_->handle_));
@@ -157,8 +163,9 @@ void  WindowsThread::swap(WindowsThread& o) {
 }
 
 unsigned int __stdcall  WindowsThread::Data::ThreadProc(void* arg) {
-  auto data = reinterpret_cast<WindowsThread::Data*>(arg);
-  data->func_();
+  auto ptr = reinterpret_cast<std::shared_ptr<Data>*>(arg);
+  std::unique_ptr<std::shared_ptr<Data>> data(ptr);
+  (*data)->func_();
   _endthreadex(0);
   return 0;
 }
diff --git a/port/win/win_thread.h b/port/win/win_thread.h
index 993cc0273..1d5b225e6 100644
--- a/port/win/win_thread.h
+++ b/port/win/win_thread.h
@@ -28,7 +28,7 @@ class WindowsThread {
 
   struct Data;
 
-  std::unique_ptr<Data>  data_;
+  std::shared_ptr<Data>  data_;
   unsigned int           th_id_;
 
   void Init(std::function<void()>&&);
-- 
GitLab