tcp: Rework client start error handling.

Ensure errors are cleaned up properly at the right level.
Abort connection attempts if we're cancelled.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/1115>

diff --git a/gst/tcp/gsttcpclientsink.c b/gst/tcp/gsttcpclientsink.c
index 36cef94..18253ce 100644 (file)
--- a/gst/tcp/gsttcpclientsink.c
+++ b/gst/tcp/gsttcpclientsink.c
@@ -301,12 +301,16 @@ gst_tcp_client_sink_start (GstBaseSink * bsink)
 
   cur_addr = addrs;
   while (cur_addr) {
+    /* clean up from possible previous iterations */
+    g_clear_error (&err);
+    g_clear_object (&this->socket);
+
     /* iterate over addresses until one works */
     this->socket =
         tcp_create_socket (GST_ELEMENT (this), &cur_addr, this->port, &saddr,
         &err);
     if (!this->socket)
-      break;
+      goto no_socket;
 
     GST_DEBUG_OBJECT (this, "opened sending client socket");
 
@@ -315,17 +319,17 @@ gst_tcp_client_sink_start (GstBaseSink * bsink)
       break;
 
     /* failed to connect, release and try next address... */
-    g_clear_object (&this->socket);
     g_clear_object (&saddr);
+    if (g_error_matches (err, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+      goto connect_failed;
   }
-  g_list_free_full (addrs, g_object_unref);
-  if (!this->socket)
-    goto no_socket;
 
-  /* we should only have a valid saddr if connect was successful */
-  if (!saddr)
+  /* final connect attempt failed */
+  if (err)
     goto connect_failed;
 
+  GST_DEBUG_OBJECT (this, "connected to %s:%d", this->host, this->port);
+  g_list_free_full (g_steal_pointer (&addrs), g_object_unref);
   g_object_unref (saddr);
 
   GST_OBJECT_FLAG_SET (this, GST_TCP_CLIENT_SINK_OPEN);
@@ -333,13 +337,7 @@ gst_tcp_client_sink_start (GstBaseSink * bsink)
   this->data_written = 0;
 
   return TRUE;
-no_socket:
-  {
-    GST_ELEMENT_ERROR (this, RESOURCE, OPEN_READ, (NULL),
-        ("Failed to create socket: %s", err->message));
-    g_clear_error (&err);
-    return FALSE;
-  }
+
 name_resolve:
   {
     if (g_error_matches (err, G_IO_ERROR, G_IO_ERROR_CANCELLED)) {
@@ -351,8 +349,17 @@ name_resolve:
     g_clear_error (&err);
     return FALSE;
   }
+no_socket:
+  {
+    g_list_free_full (g_steal_pointer (&addrs), g_object_unref);
+    GST_ELEMENT_ERROR (this, RESOURCE, OPEN_READ, (NULL),
+        ("Failed to create socket: %s", err->message));
+    g_clear_error (&err);
+    return FALSE;
+  }
 connect_failed:
   {
+    g_list_free_full (g_steal_pointer (&addrs), g_object_unref);
     if (g_error_matches (err, G_IO_ERROR, G_IO_ERROR_CANCELLED)) {
       GST_DEBUG_OBJECT (this, "Cancelled connecting");
     } else {

diff --git a/gst/tcp/gsttcpclientsrc.c b/gst/tcp/gsttcpclientsrc.c
index 8bbddce..5d65b26 100644 (file)
--- a/gst/tcp/gsttcpclientsrc.c
+++ b/gst/tcp/gsttcpclientsrc.c
@@ -420,51 +420,49 @@ gst_tcp_client_src_start (GstBaseSrc * bsrc)
   if (!addrs)
     goto name_resolve;
 
+  /* create receiving client socket */
+  GST_DEBUG_OBJECT (src, "opening receiving client socket to %s:%d",
+      src->host, src->port);
+
   cur_addr = addrs;
   while (cur_addr) {
+    /* clean up from possible previous iterations */
+    g_clear_error (&err);
+    g_clear_object (&src->socket);
+
     /* iterate over addresses until one works */
     src->socket =
         tcp_create_socket (GST_ELEMENT (src), &cur_addr, src->port, &saddr,
         &err);
     if (!src->socket)
-      break;
-
-    /* create receiving client socket */
-    GST_DEBUG_OBJECT (src, "opening receiving client socket to %s:%d",
-        src->host, src->port);
+      goto no_socket;
 
     g_socket_set_timeout (src->socket, src->timeout);
 
     GST_DEBUG_OBJECT (src, "opened receiving client socket");
-    GST_OBJECT_FLAG_SET (src, GST_TCP_CLIENT_SRC_OPEN);
 
     /* connect to server */
     if (g_socket_connect (src->socket, saddr, src->cancellable, &err))
       break;
 
     /* failed to connect, release and try next address... */
-    g_clear_object (&src->socket);
     g_clear_object (&saddr);
+    if (g_error_matches (err, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+      goto connect_failed;
   }
-  g_list_free_full (addrs, g_object_unref);
-  if (!src->socket)
-    goto no_socket;
 
-  /* we should only have a valid saddr if connect was successful */
-  if (!saddr)
+  /* final connect attempt failed */
+  if (err)
     goto connect_failed;
 
-  g_object_unref (saddr);
+  GST_DEBUG_OBJECT (src, "connected to %s:%d", src->host, src->port);
+  g_list_free_full (g_steal_pointer (&addrs), g_object_unref);
+  g_clear_object (&saddr);
+
+  GST_OBJECT_FLAG_SET (src, GST_TCP_CLIENT_SRC_OPEN);
 
   return TRUE;
 
-no_socket:
-  {
-    GST_ELEMENT_ERROR (src, RESOURCE, OPEN_READ, (NULL),
-        ("Failed to create socket: %s", err->message));
-    g_clear_error (&err);
-    return FALSE;
-  }
 name_resolve:
   {
     if (g_error_matches (err, G_IO_ERROR, G_IO_ERROR_CANCELLED)) {
@@ -476,8 +474,17 @@ name_resolve:
     g_clear_error (&err);
     return FALSE;
   }
+no_socket:
+  {
+    g_list_free_full (g_steal_pointer (&addrs), g_object_unref);
+    GST_ELEMENT_ERROR (src, RESOURCE, OPEN_READ, (NULL),
+        ("Failed to create socket: %s", err->message));
+    g_clear_error (&err);
+    return FALSE;
+  }
 connect_failed:
   {
+    g_list_free_full (g_steal_pointer (&addrs), g_object_unref);
     if (g_error_matches (err, G_IO_ERROR, G_IO_ERROR_CANCELLED)) {
       GST_DEBUG_OBJECT (src, "Cancelled connecting");
     } else {
@@ -486,6 +493,8 @@ connect_failed:
               err->message));
     }
     g_clear_error (&err);
+    /* pretend we opened ok for proper cleanup to happen */
+    GST_OBJECT_FLAG_SET (src, GST_TCP_CLIENT_SRC_OPEN);
     gst_tcp_client_src_stop (GST_BASE_SRC (src));
     return FALSE;
   }

diff --git a/gst/tcp/gsttcpelements.c b/gst/tcp/gsttcpelements.c
index f695ead..a65170f 100644 (file)
--- a/gst/tcp/gsttcpelements.c
+++ b/gst/tcp/gsttcpelements.c
@@ -92,9 +92,10 @@ tcp_create_socket (GstElement * obj, GList ** iter, guint16 port,
       g_free (ip);
     }
 #endif
+    /* clean up from possible previous iterations */
+    g_clear_error (err);
     /* update iter in case we get called again */
     *iter = (*iter)->next;
-    g_clear_error (err);
 
     *saddr = g_inet_socket_address_new (addr, port);
     sock =