From 7ab51e85ab226ce5107d1d9d4ed408f91d9058fc Mon Sep 17 00:00:00 2001 From: Seungha Yang Date: Tue, 9 Jun 2020 22:38:28 +0900 Subject: [PATCH] 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: --- sys/wasapi/gstwasapisink.c | 37 +++++++++++++++++++++++++++++++++---- sys/wasapi/gstwasapisink.h | 1 + sys/wasapi/gstwasapisrc.c | 28 ++++++++++++++++++++++++++-- sys/wasapi/gstwasapisrc.h | 1 + 4 files changed, 61 insertions(+), 6 deletions(-) diff --git a/sys/wasapi/gstwasapisink.c b/sys/wasapi/gstwasapisink.c index 0068e7da22..118843536b 100644 --- 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 7806fc3015..76ec38fb11 100644 --- 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 7dac971b84..411c9f501f 100644 --- 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 021ef464b6..3e2c968d87 100644 --- 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 */