link: fix the race condition of port_set_io

This patch is fixing random crashes when vlc player connects rtsp.
we have two places to do link_activate, one is in node_activate(1),
the other one happens on link_paused(2).

The order of (1) and (2) can vary for each different connection.
Sometimes, (1) comes before (2); Sometimes, (1) comes after (2).
This will cause one crash in about five connections from vlc to
rtsp.

If link_paused(2) is earlier than node_activate, it will do set_io
and make activated=true, then when node_activate(1) comes, nothing
will happen:
int pw_link_activate(struct pw_link *this)
{
        struct impl *impl = SPA_CONTAINER_OF(this, struct impl, this);
        int res;

        pw_log_debug(NAME" %p: activate %d %d", this, impl->activated, this->info.state);

        if (impl->activated)
                return 0;
        ...
}
In this case, everything works well.

If node_activate(1) is earlier than (2), it will do port_set_io, but
"activated" will still be false after port_set_io since the link state
can be not PW_LINK_STATE_PAUSED:
int pw_link_activate(struct pw_link *this)
{
        struct impl *impl = SPA_CONTAINER_OF(this, struct impl, this);
        int res;

        pw_log_debug(NAME" %p: activate %d %d", this, impl->activated, this->info.state);

        if (impl->activated)
                return 0;

        pw_link_prepare(this);

        ... // port_set_io

        if (this->info.state == PW_LINK_STATE_PAUSED) {
                pw_loop_invoke(this->output->node->data_loop,
                       do_activate_link, SPA_ID_INVALID, NULL, 0, false, this);
                impl->activated = true;
        }
        return 0;
}
Then when link_paused(2) happens after (1), port_set_io will be done
again.
This will lead to random crash of pipewire.

Signed-off-by: Barry Song

Fixes #186
This commit is contained in:
Barry Song 2019-09-30 09:32:35 +02:00 committed by Wim Taymans
parent 4421cc473b
commit df495b5a8e

View file

@ -55,9 +55,10 @@
struct impl {
struct pw_link this;
bool prepare;
bool activated;
bool passive;
unsigned int prepare:1;
unsigned int io_set:1;
unsigned int activated:1;
unsigned int passive:1;
struct pw_work_queue *work;
@ -756,13 +757,16 @@ int pw_link_activate(struct pw_link *this)
pw_link_prepare(this);
if ((res = port_set_io(this, this->output, SPA_IO_Buffers, this->io,
sizeof(struct spa_io_buffers), &this->rt.out_mix)) < 0)
return res;
if (!impl->io_set) {
if ((res = port_set_io(this, this->output, SPA_IO_Buffers, this->io,
sizeof(struct spa_io_buffers), &this->rt.out_mix)) < 0)
return res;
if ((res = port_set_io(this, this->input, SPA_IO_Buffers, this->io,
sizeof(struct spa_io_buffers), &this->rt.in_mix)) < 0)
return res;
if ((res = port_set_io(this, this->input, SPA_IO_Buffers, this->io,
sizeof(struct spa_io_buffers), &this->rt.in_mix)) < 0)
return res;
impl->io_set = true;
}
if (this->info.state == PW_LINK_STATE_PAUSED) {
pw_loop_invoke(this->output->node->data_loop,
@ -965,6 +969,7 @@ int pw_link_deactivate(struct pw_link *this)
port_set_io(this, this->output, SPA_IO_Buffers, NULL, 0, &this->rt.out_mix);
port_set_io(this, this->input, SPA_IO_Buffers, NULL, 0, &this->rt.in_mix);
impl->io_set = false;
impl->activated = false;
}