CubebStream: Use WorkQueueThread::PushBlocking instead of sync_event

Push and wait on WorkQueueThread items using PushBlocking. Previously we
created a Common::Event sync_event on the caller's stack, called Wait on
it, then had the WorkQueueThread call Set on the sync_event once the
thread was done.

In addition to being simpler the new way avoids a use-after-free that
could happen in convoluted and unlikely yet possible thread scheduling
sequences.

One such case can be triggered as follows:

* Set your audio backend to Cubeb
* In CubebStream::SetVolume set a breakpoint at the call to Wait and at
  the call to cubeb_stream_set_volume.
* Start a game.
* Continue until the Cubeb Worker thread hits the
  cubeb_stream_set_volume breakpoint and Emuthread hits the Wait
  breakpoint, freezing each thread when it hits its breakpoint.
* Unfreeze Cubeb Worker.
* In Event::Set set a breakpoint at the end of the scope containing the
  lock_guard such that the guard has been constructed but not destructed
  when the breakpoint is hit.
* Continue until that breakpoint is hit by Cubeb Worker. If other
  threads hit it first keep going.
* Freeze Cubeb Worker.
* For convenience remove the breakpoint in Event::Set so other threads
  don't trigger it.
* In CubebStream::SetRunning set a breakpoint at the call to Wait.
* Unfreeze Emuthread and continue until the breakpoint is hit.
* In Cubeb Worker go to Event::Set and examine the values of m_mutex's
  member variables. In Visual Studio Debug these are locking_thread_id
  == 0xcccccc01 and ownership_levels == 0xcccccccc. This is the result
  of Visual Studio overwriting the memory used on the stack by
  sync_event in CubebStream::SetVolume with cc bytes to represent
  uninitialized memory on the stack (since that function already
  returned), and then allocating enough memory on the stack when calling
  AudioCommon::SetSoundStreamRunning and then CubebStream::SetRunning
  that it overwrote one byte of the memory formerly occupied by
  locking_thread_id.
* If you unfreeze Cubeb Worker at this point it will trigger the lock
  guard's destructor which will then try to unlock m_mutex. Since
  m_mutex is no longer in scope this is a use-after-free, and in VS
  debug triggers a debug assert due to locking_thread_id not matching
  the current thread id.

Source/Core/AudioCommon/CubebStream.cpp

@@ -40,14 +40,11 @@ CubebStream::CubebStream()
 #ifdef _WIN32
     : m_work_queue("Cubeb Worker")
 {
-  Common::Event sync_event;
+  m_work_queue.PushBlocking([this] {
-  m_work_queue.Push([this, &sync_event] {
-    Common::ScopeGuard sync_event_guard([&sync_event] { sync_event.Set(); });
     auto const result = CoInitializeEx(nullptr, COINIT_MULTITHREADED | COINIT_DISABLE_OLE1DDE);
     m_coinit_success = result == S_OK;
     m_should_couninit = result == S_OK || result == S_FALSE;
   });
-  sync_event.Wait();
 }
 #else
     = default;
@@ -60,11 +57,8 @@ bool CubebStream::Init()
 #ifdef _WIN32
   if (!m_coinit_success)
     return false;
-  Common::Event sync_event;
+  m_work_queue.PushBlocking([this, &return_value] {
-  m_work_queue.Push([this, &return_value, &sync_event] {
-    Common::ScopeGuard sync_event_guard([&sync_event] { sync_event.Set(); });
 #endif
-
     m_ctx = CubebUtils::GetContext();
     if (m_ctx)
     {
@@ -98,7 +92,6 @@ bool CubebStream::Init()
 
 #ifdef _WIN32
   });
-  sync_event.Wait();
 #endif
 
   return return_value;
@@ -111,9 +104,7 @@ bool CubebStream::SetRunning(bool running)
 #ifdef _WIN32
   if (!m_coinit_success)
     return false;
-  Common::Event sync_event;
+  m_work_queue.PushBlocking([this, running, &return_value] {
-  m_work_queue.Push([this, running, &return_value, &sync_event] {
-    Common::ScopeGuard sync_event_guard([&sync_event] { sync_event.Set(); });
 #endif
     if (running)
       return_value = cubeb_stream_start(m_stream) == CUBEB_OK;
@@ -121,7 +112,6 @@ bool CubebStream::SetRunning(bool running)
       return_value = cubeb_stream_stop(m_stream) == CUBEB_OK;
 #ifdef _WIN32
   });
-  sync_event.Wait();
 #endif
 
   return return_value;
@@ -130,9 +120,7 @@ bool CubebStream::SetRunning(bool running)
 CubebStream::~CubebStream()
 {
 #ifdef _WIN32
-  Common::Event sync_event;
+  m_work_queue.PushBlocking([this] {
-  m_work_queue.Push([this, &sync_event] {
-    Common::ScopeGuard sync_event_guard([&sync_event] { sync_event.Set(); });
 #endif
     cubeb_stream_stop(m_stream);
     cubeb_stream_destroy(m_stream);
@@ -144,7 +132,6 @@ CubebStream::~CubebStream()
     }
     m_coinit_success = false;
   });
-  sync_event.Wait();
 #endif
   m_ctx.reset();
 }
@@ -154,13 +141,10 @@ void CubebStream::SetVolume(int volume)
 #ifdef _WIN32
   if (!m_coinit_success)
     return;
-  Common::Event sync_event;
+  m_work_queue.PushBlocking([this, volume] {
-  m_work_queue.Push([this, volume, &sync_event] {
-    Common::ScopeGuard sync_event_guard([&sync_event] { sync_event.Set(); });
 #endif
     cubeb_stream_set_volume(m_stream, volume / 100.0f);
 #ifdef _WIN32
   });
-  sync_event.Wait();
 #endif
 }

Source/Core/Core/HW/EXI/EXI_DeviceMic.cpp

@@ -36,14 +36,11 @@ void CEXIMic::StreamInit()
 #ifdef _WIN32
   if (!m_coinit_success)
     return;
-  Common::Event sync_event;
+  m_work_queue.PushBlocking([this] {
-  m_work_queue.Push([this, &sync_event] {
-    Common::ScopeGuard sync_event_guard([&sync_event] { sync_event.Set(); });
 #endif
     m_cubeb_ctx = CubebUtils::GetContext();
 #ifdef _WIN32
   });
-  sync_event.Wait();
 #endif
 }
 
@@ -56,14 +53,11 @@ void CEXIMic::StreamTerminate()
 #ifdef _WIN32
     if (!m_coinit_success)
       return;
-    Common::Event sync_event;
+    m_work_queue.PushBlocking([this] {
-    m_work_queue.Push([this, &sync_event] {
-      Common::ScopeGuard sync_event_guard([&sync_event] { sync_event.Set(); });
 #endif
       m_cubeb_ctx.reset();
 #ifdef _WIN32
     });
-    sync_event.Wait();
 #endif
   }
 }
@@ -104,9 +98,7 @@ void CEXIMic::StreamStart()
 #ifdef _WIN32
   if (!m_coinit_success)
     return;
-  Common::Event sync_event;
+  m_work_queue.PushBlocking([this] {
-  m_work_queue.Push([this, &sync_event] {
-    Common::ScopeGuard sync_event_guard([&sync_event] { sync_event.Set(); });
 #endif
     // Open stream with current parameters
     stream_size = buff_size_samples * 500;
@@ -142,7 +134,6 @@ void CEXIMic::StreamStart()
     INFO_LOG_FMT(EXPANSIONINTERFACE, "started cubeb stream");
 #ifdef _WIN32
   });
-  sync_event.Wait();
 #endif
 }
 
@@ -151,9 +142,7 @@ void CEXIMic::StreamStop()
   if (m_cubeb_stream)
   {
 #ifdef _WIN32
-    Common::Event sync_event;
+    m_work_queue.PushBlocking([this] {
-    m_work_queue.Push([this, &sync_event] {
-      Common::ScopeGuard sync_event_guard([&sync_event] { sync_event.Set(); });
 #endif
       if (cubeb_stream_stop(m_cubeb_stream) != CUBEB_OK)
         ERROR_LOG_FMT(EXPANSIONINTERFACE, "Error stopping cubeb stream");
@@ -161,7 +150,6 @@ void CEXIMic::StreamStop()
       m_cubeb_stream = nullptr;
 #ifdef _WIN32
     });
-    sync_event.Wait();
 #endif
   }
 
@@ -217,14 +205,11 @@ CEXIMic::CEXIMic(Core::System& system, int index)
   next_int_ticks = 0;
 
 #ifdef _WIN32
-  Common::Event sync_event;
+  m_work_queue.PushBlocking([this] {
-  m_work_queue.Push([this, &sync_event] {
-    Common::ScopeGuard sync_event_guard([&sync_event] { sync_event.Set(); });
     auto result = ::CoInitializeEx(nullptr, COINIT_MULTITHREADED | COINIT_DISABLE_OLE1DDE);
     m_coinit_success = result == S_OK;
     m_should_couninit = result == S_OK || result == S_FALSE;
   });
-  sync_event.Wait();
 #endif
 
   StreamInit();
@@ -237,13 +222,10 @@ CEXIMic::~CEXIMic()
 #ifdef _WIN32
   if (m_should_couninit)
   {
-    Common::Event sync_event;
+    m_work_queue.PushBlocking([this] {
-    m_work_queue.Push([this, &sync_event] {
-      Common::ScopeGuard sync_event_guard([&sync_event] { sync_event.Set(); });
       m_should_couninit = false;
       CoUninitialize();
     });
-    sync_event.Wait();
   }
   m_coinit_success = false;
 #endif