From df495b5a8e98123d765f8d9c3a6908665c35bf57 Mon Sep 17 00:00:00 2001
From: Barry Song <barrysong6@github.com>
Date: Mon, 30 Sep 2019 09:32:35 +0200
Subject: [PATCH] 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
---
 src/pipewire/link.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/src/pipewire/link.c b/src/pipewire/link.c
index 99f250c30..4709996ed 100644
--- a/src/pipewire/link.c
+++ b/src/pipewire/link.c
@@ -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;
 	}