[pulseaudio-discuss] [PATCH] sink-input, source-output: don't allow moving of streams connected to moving filter devices

[Tanu Kaskinen]

This fixes a crash that was observed in the following scenario:

A phone applications creates playback and recording streams with
filter.want=echo-cancel. An echo-cancel sink gets created. The alsa
card profile has enabled a 4.0 sink, and the user changes the profile
to have a stereo sink instead. A complex sequence of events happens:
the echo-cancel sink input gets detached from the alsa sink, the 4.0
sink gets removed, which triggers loading of a null sink by
module-always-sink, and that in turn triggers rerouting in
module-device-manager, which decides to move the phone stream to the
null sink. Moving a sink input involves sending a message to the IO
thread of the old sink, but in this case the old sink is the
echo-cancel sink, which was detached from the now-dead alsa sink.
Therefore, the echo-cancel sink doesn't have an IO thread associated
with it, and as can be expected, sending a message to a non-existing
thread results in a crash.

The crash can be avoided by disallowing moving streams that are
connected to filter devices that themselves are being moved. The
profile switch still doesn't work smoothly, though, because the
echo-cancel streams don't support moving, so when the old alsa sink
goes away, module-echo-cancel gets unloaded, and since the
echo-cancel sink input got detached before that, the phone sink input
isn't movable either and gets killed. But that's a separate issue.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=93443
---
 src/pulsecore/sink-input.c    | 26 ++++++++++++++++++++++++++
 src/pulsecore/source-output.c | 27 +++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 539ae17..0e01893 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -1528,6 +1528,22 @@ static bool find_filter_sink_input(pa_sink_input *target, pa_sink *s) {
     return false;
 }
 
+static bool is_filter_sink_moving(pa_sink_input *i) {
+    pa_sink *sink = i->sink;
+
+    if (!sink)
+        return false;
+
+    while (sink->input_to_master) {
+        sink = sink->input_to_master->sink;
+
+        if (!sink)
+            return true;
+    }
+
+    return false;
+}
+
 /* Called from main context */
 bool pa_sink_input_may_move_to(pa_sink_input *i, pa_sink *dest) {
     pa_sink_input_assert_ref(i);
@@ -1547,6 +1563,16 @@ bool pa_sink_input_may_move_to(pa_sink_input *i, pa_sink *dest) {
         return false;
     }
 
+    /* If this sink input is connected to a filter sink that itself is moving,
+     * then don't allow the move. Moving requires sending a message to the IO
+     * thread of the old sink, and if the old sink is a filter sink that is
+     * moving, there's no IO thread associated to the old sink. */
+    if (is_filter_sink_moving(i)) {
+        pa_log_debug("Can't move input from filter sink %s, because the filter sink itself is currently moving.",
+                     i->sink->name);
+        return false;
+    }
+
     if (pa_idxset_size(dest->inputs) >= PA_MAX_INPUTS_PER_SINK) {
         pa_log_warn("Failed to move sink input: too many inputs per sink.");
         return false;
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index 9000972..24e9df4 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -1184,6 +1184,22 @@ static bool find_filter_source_output(pa_source_output *target, pa_source *s) {
     return false;
 }
 
+static bool is_filter_source_moving(pa_source_output *o) {
+    pa_source *source = o->source;
+
+    if (!source)
+        return false;
+
+    while (source->output_from_master) {
+        source = source->output_from_master->source;
+
+        if (!source)
+            return true;
+    }
+
+    return false;
+}
+
 /* Called from main context */
 bool pa_source_output_may_move_to(pa_source_output *o, pa_source *dest) {
     pa_source_output_assert_ref(o);
@@ -1202,6 +1218,17 @@ bool pa_source_output_may_move_to(pa_source_output *o, pa_source *dest) {
         return false;
     }
 
+    /* If this source output is connected to a filter source that itself is
+     * moving, then don't allow the move. Moving requires sending a message to
+     * the IO thread of the old source, and if the old source is a filter
+     * source that is moving, there's no IO thread associated to the old
+     * source. */
+    if (is_filter_source_moving(o)) {
+        pa_log_debug("Can't move output from filter source %s, because the filter source itself is currently moving.",
+                     o->source->name);
+        return false;
+    }
+
     if (pa_idxset_size(dest->outputs) >= PA_MAX_OUTPUTS_PER_SOURCE) {
         pa_log_warn("Failed to move source output: too many outputs per source.");
         return false;
-- 
2.6.4