[pulseaudio-discuss] [PATCH] loopback: Fix assertion triggered if move fails

[Tanu Kaskinen]

On Mon, 2012-06-04 at 19:28 +0200, Frédéric Dalleau wrote:
> To reproduce select a card with a source and a sink.
> For example card 0 has sink 0, source 0 and source 1 is the monitor.
> Load the loopback module:
> $ pacmd load-module module-loopback source=0 sink=0
> 
> Make sure the loopback works, then enter the following command:
> $ pacmd set-card-profile 0 off
> 
> The assertion is triggered, the patch should fix it.
> ---
>  src/modules/module-loopback.c |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)

Thanks for reporting this, and special thanks for the precise
instructions for reproducing the bug. I was still, half a year later,
able to reproduce this (although I got a segfault instead of an
assertion crash). I don't agree with the fix, though. It should always
be safe to tear down both the source output and the sink input. I don't
know what assertion you got, but my segfault happened in
source_output_may_move_to(), and the problem turned out to be
dereferencing u->sink_input->sink while the sink input was moving. I
will send a few patches (the instructions that you gave triggered a
couple of different crashes after fixing this one).

-- 
Tanu

> 
> diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
> index 1a69445..17eaba2 100644
> --- a/src/modules/module-loopback.c
> +++ b/src/modules/module-loopback.c
> @@ -132,7 +132,7 @@ enum {
>  };
>  
>  /* Called from main context */
> -static void teardown(struct userdata *u) {
> +static void teardown(struct userdata *u, pa_bool_t teardown_input, pa_bool_t teardown_output) {
>      pa_assert(u);
>      pa_assert_ctl_context();
>  
> @@ -145,19 +145,19 @@ static void teardown(struct userdata *u) {
>          u->time_event = NULL;
>      }
>  
> -    if (u->sink_input)
> +    if (teardown_input && u->sink_input)
>          pa_sink_input_unlink(u->sink_input);
>  
> -    if (u->source_output)
> +    if (teardown_output && u->source_output)
>          pa_source_output_unlink(u->source_output);
>  
> -    if (u->sink_input) {
> +    if (teardown_input && u->sink_input) {
>          u->sink_input->parent.process_msg = pa_sink_input_process_msg;
>          pa_sink_input_unref(u->sink_input);
>          u->sink_input = NULL;
>      }
>  
> -    if (u->source_output) {
> +    if (teardown_output && u->source_output) {
>          u->source_output->parent.process_msg = pa_source_output_process_msg;
>          pa_source_output_unref(u->source_output);
>          u->source_output = NULL;
> @@ -351,7 +351,7 @@ static void source_output_kill_cb(pa_source_output *o) {
>      pa_assert_ctl_context();
>      pa_assert_se(u = o->userdata);
>  
> -    teardown(u);
> +    teardown(u, FALSE, TRUE);
>      pa_module_unload_request(u->module, TRUE);
>  }
>  
> @@ -603,7 +603,7 @@ static void sink_input_kill_cb(pa_sink_input *i) {
>      pa_assert_ctl_context();
>      pa_assert_se(u = i->userdata);
>  
> -    teardown(u);
> +    teardown(u, TRUE, FALSE);
>      pa_module_unload_request(u->module, TRUE);
>  }
>  
> @@ -862,7 +862,7 @@ void pa__done(pa_module*m) {
>      if (!(u = m->userdata))
>          return;
>  
> -    teardown(u);
> +    teardown(u, TRUE, TRUE);
>  
>      if (u->memblockq)
>          pa_memblockq_free(u->memblockq);