wasapi: Fix possible deadlock while downwards state change

IAudioClient::Stop() doesn't seem to wake up the event handle,
then read() or write() could be blocked forever by WaitForSingleObject.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/1329>

diff --git a/sys/wasapi/gstwasapisink.c b/sys/wasapi/gstwasapisink.c
index 0068e7d..1188435 100644 (file)
--- a/sys/wasapi/gstwasapisink.c
+++ b/sys/wasapi/gstwasapisink.c
@@ -175,6 +175,7 @@ gst_wasapi_sink_init (GstWasapiSink * self)
   self->low_latency = DEFAULT_LOW_LATENCY;
   self->try_audioclient3 = DEFAULT_AUDIOCLIENT3;
   self->event_handle = CreateEvent (NULL, FALSE, FALSE, NULL);
+  self->cancellable = CreateEvent (NULL, TRUE, FALSE, NULL);
   self->client_needs_restart = FALSE;
 
   CoInitializeEx (NULL, COINIT_MULTITHREADED);
@@ -190,6 +191,11 @@ gst_wasapi_sink_dispose (GObject * object)
     self->event_handle = NULL;
   }
 
+  if (self->cancellable != NULL) {
+    CloseHandle (self->cancellable);
+    self->cancellable = NULL;
+  }
+
   if (self->client != NULL) {
     IUnknown_Release (self->client);
     self->client = NULL;
@@ -564,6 +570,9 @@ gst_wasapi_sink_prepare (GstAudioSink * asink, GstAudioRingBufferSpec * spec)
 
   res = TRUE;
 
+  /* reset cancellable event handle */
+  ResetEvent (self->cancellable);
+
 beach:
   /* unprepare() is not called if prepare() fails, but we want it to be, so call
    * it manually when needed */
@@ -600,6 +609,10 @@ gst_wasapi_sink_write (GstAudioSink * asink, gpointer data, guint length)
   gint16 *dst = NULL;
   DWORD dwWaitResult;
   guint can_frames, have_frames, n_frames, write_len, written_len = 0;
+  HANDLE event_handle[2];
+
+  event_handle[0] = self->event_handle;
+  event_handle[1] = self->cancellable;
 
   GST_OBJECT_LOCK (self);
   if (self->client_needs_restart) {
@@ -607,6 +620,7 @@ gst_wasapi_sink_write (GstAudioSink * asink, gpointer data, guint length)
     HR_FAILED_ELEMENT_ERROR_AND (hr, IAudioClient::Start, self,
         GST_OBJECT_UNLOCK (self); goto err);
     self->client_needs_restart = FALSE;
+    ResetEvent (self->cancellable);
   }
   GST_OBJECT_UNLOCK (self);
 
@@ -615,13 +629,19 @@ gst_wasapi_sink_write (GstAudioSink * asink, gpointer data, guint length)
 
   if (self->sharemode == AUDCLNT_SHAREMODE_EXCLUSIVE) {
     /* In exclusive mode we have to wait always */
-    dwWaitResult = WaitForSingleObject (self->event_handle, INFINITE);
-    if (dwWaitResult != WAIT_OBJECT_0) {
+    dwWaitResult = WaitForMultipleObjects (2, event_handle, FALSE, INFINITE);
+    if (dwWaitResult != WAIT_OBJECT_0 && dwWaitResult != WAIT_OBJECT_0 + 1) {
       GST_ERROR_OBJECT (self, "Error waiting for event handle: %x",
           (guint) dwWaitResult);
       goto err;
     }
 
+    /* ::reset was requested */
+    if (dwWaitResult == WAIT_OBJECT_0 + 1) {
+      GST_DEBUG_OBJECT (self, "operation was cancelled");
+      return -1;
+    }
+
     can_frames = gst_wasapi_sink_get_can_frames (self);
     if (can_frames < 0) {
       GST_ERROR_OBJECT (self, "Error getting frames to write to");
@@ -645,12 +665,19 @@ gst_wasapi_sink_write (GstAudioSink * asink, gpointer data, guint length)
     }
 
     if (can_frames == 0) {
-      dwWaitResult = WaitForSingleObject (self->event_handle, INFINITE);
-      if (dwWaitResult != WAIT_OBJECT_0) {
+      dwWaitResult = WaitForMultipleObjects (2, event_handle, FALSE, INFINITE);
+      if (dwWaitResult != WAIT_OBJECT_0 && dwWaitResult != WAIT_OBJECT_0 + 1) {
         GST_ERROR_OBJECT (self, "Error waiting for event handle: %x",
             (guint) dwWaitResult);
         goto err;
       }
+
+      /* ::reset was requested */
+      if (dwWaitResult == WAIT_OBJECT_0 + 1) {
+        GST_DEBUG_OBJECT (self, "operation was cancelled");
+        return -1;
+      }
+
       can_frames = gst_wasapi_sink_get_can_frames (self);
       if (can_frames < 0) {
         GST_ERROR_OBJECT (self, "Error getting frames to write to");
@@ -713,6 +740,8 @@ gst_wasapi_sink_reset (GstAudioSink * asink)
   if (!self->client)
     return;
 
+  SetEvent (self->cancellable);
+
   GST_OBJECT_LOCK (self);
   hr = IAudioClient_Stop (self->client);
   HR_FAILED_AND (hr, IAudioClient::Stop,);

diff --git a/sys/wasapi/gstwasapisink.h b/sys/wasapi/gstwasapisink.h
index 7806fc3..76ec38f 100644 (file)
--- a/sys/wasapi/gstwasapisink.h
+++ b/sys/wasapi/gstwasapisink.h
@@ -44,6 +44,7 @@ struct _GstWasapiSink
   IAudioClient *client;
   IAudioRenderClient *render_client;
   HANDLE event_handle;
+  HANDLE cancellable;
   /* Client was reset, so it needs to be started again */
   gboolean client_needs_restart;
 

diff --git a/sys/wasapi/gstwasapisrc.c b/sys/wasapi/gstwasapisrc.c
index 7dac971..411c9f5 100644 (file)
--- a/sys/wasapi/gstwasapisrc.c
+++ b/sys/wasapi/gstwasapisrc.c
@@ -189,6 +189,7 @@ gst_wasapi_src_init (GstWasapiSrc * self)
   self->low_latency = DEFAULT_LOW_LATENCY;
   self->try_audioclient3 = DEFAULT_AUDIOCLIENT3;
   self->event_handle = CreateEvent (NULL, FALSE, FALSE, NULL);
+  self->cancellable = CreateEvent (NULL, TRUE, FALSE, NULL);
   self->client_needs_restart = FALSE;
   self->adapter = gst_adapter_new ();
 
@@ -205,6 +206,11 @@ gst_wasapi_src_dispose (GObject * object)
     self->event_handle = NULL;
   }
 
+  if (self->cancellable != NULL) {
+    CloseHandle (self->cancellable);
+    self->cancellable = NULL;
+  }
+
   if (self->client_clock != NULL) {
     IUnknown_Release (self->client_clock);
     self->client_clock = NULL;
@@ -516,7 +522,12 @@ gst_wasapi_src_prepare (GstAudioSrc * asrc, GstAudioRingBufferSpec * spec)
       (self)->ringbuffer, self->positions);
 
   res = TRUE;
+
+  /* reset cancellable event handle */
+  ResetEvent (self->cancellable);
+
 beach:
+
   /* unprepare() is not called if prepare() fails, but we want it to be, so call
    * it manually when needed */
   if (!res)
@@ -568,6 +579,7 @@ gst_wasapi_src_read (GstAudioSrc * asrc, gpointer data, guint length,
     HR_FAILED_ELEMENT_ERROR_AND (hr, IAudioClient::Start, self,
         GST_OBJECT_UNLOCK (self); goto err);
     self->client_needs_restart = FALSE;
+    ResetEvent (self->cancellable);
     gst_adapter_clear (self->adapter);
   }
 
@@ -585,15 +597,25 @@ gst_wasapi_src_read (GstAudioSrc * asrc, gpointer data, guint length,
   while (wanted > 0) {
     DWORD dwWaitResult;
     guint got_frames, avail_frames, n_frames, want_frames, read_len;
+    HANDLE event_handle[2];
+
+    event_handle[0] = self->event_handle;
+    event_handle[1] = self->cancellable;
 
     /* Wait for data to become available */
-    dwWaitResult = WaitForSingleObject (self->event_handle, INFINITE);
-    if (dwWaitResult != WAIT_OBJECT_0) {
+    dwWaitResult = WaitForMultipleObjects (2, event_handle, FALSE, INFINITE);
+    if (dwWaitResult != WAIT_OBJECT_0 && dwWaitResult != WAIT_OBJECT_0 + 1) {
       GST_ERROR_OBJECT (self, "Error waiting for event handle: %x",
           (guint) dwWaitResult);
       goto err;
     }
 
+    /* ::reset was requested */
+    if (dwWaitResult == WAIT_OBJECT_0 + 1) {
+      GST_DEBUG_OBJECT (self, "operation was cancelled");
+      return -1;
+    }
+
     hr = IAudioCaptureClient_GetBuffer (self->capture_client,
         (BYTE **) & from, &got_frames, &flags, NULL, NULL);
     if (hr != S_OK) {
@@ -685,6 +707,8 @@ gst_wasapi_src_reset (GstAudioSrc * asrc)
   if (!self->client)
     return;
 
+  SetEvent (self->cancellable);
+
   GST_OBJECT_LOCK (self);
   hr = IAudioClient_Stop (self->client);
   HR_FAILED_RET (hr, IAudioClock::Stop,);

diff --git a/sys/wasapi/gstwasapisrc.h b/sys/wasapi/gstwasapisrc.h
index 021ef46..3e2c968 100644 (file)
--- a/sys/wasapi/gstwasapisrc.h
+++ b/sys/wasapi/gstwasapisrc.h
@@ -46,6 +46,7 @@ struct _GstWasapiSrc
   guint64 client_clock_freq;
   IAudioCaptureClient *capture_client;
   HANDLE event_handle;
+  HANDLE cancellable;
   /* Smooth frames captured from WASAPI, which can be irregular sometimes */
   GstAdapter *adapter;
   /* Client was reset, so it needs to be started again */