[pulseaudio-discuss] [PATCH] sink, source: Don't finish move if unlink happens after pa_*_move_all_start()

[Tanu Kaskinen]

On Sat, 2017-12-02 at 23:19 +0100, Georg Chini wrote:
> When a sink input is unlinked between the calls to pa_sink_move_all_start() and
> pa_sink_move_all_finish(), pa_sink_move_all_finish() tried to finish the move
> of the already unlinked sink input, which lead to an assertion in
> pa_sink_input_finish_move(). The same applies for the source side.
> 
> This patch fixes the problem by checking the state of the sink input or
> source output in pa_*_move_all_finish().
> 
> Bug report: https://bugs.freedesktop.org/show_bug.cgi?id=103752
> ---
>  src/pulsecore/sink.c   | 9 ++++++---
>  src/pulsecore/source.c | 9 ++++++---
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
> index 017b9539..8b13aecd 100644
> --- a/src/pulsecore/sink.c
> +++ b/src/pulsecore/sink.c
> @@ -920,10 +920,13 @@ void pa_sink_move_all_finish(pa_sink *s, pa_queue *q, bool save) {
>      pa_assert(q);
>  
>      while ((i = PA_SINK_INPUT(pa_queue_pop(q)))) {
> -        if (pa_sink_input_finish_move(i, s, save) < 0)
> -            pa_sink_input_fail_move(i);
> +        if (PA_SINK_INPUT_IS_LINKED(i->state)) {
> +            if (pa_sink_input_finish_move(i, s, save) < 0)
> +                pa_sink_input_fail_move(i);
>  
> -        pa_sink_input_unref(i);
> +            pa_sink_input_unref(i);
> +        } else
> +            pa_sink_input_unref(i);
>      }
>  
>      pa_queue_free(q, NULL);
> diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
> index d579c357..64617024 100644
> --- a/src/pulsecore/source.c
> +++ b/src/pulsecore/source.c
> @@ -860,10 +860,13 @@ void pa_source_move_all_finish(pa_source *s, pa_queue *q, bool save) {
>      pa_assert(q);
>  
>      while ((o = PA_SOURCE_OUTPUT(pa_queue_pop(q)))) {
> -        if (pa_source_output_finish_move(o, s, save) < 0)
> -            pa_source_output_fail_move(o);
> +        if (PA_SOURCE_OUTPUT_IS_LINKED(o->state)) {
> +            if (pa_source_output_finish_move(o, s, save) < 0)
> +                pa_source_output_fail_move(o);
>  
> -        pa_source_output_unref(o);
> +            pa_source_output_unref(o);
> +        } else
> +            pa_source_output_unref(o);
>      }
>  
>      pa_queue_free(q, NULL);

The unref() calls are unnecessarily duplicated in both of the if-else
branches. Otherwise looks good, thanks for the fix!

-- 
Tanu

https://www.patreon.com/tanuk