[pulseaudio-discuss] [PATCH] switch-on-connect: Don't move a stream already moving

[Tanu Kaskinen]

On Thu, 2012-06-07 at 11:16 +0200, Frédéric Dalleau wrote:
> When switching profiles, module-bluetooth starts to move all streams
> from the destroyed sink (resp. source) to the newly created sink (resp. source).

As a sidenote, module-bluetooth-device shouldn't move any streams. It's
the job of the policy modules to decide when to move streams.

> At the same time, module-switch-on-connect detects that a new sink (resp. source)
>  is created and tries to move all the streams to it. This crashes as follow:
> 
> (gdb) bt
> 0  0xb7ee42f0 in sink_input_may_move_to_cb (i=0x80d9b48, dest=0x80dd3f8)
>    at modules/module-loopback.c:641
> 1  0xb7d10627 in pa_sink_input_may_move_to (i=0x80d9b48, dest=0x80dd3f8)
>    at pulsecore/sink-input.c:1426
> 2  0xb7d11c94 in pa_sink_input_move_to (i=0x80d9b48, dest=0x80dd3f8,
> save=false)
>    at pulsecore/sink-input.c:1730
> 3  0xb7ec4f53 in sink_put_hook_callback (c=0x807be88, sink=0x80dd3f8,
> userdata=0x80abb70)
>    at modules/module-switch-on-connect.c:93
> 4  0xb7ceda5e in pa_hook_fire (hook=0x807c010, data=0x80dd3f8)
>    at pulsecore/hook-list.c:106
> 5  0xb7d16863 in pa_sink_put (s=0x80dd3f8) at pulsecore/sink.c:651
> 6  0xb7d7b26b in start_thread (u=0x80a68b0)
>    at modules/bluetooth/module-bluetooth-device.c:2571
> 7  0xb7d7ba8a in card_set_profile (c=0x80a9530, new_profile=0x80a2b98)
>    at modules/bluetooth/module-bluetooth-device.c:2680
> 
> This logic is already implemented in module-device-manager.
> ---
>  src/modules/module-switch-on-connect.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/modules/module-switch-on-connect.c b/src/modules/module-switch-on-connect.c
> index efe1270..d0bdf9e 100644
> --- a/src/modules/module-switch-on-connect.c
> +++ b/src/modules/module-switch-on-connect.c
> @@ -87,6 +87,10 @@ static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void*
>      }
>  
>      PA_IDXSET_FOREACH(i, def->inputs, idx) {
> +        /* Skip this if it is already in the process of being moved anyway */
> +        if (!i->sink)
> +            continue;
> +
>          if (i->save_sink || !PA_SINK_INPUT_IS_LINKED(i->state))
>              continue;
>  
> @@ -140,6 +144,10 @@ static pa_hook_result_t source_put_hook_callback(pa_core *c, pa_source *source,
>      }
>  
>      PA_IDXSET_FOREACH(o, def->outputs, idx) {
> +        /* Skip this if it is already in the process of being moved anyway */
> +        if (!o->source)
> +            continue;
> +

Can you explain how o->source can be NULL? It can't be just because it's
"already in the process of being moved". The only places that I found
where pa_source_output.source is set to NULL are
pa_source_output_start_move() and pa_source_output_unlink(). Of those,
pa_source_output_start_move() is the interesting one, but prior to
setting o->source to NULL, the source output is removed from the
source's list of outputs, so PA_IDXSET_FOREACH(o, def->outputs, idx)
shouldn't touch any outputs that are being moved.

If the line numbers in the stack trace still match the current source
tree, the crashing line is this:

    if (!u->source_output->source->monitor_of)

I'm suspecting that the problem is something else than the source being
NULL.

-- 
Tanu