Skip to content

Commit

Permalink
tcp: Rework client start error handling.
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
dougnazar authored and GStreamer Marge Bot committed Apr 22, 2021
1 parent 538e2ef commit 3676a2c
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 35 deletions.
35 changes: 21 additions & 14 deletions gst/tcp/gsttcpclientsink.c
Expand Up @@ -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");

Expand All @@ -315,31 +319,25 @@ 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);

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)) {
Expand All @@ -351,8 +349,17 @@ gst_tcp_client_sink_start (GstBaseSink * bsink)
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 {
Expand Down
49 changes: 29 additions & 20 deletions gst/tcp/gsttcpclientsrc.c
Expand Up @@ -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)) {
Expand All @@ -476,8 +474,17 @@ gst_tcp_client_src_start (GstBaseSrc * bsrc)
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 {
Expand All @@ -486,6 +493,8 @@ gst_tcp_client_src_start (GstBaseSrc * bsrc)
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;
}
Expand Down
3 changes: 2 additions & 1 deletion gst/tcp/gsttcpelements.c
Expand Up @@ -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 =
Expand Down

0 comments on commit 3676a2c

Please sign in to comment.