Fixed a deadlock caused by the UI thread dropping important messages

venkatarajasekhar · Aug 3, 2016 · 83f7cc2 · 83f7cc2
1 parent a7e4bc8
commit 83f7cc2
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 34 deletions.
diff --git a/src/xenia/ui/loop_win.cc b/src/xenia/ui/loop_win.cc
@@ -14,18 +14,6 @@
 namespace xe {
 namespace ui {
 
-const DWORD kWmWin32LoopPost = WM_APP + 0x100;
-const DWORD kWmWin32LoopQuit = WM_APP + 0x101;
-
-class PostedFn {
- public:
-  explicit PostedFn(std::function<void()> fn) : fn_(std::move(fn)) {}
-  void Call() { fn_(); }
-
- private:
-  std::function<void()> fn_;
-};
-
 std::unique_ptr<Loop> Loop::Create() { return std::make_unique<Win32Loop>(); }
 
 Win32Loop::Win32Loop() : thread_id_(0) {
@@ -64,22 +52,20 @@ Win32Loop::~Win32Loop() {
 
 void Win32Loop::ThreadMain() {
   MSG msg;
-  while (GetMessage(&msg, nullptr, 0, 0)) {
-    TranslateMessage(&msg);
-    DispatchMessage(&msg);
-    switch (msg.message) {
-      case kWmWin32LoopPost:
-        if (msg.wParam == reinterpret_cast<WPARAM>(this)) {
-          auto posted_fn = reinterpret_cast<PostedFn*>(msg.lParam);
-          posted_fn->Call();
-          delete posted_fn;
-        }
-        break;
-      case kWmWin32LoopQuit:
-        if (msg.wParam == reinterpret_cast<WPARAM>(this)) {
-          return;
-        }
-        break;
+  while (!should_exit_) {
+    DWORD result =
+        MsgWaitForMultipleObjectsEx(0, nullptr, INFINITE, QS_ALLEVENTS, 0);
+
+    if (PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) {
+      TranslateMessage(&msg);
+      DispatchMessage(&msg);
+    }
+
+    // Process queued functions.
+    std::lock_guard<std::mutex> lock(posted_functions_mutex_);
+    for (auto it = posted_functions_.begin(); it != posted_functions_.end();) {
+      (*it).Call();
+      it = posted_functions_.erase(it);
     }
   }
 
@@ -93,10 +79,14 @@ bool Win32Loop::is_on_loop_thread() {
 
 void Win32Loop::Post(std::function<void()> fn) {
   assert_true(thread_id_ != 0);
-  if (!PostThreadMessage(
-          thread_id_, kWmWin32LoopPost, reinterpret_cast<WPARAM>(this),
-          reinterpret_cast<LPARAM>(new PostedFn(std::move(fn))))) {
-    assert_always("Unable to post message to thread queue");
+  {
+    std::lock_guard<std::mutex> lock(posted_functions_mutex_);
+    PostedFn posted_fn(fn);
+    posted_functions_.push_back(posted_fn);
+  }
+
+  while (!PostThreadMessage(thread_id_, WM_NULL, 0, 0)) {
+    Sleep(1);
   }
 }
 
@@ -134,8 +124,10 @@ void Win32Loop::PostDelayed(std::function<void()> fn, uint64_t delay_millis) {
 
 void Win32Loop::Quit() {
   assert_true(thread_id_ != 0);
-  PostThreadMessage(thread_id_, kWmWin32LoopQuit,
-                    reinterpret_cast<WPARAM>(this), 0);
+  should_exit_ = true;
+  while (!PostThreadMessage(thread_id_, WM_NULL, 0, 0)) {
+    Sleep(1);
+  }
 }
 
 void Win32Loop::AwaitQuit() { quit_fence_.Wait(); }

diff --git a/src/xenia/ui/loop_win.h b/src/xenia/ui/loop_win.h
@@ -42,17 +42,30 @@ class Win32Loop : public Loop {
     std::function<void()> fn;
   };
 
+  class PostedFn {
+   public:
+    explicit PostedFn(std::function<void()> fn) : fn_(std::move(fn)) {}
+    void Call() { fn_(); }
+
+   private:
+    std::function<void()> fn_;
+  };
+
   void ThreadMain();
 
   static void TimerQueueCallback(void* context, uint8_t);
 
   std::thread thread_;
   DWORD thread_id_;
+  bool should_exit_ = false;
   xe::threading::Fence quit_fence_;
 
   HANDLE timer_queue_;
   std::mutex pending_timers_mutex_;
   std::list<PendingTimer*> pending_timers_;
+
+  std::mutex posted_functions_mutex_;
+  std::list<PostedFn> posted_functions_;
 };
 
 }  // namespace ui