[pulseaudio-discuss] [PATCH 2/7] don't move streams to devices that are going away

[Tanu Kaskinen]

Before a device is unlinked, the unlink hook is fired, and it's
possible that a routing module tries to move streams to the unlinked
device in that hook, because it doesn't know that the device is being
unlinked. Of course, the unlinking is obvious when the code is in an
unlink hook callback, but it's possible that some other module does
something in the unlink hook that in turn triggers some other hook,
and it's this second hook where the routing module may get confused.
This patch adds an "unlink_requested" flag that is set before the
unlink hook is fired, and moving streams to a device with that flag
set is prevented.

This patch is motivated by seeing module-device-manager moving a
stream to a sink that was being unlinked. It was a complex case where
an alsa card was changing its profile, while an echo-cancel sink was
connected to the old alsa sink. module-always-sink loaded a null sink
in the middle of the profile change, and after a stream had been
rescued to the null sink, module-device-manager decided to move it
back to the old alsa sink that was being unlinked. That move made no
sense, so I came up with this patch.
---
 src/pulsecore/sink-input.c    | 3 +++
 src/pulsecore/sink.c          | 7 ++++---
 src/pulsecore/sink.h          | 6 ++++++
 src/pulsecore/source-output.c | 3 +++
 src/pulsecore/source.c        | 5 +++++
 src/pulsecore/source.h        | 6 ++++++
 6 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 8ec63b5..843297f 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -1538,6 +1538,9 @@ bool pa_sink_input_may_move_to(pa_sink_input *i, pa_sink *dest) {
     if (dest == i->sink)
         return true;
 
+    if (dest->unlink_requested)
+        return false;
+
     if (!pa_sink_input_may_move(i))
         return false;
 
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 0b44fc7..3f1ef72 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -676,9 +676,10 @@ void pa_sink_unlink(pa_sink* s) {
      * reversing pa_sink_put(). It also undoes the registrations
      * already done in pa_sink_new()! */
 
-    /* All operations here shall be idempotent, i.e. pa_sink_unlink()
-     * may be called multiple times on the same sink without bad
-     * effects. */
+    if (s->unlink_requested)
+        return;
+
+    s->unlink_requested = true;
 
     linked = PA_SINK_IS_LINKED(s->state);
 
diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
index 5df109e..b64a666 100644
--- a/src/pulsecore/sink.h
+++ b/src/pulsecore/sink.h
@@ -63,6 +63,12 @@ struct pa_sink {
     pa_core *core;
 
     pa_sink_state_t state;
+
+    /* Set in the beginning of pa_sink_unlink() before setting the sink state
+     * to UNLINKED. The purpose is to prevent moving streams to a sink that is
+     * about to be removed. */
+    bool unlink_requested;
+
     pa_sink_flags_t flags;
     pa_suspend_cause_t suspend_cause;
 
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index 9217ad4..6d54ae8 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -1187,6 +1187,9 @@ bool pa_source_output_may_move_to(pa_source_output *o, pa_source *dest) {
     if (dest == o->source)
         return true;
 
+    if (dest->unlink_requested)
+        return false;
+
     if (!pa_source_output_may_move(o))
         return false;
 
diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index f4b96ab..98374ae 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -618,6 +618,11 @@ void pa_source_unlink(pa_source *s) {
     /* See pa_sink_unlink() for a couple of comments how this function
      * works. */
 
+    if (s->unlink_requested)
+        return;
+
+    s->unlink_requested = true;
+
     linked = PA_SOURCE_IS_LINKED(s->state);
 
     if (linked)
diff --git a/src/pulsecore/source.h b/src/pulsecore/source.h
index 19fb41b..91e8674 100644
--- a/src/pulsecore/source.h
+++ b/src/pulsecore/source.h
@@ -64,6 +64,12 @@ struct pa_source {
     pa_core *core;
 
     pa_source_state_t state;
+
+    /* Set in the beginning of pa_source_unlink() before setting the source
+     * state to UNLINKED. The purpose is to prevent moving streams to a source
+     * that is about to be removed. */
+    bool unlink_requested;
+
     pa_source_flags_t flags;
     pa_suspend_cause_t suspend_cause;
 
-- 
2.7.0