From a5d08e347e4fe938dcd27c50800e08554cd7fe93 Mon Sep 17 00:00:00 2001 From: Hector Martin Date: Thu, 10 Dec 2020 01:27:55 +0900 Subject: [PATCH] linux-pulseaudio: fix race conditions PulseAudio code needs to be called with the PA lock held. This chiefly fixes multiple races during stream shutdown: * If the functions are called without the lock held, deferred event handling races end up with PA infinite looping on the mainloop, or asserting, or other badness, as the reentrant calls cause data structure corruption on the PA side. * If we don't reset our callbacks, PA might call us even after we request stream disconnection (since the stream actually getting fully shut down is asynchronous), and then we dereference NULL pointers from our userdata etc. PA will keep its data structures alive until necessary via reference counting, but not ours. The lock around pa_stream_begin_write doesn't result from any issues I experienced, but it looks correct; PA doesn't say anywhere that that function is thread-safe. --- libobs/audio-monitoring/pulse/pulseaudio-output.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/libobs/audio-monitoring/pulse/pulseaudio-output.c b/libobs/audio-monitoring/pulse/pulseaudio-output.c index e9cd22cf6..9abfcd78b 100644 --- a/libobs/audio-monitoring/pulse/pulseaudio-output.c +++ b/libobs/audio-monitoring/pulse/pulseaudio-output.c @@ -183,8 +183,10 @@ static void do_stream_write(void *param) if (bytesToFill > data->bytesRemaining) bytesToFill = data->bytesRemaining; + pulseaudio_lock(); pa_stream_begin_write(data->stream, (void **)&buffer, &bytesToFill); + pulseaudio_unlock(); circlebuf_pop_front(&data->new_data, buffer, bytesToFill); @@ -331,8 +333,20 @@ skip: static void pulseaudio_stop_playback(struct audio_monitor *monitor) { if (monitor->stream) { + /* Stop the stream */ + pulseaudio_lock(); pa_stream_disconnect(monitor->stream); + pulseaudio_unlock(); + + /* Remove the callbacks, to ensure we no longer try to do anything + * with this stream object */ + pulseaudio_write_callback(monitor->stream, NULL, NULL); + pulseaudio_set_underflow_callback(monitor->stream, NULL, NULL); + + /* Unreference the stream and drop it. PA will free it when it can. */ + pulseaudio_lock(); pa_stream_unref(monitor->stream); + pulseaudio_unlock(); monitor->stream = NULL; }