From c497370254b03fc1ecd72221e6359996ceb73e0c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
Date: Thu, 6 Feb 2020 18:43:35 +0200
Subject: [PATCH] sctp: Start connection synchronously when starting the
 association

Calling bind() only sets up some data structures and calling connect()
only produces one packet before it returns. That packet is stored in a
queue that is asynchronously forwarded by the encoder's source pad loop,
so not much is happening there either. Especially no waiting is
happening here and no forwarding of data to other elements.

This fixes a race condition during connection setup: the connection
would immediately fail if we pass a packet from the peer to the socket
before bind() and connect() have returned.

This can't happen anymore as bind() and connect() have returned already
before both elements reach the PAUSED state, and in webrtcbin there is
an additional blocking pad probe before the decoder that does not let
any data pass through before that anyway.
---
 ext/sctp/sctpassociation.c | 35 +++++++----------------------------
 ext/sctp/sctpassociation.h |  2 --
 2 files changed, 7 insertions(+), 30 deletions(-)

diff --git a/ext/sctp/sctpassociation.c b/ext/sctp/sctpassociation.c
index 209468f0e0..e6e197a2d0 100644
--- a/ext/sctp/sctpassociation.c
+++ b/ext/sctp/sctpassociation.c
@@ -111,7 +111,6 @@ static struct socket *create_sctp_socket (GstSctpAssociation *
     gst_sctp_association);
 static struct sockaddr_conn get_sctp_socket_address (GstSctpAssociation *
     gst_sctp_association, guint16 port);
-static gpointer connection_thread_func (GstSctpAssociation * self);
 static gboolean client_role_connect (GstSctpAssociation * self);
 static int sctp_packet_out (void *addr, void *buffer, size_t length, guint8 tos,
     guint8 set_df);
@@ -193,7 +192,6 @@ gst_sctp_association_init (GstSctpAssociation * self)
   self->remote_port = DEFAULT_REMOTE_SCTP_PORT;
   self->sctp_ass_sock = NULL;
 
-  self->connection_thread = NULL;
   g_mutex_init (&self->association_mutex);
 
   self->state = GST_SCTP_ASSOCIATION_STATE_NEW;
@@ -219,9 +217,6 @@ gst_sctp_association_finalize (GObject * object)
   }
   G_UNLOCK (associations_lock);
 
-  if (self->connection_thread)
-    g_thread_join (self->connection_thread);
-
   G_OBJECT_CLASS (gst_sctp_association_parent_class)->finalize (object);
 }
 
@@ -354,8 +349,6 @@ gst_sctp_association_get (guint32 association_id)
 gboolean
 gst_sctp_association_start (GstSctpAssociation * self)
 {
-  gchar *thread_name;
-
   if (self->state != GST_SCTP_ASSOCIATION_STATE_READY) {
     GST_WARNING_OBJECT (self,
         "SCTP association is in wrong state and cannot be started");
@@ -365,14 +358,16 @@ gst_sctp_association_start (GstSctpAssociation * self)
   if ((self->sctp_ass_sock = create_sctp_socket (self)) == NULL)
     goto error;
 
+  /* TODO: Support both server and client role */
+  if (!client_role_connect (self)) {
+    gst_sctp_association_change_state (self, GST_SCTP_ASSOCIATION_STATE_ERROR,
+        TRUE);
+    goto error;
+  }
+
   gst_sctp_association_change_state (self,
       GST_SCTP_ASSOCIATION_STATE_CONNECTING, TRUE);
 
-  thread_name = g_strdup_printf ("connection_thread_%u", self->association_id);
-  self->connection_thread = g_thread_new (thread_name,
-      (GThreadFunc) connection_thread_func, self);
-  g_free (thread_name);
-
   return TRUE;
 error:
   gst_sctp_association_change_state (self, GST_SCTP_ASSOCIATION_STATE_ERROR,
@@ -526,11 +521,6 @@ gst_sctp_association_force_close (GstSctpAssociation * self)
     usrsctp_close (s);
   }
 
-  if (self->connection_thread) {
-    g_thread_join (self->connection_thread);
-    self->connection_thread = NULL;
-  }
-
   gst_sctp_association_change_state (self,
       GST_SCTP_ASSOCIATION_STATE_DISCONNECTED, TRUE);
 }
@@ -640,17 +630,6 @@ get_sctp_socket_address (GstSctpAssociation * gst_sctp_association,
   return addr;
 }
 
-static gpointer
-connection_thread_func (GstSctpAssociation * self)
-{
-  /* TODO: Support both server and client role */
-  if (!client_role_connect (self))
-    gst_sctp_association_change_state (self, GST_SCTP_ASSOCIATION_STATE_ERROR,
-        TRUE);
-
-  return NULL;
-}
-
 static gboolean
 client_role_connect (GstSctpAssociation * self)
 {
diff --git a/ext/sctp/sctpassociation.h b/ext/sctp/sctpassociation.h
index 1a54745f1b..8133290440 100644
--- a/ext/sctp/sctpassociation.h
+++ b/ext/sctp/sctpassociation.h
@@ -83,8 +83,6 @@ struct _GstSctpAssociation
 
   GstSctpAssociationState state;
 
-  GThread *connection_thread;
-
   GstSctpAssociationPacketReceivedCb packet_received_cb;
   gpointer packet_received_user_data;
   GDestroyNotify packet_received_destroy_notify;