From 64cc9bbc40faaec9cfc541377c331ce34de9ae17 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 21 Mar 2017 17:31:53 +0100 Subject: [PATCH 1/6] gsocketlistener: Fix IPv4 listen() error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In add_any_inet_port() for the case where we successfully listen()ed on IPv6, but failed to listen on IPv4, we would erroneously unref the IPv6 socket which we just gave away ownership of by adding it to priv->sockets. However, given that we have a successful IPv6 socket, we shouldn’t report it as an error if the IPv4 listen() fails. While this code tries quite hard to return both sockets, returning only one seems to be the best we can do in this situation. This issue was observed in an Android 4.4 ARM emulator. It’s possible the emulator deferred the bind() on the host until the listen() on the client. Based on a patch by Ole André Vadla Ravnås . https://gitlab.gnome.org/GNOME/glib/issues/1250 --- gio/gsocketlistener.c | 167 ++++++++++++++++++++++++++---------------- 1 file changed, 103 insertions(+), 64 deletions(-) diff --git a/gio/gsocketlistener.c b/gio/gsocketlistener.c index 8a296a5b5..4c63f009f 100644 --- a/gio/gsocketlistener.c +++ b/gio/gsocketlistener.c @@ -442,6 +442,8 @@ g_socket_listener_add_inet_port (GSocketListener *listener, gboolean need_ipv4_socket = TRUE; GSocket *socket4 = NULL; GSocket *socket6; + GError *socket4_create_error = NULL; + GError *socket4_listen_error = NULL, *socket6_listen_error = NULL; g_return_val_if_fail (listener != NULL, FALSE); g_return_val_if_fail (port != 0, FALSE); @@ -487,23 +489,20 @@ g_socket_listener_add_inet_port (GSocketListener *listener, g_signal_emit (listener, signals[EVENT], 0, G_SOCKET_LISTENER_LISTENING, socket6); - if (!g_socket_listen (socket6, error)) + if (g_socket_listen (socket6, &socket6_listen_error)) { - g_object_unref (socket6); - return FALSE; + g_signal_emit (listener, signals[EVENT], 0, + G_SOCKET_LISTENER_LISTENED, socket6); + + if (source_object) + g_object_set_qdata_full (G_OBJECT (socket6), source_quark, + g_object_ref (source_object), + g_object_unref); + + /* If this socket already speaks IPv4 then we are done. */ + if (g_socket_speaks_ipv4 (socket6)) + need_ipv4_socket = FALSE; } - - g_signal_emit (listener, signals[EVENT], 0, - G_SOCKET_LISTENER_LISTENED, socket6); - - if (source_object) - g_object_set_qdata_full (G_OBJECT (socket6), source_quark, - g_object_ref (source_object), - g_object_unref); - - /* If this socket already speaks IPv4 then we are done. */ - if (g_socket_speaks_ipv4 (socket6)) - need_ipv4_socket = FALSE; } if (need_ipv4_socket) @@ -519,7 +518,7 @@ g_socket_listener_add_inet_port (GSocketListener *listener, socket4 = g_socket_new (G_SOCKET_FAMILY_IPV4, G_SOCKET_TYPE_STREAM, G_SOCKET_PROTOCOL_DEFAULT, - error); + &socket4_create_error); if (socket4 != NULL) /* IPv4 is supported on this platform, so if we fail now it is @@ -546,6 +545,7 @@ g_socket_listener_add_inet_port (GSocketListener *listener, g_object_unref (socket4); if (socket6 != NULL) g_object_unref (socket6); + g_clear_error (&socket6_listen_error); return FALSE; } @@ -557,22 +557,16 @@ g_socket_listener_add_inet_port (GSocketListener *listener, g_signal_emit (listener, signals[EVENT], 0, G_SOCKET_LISTENER_LISTENING, socket4); - if (!g_socket_listen (socket4, error)) + if (g_socket_listen (socket4, &socket4_listen_error)) { - g_object_unref (socket4); - if (socket6 != NULL) - g_object_unref (socket6); + g_signal_emit (listener, signals[EVENT], 0, + G_SOCKET_LISTENER_LISTENED, socket4); - return FALSE; + if (source_object) + g_object_set_qdata_full (G_OBJECT (socket4), source_quark, + g_object_ref (source_object), + g_object_unref); } - - g_signal_emit (listener, signals[EVENT], 0, - G_SOCKET_LISTENER_LISTENED, socket4); - - if (source_object) - g_object_set_qdata_full (G_OBJECT (socket4), source_quark, - g_object_ref (source_object), - g_object_unref); } else /* Ok. So IPv4 is not supported on this platform. If we @@ -580,13 +574,34 @@ g_socket_listener_add_inet_port (GSocketListener *listener, * otherwise we need to tell the user we failed. */ { - if (socket6 != NULL) - g_clear_error (error); - else - return FALSE; + if (socket6 == NULL || socket6_listen_error != NULL) + { + g_clear_object (&socket6); + g_clear_error (&socket6_listen_error); + g_propagate_error (error, g_steal_pointer (&socket4_create_error)); + return FALSE; + } } } + /* Only error out if both listen() calls failed. */ + if ((socket6 == NULL || socket6_listen_error != NULL) && + (socket4 == NULL || socket4_listen_error != NULL)) + { + GError **set_error = ((socket6_listen_error != NULL) ? + &socket6_listen_error : + &socket4_listen_error); + g_propagate_error (error, g_steal_pointer (set_error)); + + g_clear_error (&socket6_listen_error); + g_clear_error (&socket4_listen_error); + + g_clear_object (&socket6); + g_clear_object (&socket4); + + return FALSE; + } + g_assert (socket6 != NULL || socket4 != NULL); if (socket6 != NULL) @@ -598,6 +613,9 @@ g_socket_listener_add_inet_port (GSocketListener *listener, if (G_SOCKET_LISTENER_GET_CLASS (listener)->changed) G_SOCKET_LISTENER_GET_CLASS (listener)->changed (listener); + g_clear_error (&socket6_listen_error); + g_clear_error (&socket4_listen_error); + return TRUE; } @@ -1061,6 +1079,7 @@ g_socket_listener_add_any_inet_port (GSocketListener *listener, GSocket *socket6 = NULL; GSocket *socket4 = NULL; gint attempts = 37; + GError *socket4_listen_error = NULL, *socket6_listen_error = NULL; /* * multi-step process: @@ -1223,7 +1242,9 @@ g_socket_listener_add_any_inet_port (GSocketListener *listener, sockets_to_close); } - /* now we actually listen() the sockets and add them to the listener */ + /* now we actually listen() the sockets and add them to the listener. If + * either of the listen()s fails, only return the other socket. Fail if both + * failed. */ if (socket6 != NULL) { g_socket_set_listen_backlog (socket6, listener->priv->listen_backlog); @@ -1231,24 +1252,22 @@ g_socket_listener_add_any_inet_port (GSocketListener *listener, g_signal_emit (listener, signals[EVENT], 0, G_SOCKET_LISTENER_LISTENING, socket6); - if (!g_socket_listen (socket6, error)) + if (g_socket_listen (socket6, &socket6_listen_error)) { - g_object_unref (socket6); - if (socket4) - g_object_unref (socket4); + g_signal_emit (listener, signals[EVENT], 0, + G_SOCKET_LISTENER_LISTENED, socket6); - return 0; + if (source_object) + g_object_set_qdata_full (G_OBJECT (socket6), source_quark, + g_object_ref (source_object), + g_object_unref); + + g_ptr_array_add (listener->priv->sockets, socket6); + } + else + { + g_clear_object (&socket6); } - - g_signal_emit (listener, signals[EVENT], 0, - G_SOCKET_LISTENER_LISTENED, socket6); - - if (source_object) - g_object_set_qdata_full (G_OBJECT (socket6), source_quark, - g_object_ref (source_object), - g_object_unref); - - g_ptr_array_add (listener->priv->sockets, socket6); } if (socket4 != NULL) @@ -1258,26 +1277,46 @@ g_socket_listener_add_any_inet_port (GSocketListener *listener, g_signal_emit (listener, signals[EVENT], 0, G_SOCKET_LISTENER_LISTENING, socket4); - if (!g_socket_listen (socket4, error)) + if (g_socket_listen (socket4, &socket4_listen_error)) { - g_object_unref (socket4); - if (socket6) - g_object_unref (socket6); + g_signal_emit (listener, signals[EVENT], 0, + G_SOCKET_LISTENER_LISTENED, socket4); - return 0; + if (source_object) + g_object_set_qdata_full (G_OBJECT (socket4), source_quark, + g_object_ref (source_object), + g_object_unref); + + g_ptr_array_add (listener->priv->sockets, socket4); + } + else + { + g_clear_object (&socket4); } - - g_signal_emit (listener, signals[EVENT], 0, - G_SOCKET_LISTENER_LISTENED, socket4); - - if (source_object) - g_object_set_qdata_full (G_OBJECT (socket4), source_quark, - g_object_ref (source_object), - g_object_unref); - - g_ptr_array_add (listener->priv->sockets, socket4); } + /* Error out if both listen() calls failed (or if there’s no separate IPv4 + * socket and the IPv6 listen() call failed). */ + if (socket6_listen_error != NULL && + (socket4 == NULL || socket4_listen_error != NULL)) + { + GError **set_error = ((socket6_listen_error != NULL) ? + &socket6_listen_error : + &socket4_listen_error); + g_propagate_error (error, g_steal_pointer (set_error)); + + g_clear_error (&socket6_listen_error); + g_clear_error (&socket4_listen_error); + + g_clear_object (&socket6); + g_clear_object (&socket4); + + return 0; + } + + g_clear_error (&socket6_listen_error); + g_clear_error (&socket4_listen_error); + if ((socket4 != NULL || socket6 != NULL) && G_SOCKET_LISTENER_GET_CLASS (listener)->changed) G_SOCKET_LISTENER_GET_CLASS (listener)->changed (listener); From 36189525f7ad2d1eb21727bf84eb81033cef438e Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 11 Feb 2019 00:32:47 +0000 Subject: [PATCH 2/6] gsocketlistener: Clarify documentation of add_any_inet_port() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It wasn’t clear what the fallback behaviour of IPv4 vs IPv6 was, or that the same port was used for both. Signed-off-by: Philip Withnall --- gio/gsocketlistener.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/gio/gsocketlistener.c b/gio/gsocketlistener.c index 4c63f009f..d6a1522cb 100644 --- a/gio/gsocketlistener.c +++ b/gio/gsocketlistener.c @@ -1060,6 +1060,14 @@ g_socket_listener_close (GSocketListener *listener) * This is useful if you need to have a socket for incoming connections * but don't care about the specific port number. * + * If possible, the [class@Gio.SocketListener] will listen on both IPv4 and + * IPv6 (listening on the same port on both). If listening on one of the socket + * families fails, the [class@Gio.SocketListener] will only listen on the other. + * If listening on both fails, an error will be returned. + * + * If you need to distinguish whether listening on IPv4 or IPv6 or both was + * successful, connect to [signal@Gio.SocketListener::event]. + * * @source_object will be passed out in the various calls * to accept to identify this particular source, which is * useful if you're listening on multiple addresses and do From d0b497932741eff3875297bf5c981bceeba6cb06 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 19 Feb 2019 18:08:59 +0000 Subject: [PATCH 3/6] gsocketlistener: Clarify documentation of add_inet_port() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It wasn’t clear what the fallback behaviour of IPv4 vs IPv6 was, or that the same port was used for both. Signed-off-by: Philip Withnall --- gio/gsocketlistener.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/gio/gsocketlistener.c b/gio/gsocketlistener.c index d6a1522cb..e3a2da7a3 100644 --- a/gio/gsocketlistener.c +++ b/gio/gsocketlistener.c @@ -420,6 +420,14 @@ g_socket_listener_add_address (GSocketListener *listener, * creates a TCP/IP socket listening on IPv4 and IPv6 (if * supported) on the specified port on all interfaces. * + * If possible, the [class@Gio.SocketListener] will listen on both IPv4 and + * IPv6 (listening on the same port on both). If listening on one of the socket + * families fails, the [class@Gio.SocketListener] will only listen on the other. + * If listening on both fails, an error will be returned. + * + * If you need to distinguish whether listening on IPv4 or IPv6 or both was + * successful, connect to [signal@Gio.SocketListener::event]. + * * @source_object will be passed out in the various calls * to accept to identify this particular source, which is * useful if you're listening on multiple addresses and do From 05e529f3f88c52bc0483ecda120730c724209c10 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 7 Feb 2025 14:31:38 +0000 Subject: [PATCH 4/6] tests: Add tests for g_socket_listener_add_any_inet_port() algorithm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The algorithm that `g_socket_listener_add_any_inet_port()` and `g_socket_listener_add_inet_port()` use to try to connect to IPv4 and/or IPv6 ports are a bit complex (especially when port allocation has to happen in the former method). So far they’ve not really been unit tested, which is unfortunate, and has left latent bugs. Add some unit tests for both methods, by providing mock `socket()` (and friends) functions to override those from libc, and using those to cause specific syscalls to fail according to the test’s needs. These tests demonstrate the fix for #1250 works, as the tests can be run under memcheck and show no memory leaks. They’ve revealed a follow-up issue, though — `g_socket_listener_add_any_inet_port()` doesn’t try a fallback IPv4-only socket if it tries an IPv6 socket and that socket accepts IPv4 but then fails to `listen()`. I’ve filed issue #3604 for that. Signed-off-by: Philip Withnall Helps: #1250 --- gio/tests/socket-listener.c | 463 ++++++++++++++++++++++++++++++++++++ 1 file changed, 463 insertions(+) diff --git a/gio/tests/socket-listener.c b/gio/tests/socket-listener.c index 82b3e92ff..c879c8b2e 100644 --- a/gio/tests/socket-listener.c +++ b/gio/tests/socket-listener.c @@ -1,6 +1,7 @@ /* GLib testing framework examples and tests * * Copyright 2014 Red Hat, Inc. + * Copyright 2025 GNOME Foundation, Inc. * * SPDX-License-Identifier: LGPL-2.1-or-later * @@ -19,8 +20,100 @@ * . */ +#include "config.h" + #include +#ifdef HAVE_RTLD_NEXT +#include +#include +#include +#include +#include +#include +#endif + +/* Override the socket(), bind(), listen() and getsockopt() functions from libc + * so that we can mock results from them in the tests. The libc implementations + * are used by default (via `dlsym()`) unless a test sets a callback + * deliberately. + * + * These override functions are used simply because the linker will resolve them + * before it finds the symbols in libc. This is effectively like `LD_PRELOAD`, + * except without using an external library for them. + * + * This mechanism is intended to be generic and not to force tests in this file + * to be written in a certain way. Tests are free to override these functions + * with their own implementations, or leave them as default. Different tests may + * need to mock these socket functions differently. + * + * If a test overrides these functions, it *must* do so at the start of the test + * (before starting any threads), and *must* clear them to `NULL` at the end of + * the test. The overrides are not thread-safe and will not be automatically + * cleared at the end of a test. + */ +#ifdef HAVE_RTLD_NEXT +#define MOCK_SOCKET_SUPPORTED +#endif + +#ifdef MOCK_SOCKET_SUPPORTED +static int (*real_socket) (int, int, int); +static int (*real_bind) (int, const struct sockaddr *, socklen_t); +static int (*real_listen) (int, int); +static int (*real_getsockopt) (int, int, int, void *, socklen_t *); +static int (*mock_socket) (int, int, int); +static int (*mock_bind) (int, const struct sockaddr *, socklen_t); +static int (*mock_listen) (int, int); +static int (*mock_getsockopt) (int, int, int, void *, socklen_t *); + +int +socket (int domain, + int type, + int protocol) +{ + if (real_socket == NULL) + real_socket = dlsym (RTLD_NEXT, "socket"); + + return ((mock_socket != NULL) ? mock_socket : real_socket) (domain, type, protocol); +} + +int +bind (int sockfd, + const struct sockaddr *addr, + socklen_t addrlen) +{ + if (real_bind == NULL) + real_bind = dlsym (RTLD_NEXT, "bind"); + + return ((mock_bind != NULL) ? mock_bind : real_bind) (sockfd, addr, addrlen); +} + +int +listen (int sockfd, + int backlog) +{ + if (real_listen == NULL) + real_listen = dlsym (RTLD_NEXT, "listen"); + + return ((mock_listen != NULL) ? mock_listen : real_listen) (sockfd, backlog); +} + +int +getsockopt (int sockfd, + int level, + int optname, + void *optval, + socklen_t *optlen) +{ + if (real_getsockopt == NULL) + real_getsockopt = dlsym (RTLD_NEXT, "getsockopt"); + + return ((mock_getsockopt != NULL) ? mock_getsockopt : real_getsockopt) (sockfd, level, optname, optval, optlen); +} + +#endif /* MOCK_SOCKET_SUPPORTED */ + +/* Test event signals. */ static void event_cb (GSocketListener *listener, GSocketListenerEvent event, @@ -82,6 +175,374 @@ test_event_signal (void) g_object_unref (listener); } +/* Provide a mock implementation of socket(), listen(), bind() and getsockopt() + * which use a simple fixed configuration to either force a call to fail with a + * given errno, or allow it to pass through to the system implementation (which + * is assumed to succeed). Results are differentiated by protocol (IPv4 or IPv6) + * but nothing more complex than that. + * + * This allows the `listen()` fallback code in + * `g_socket_listener_add_any_inet_port()` and + * `g_socket_listener_add_inet_port()` to be tested for different situations + * where IPv4 and/or IPv6 sockets don’t work. It doesn’t allow the port + * allocation retry logic to be tested (as it forces all IPv6 `bind()` calls to + * have the same result), but this means it doesn’t have to do more complex + * state tracking of fully mocked-up sockets. + * + * It also means that the test won’t work on systems which don’t support IPv6, + * or which have a configuration which causes the first test case (which passes + * all syscalls through to the system) to fail. On those systems, the test + * should be skipped rather than the mock made more complex. + */ +#ifdef MOCK_SOCKET_SUPPORTED + +typedef struct { + gboolean ipv6_socket_supports_ipv4; + int ipv4_socket_errno; /* 0 for socket() to succeed on the IPv4 socket (i.e. IPv4 sockets are supported) */ + int ipv6_socket_errno; /* similarly */ + int ipv4_bind_errno; /* 0 for bind() to succeed on the IPv4 socket */ + int ipv6_bind_errno; /* similarly */ + int ipv4_listen_errno; /* 0 for listen() to succeed on the IPv4 socket */ + int ipv6_listen_errno; /* similarly */ +} ListenFailuresConfig; + +static struct { + /* Input: */ + ListenFailuresConfig config; + + /* State (we only support tracking one socket of each type): */ + int ipv4_socket_fd; + int ipv6_socket_fd; +} listen_failures_mock_state; + +static int +listen_failures_socket (int domain, + int type, + int protocol) +{ + int fd; + + /* Error out if told to */ + if (domain == AF_INET && listen_failures_mock_state.config.ipv4_socket_errno != 0) + { + errno = listen_failures_mock_state.config.ipv4_socket_errno; + return -1; + } + else if (domain == AF_INET6 && listen_failures_mock_state.config.ipv6_socket_errno != 0) + { + errno = listen_failures_mock_state.config.ipv6_socket_errno; + return -1; + } + else if (domain != AF_INET && domain != AF_INET6) + { + /* we don’t expect to support other socket types */ + g_assert_not_reached (); + } + + /* Pass through to the system, which we require to succeed because we’re only + * mocking errors and not the full socket stack state */ + fd = real_socket (domain, type, protocol); + g_assert_no_errno (fd); + + /* Track the returned FD for each socket type */ + if (domain == AF_INET) + { + g_assert (listen_failures_mock_state.ipv4_socket_fd == 0); + listen_failures_mock_state.ipv4_socket_fd = fd; + } + else if (domain == AF_INET6) + { + g_assert (listen_failures_mock_state.ipv6_socket_fd == 0); + listen_failures_mock_state.ipv6_socket_fd = fd; + } + + return fd; +} + +static int +listen_failures_bind (int sockfd, + const struct sockaddr *addr, + socklen_t addrlen) +{ + int retval; + + /* Error out if told to */ + if (listen_failures_mock_state.ipv4_socket_fd == sockfd && + listen_failures_mock_state.config.ipv4_bind_errno != 0) + { + errno = listen_failures_mock_state.config.ipv4_bind_errno; + return -1; + } + else if (listen_failures_mock_state.ipv6_socket_fd == sockfd && + listen_failures_mock_state.config.ipv6_bind_errno != 0) + { + errno = listen_failures_mock_state.config.ipv6_bind_errno; + return -1; + } + else if (listen_failures_mock_state.ipv4_socket_fd != sockfd && + listen_failures_mock_state.ipv6_socket_fd != sockfd) + { + g_assert_not_reached (); + } + + /* Pass through to the system, which we require to succeed because we’re only + * mocking errors and not the full socket stack state */ + retval = real_bind (sockfd, addr, addrlen); + g_assert_no_errno (retval); + + return retval; +} + +static int +listen_failures_listen (int sockfd, + int backlog) +{ + int retval; + + /* Error out if told to */ + if (listen_failures_mock_state.ipv4_socket_fd == sockfd && + listen_failures_mock_state.config.ipv4_listen_errno != 0) + { + errno = listen_failures_mock_state.config.ipv4_listen_errno; + return -1; + } + else if (listen_failures_mock_state.ipv6_socket_fd == sockfd && + listen_failures_mock_state.config.ipv6_listen_errno != 0) + { + errno = listen_failures_mock_state.config.ipv6_listen_errno; + return -1; + } + else if (listen_failures_mock_state.ipv4_socket_fd != sockfd && + listen_failures_mock_state.ipv6_socket_fd != sockfd) + { + g_assert_not_reached (); + } + + /* Pass through to the system, which we require to succeed because we’re only + * mocking errors and not the full socket stack state */ + retval = real_listen (sockfd, backlog); + g_assert_no_errno (retval); + + return retval; +} + +static int +listen_failures_getsockopt (int sockfd, + int level, + int optname, + void *optval, + socklen_t *optlen) +{ + /* Mock whether IPv6 sockets claim to support IPv4 */ +#if defined (IPPROTO_IPV6) && defined (IPV6_V6ONLY) + if (listen_failures_mock_state.ipv6_socket_fd == sockfd && + level == IPPROTO_IPV6 && optname == IPV6_V6ONLY) + { + int *v6_only = optval; + *v6_only = !listen_failures_mock_state.config.ipv6_socket_supports_ipv4; + return 0; + } +#endif + + /* Don’t assert that the system getsockopt() succeeded, as it could be used + * in complex ways, and it’s incidental to what we’re actually trying to test. */ + return real_getsockopt (sockfd, level, optname, optval, optlen); +} + +#endif /* MOCK_SOCKET_SUPPORTED */ + +static void +test_add_any_inet_port_listen_failures (void) +{ +#ifdef MOCK_SOCKET_SUPPORTED + const struct + { + ListenFailuresConfig config; + GQuark expected_error_domain; /* 0 if success is expected */ + int expected_error_code; /* 0 if success is expected */ + } + test_matrix[] = + { + /* If everything works, it should all work: */ + { { TRUE, 0, 0, 0, 0, 0, 0 }, 0, 0 }, + /* If IPv4 sockets are not supported, IPv6 should be used: */ + { { TRUE, EAFNOSUPPORT, 0, 0, 0, 0, 0 }, 0, 0 }, + /* If IPv6 sockets are not supported, IPv4 should be used: */ + { { TRUE, 0, EAFNOSUPPORT, 0, 0, 0, 0, }, 0, 0 }, + /* If no sockets are supported, everything should fail: */ + { { TRUE, EAFNOSUPPORT, EAFNOSUPPORT, 0, 0, 0, 0 }, + G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED }, + /* If binding IPv4 fails, IPv6 should be used: */ + { { TRUE, 0, 0, EADDRINUSE, 0, 0, 0 }, 0, 0 }, + /* If binding IPv6 fails, fail overall (the algorithm is not symmetric): */ + { { TRUE, 0, 0, 0, EADDRINUSE, 0, 0 }, + G_IO_ERROR, G_IO_ERROR_ADDRESS_IN_USE }, + /* If binding them both fails, fail overall: */ + { { TRUE, 0, 0, EADDRINUSE, EADDRINUSE, 0, 0 }, + G_IO_ERROR, G_IO_ERROR_ADDRESS_IN_USE }, + /* If listening on IPv4 fails, IPv6 should be used: */ + { { TRUE, 0, 0, 0, 0, EADDRINUSE, 0 }, 0, 0 }, + /* If listening on IPv6 fails, IPv4 should be used: + * FIXME: If the IPv6 socket claims to support IPv4, this currently won’t + * retry with an IPv4-only socket; see https://gitlab.gnome.org/GNOME/glib/-/issues/3604 */ + { { TRUE, 0, 0, 0, 0, 0, EADDRINUSE }, + G_IO_ERROR, G_IO_ERROR_ADDRESS_IN_USE }, + /* If listening on IPv6 fails (and the IPv6 socket doesn’t claim to + * support IPv4), IPv4 should be used: */ + { { FALSE, 0, 0, 0, 0, 0, EADDRINUSE }, 0, 0 }, + /* If listening on both fails, fail overall: */ + { { TRUE, 0, 0, 0, 0, EADDRINUSE, EADDRINUSE }, + G_IO_ERROR, G_IO_ERROR_ADDRESS_IN_USE }, + }; + + /* Override the socket(), bind(), listen() and getsockopt() functions */ + mock_socket = listen_failures_socket; + mock_bind = listen_failures_bind; + mock_listen = listen_failures_listen; + mock_getsockopt = listen_failures_getsockopt; + + g_test_summary ("Test that adding a listening port succeeds if either " + "listening on IPv4 or IPv6 succeeds"); + + for (size_t i = 0; i < G_N_ELEMENTS (test_matrix); i++) + { + GSocketService *service = NULL; + GError *local_error = NULL; + uint16_t port; + + g_test_message ("Test %" G_GSIZE_FORMAT, i); + + /* Configure the mock socket behaviour. */ + memset (&listen_failures_mock_state, 0, sizeof (listen_failures_mock_state)); + listen_failures_mock_state.config = test_matrix[i].config; + + /* Create a GSocketService to test. */ + service = g_socket_service_new (); + port = g_socket_listener_add_any_inet_port (G_SOCKET_LISTENER (service), NULL, &local_error); + + if (test_matrix[i].expected_error_domain == 0) + { + g_assert_no_error (local_error); + g_assert_cmpuint (port, !=, 0); + } + else + { + g_assert_error (local_error, test_matrix[i].expected_error_domain, + test_matrix[i].expected_error_code); + g_assert_cmpuint (port, ==, 0); + } + + g_clear_error (&local_error); + g_socket_listener_close (G_SOCKET_LISTENER (service)); + g_clear_object (&service); + } + + /* Tidy up. */ + mock_socket = NULL; + mock_bind = NULL; + mock_listen = NULL; + mock_getsockopt = NULL; + memset (&listen_failures_mock_state, 0, sizeof (listen_failures_mock_state)); +#else /* if !MOCK_SOCKET_SUPPORTED */ + g_test_skip ("Mock socket not supported"); +#endif +} + +static void +test_add_inet_port_listen_failures (void) +{ +#ifdef MOCK_SOCKET_SUPPORTED + const struct + { + ListenFailuresConfig config; + GQuark expected_error_domain; /* 0 if success is expected */ + int expected_error_code; /* 0 if success is expected */ + } + test_matrix[] = + { + /* If everything works, it should all work: */ + { { TRUE, 0, 0, 0, 0, 0, 0 }, 0, 0 }, + /* If IPv4 sockets are not supported, IPv6 should be used: */ + { { TRUE, EAFNOSUPPORT, 0, 0, 0, 0, 0 }, 0, 0 }, + /* If IPv6 sockets are not supported, IPv4 should be used: */ + { { TRUE, 0, EAFNOSUPPORT, 0, 0, 0, 0, }, 0, 0 }, + /* If no sockets are supported, everything should fail: */ + { { TRUE, EAFNOSUPPORT, EAFNOSUPPORT, 0, 0, 0, 0 }, + G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED }, + /* If binding IPv4 fails, IPv6 should be used: */ + { { TRUE, 0, 0, EADDRINUSE, 0, 0, 0 }, 0, 0 }, + /* If binding IPv6 fails, fail overall (the algorithm is not symmetric): */ + { { TRUE, 0, 0, 0, EADDRINUSE, 0, 0 }, + G_IO_ERROR, G_IO_ERROR_ADDRESS_IN_USE }, + /* If binding them both fails, fail overall: */ + { { TRUE, 0, 0, EADDRINUSE, EADDRINUSE, 0, 0 }, + G_IO_ERROR, G_IO_ERROR_ADDRESS_IN_USE }, + /* If listening on IPv4 fails, IPv6 should be used: */ + { { TRUE, 0, 0, 0, 0, EADDRINUSE, 0 }, 0, 0 }, + /* If listening on IPv6 fails, IPv4 should be used: */ + { { TRUE, 0, 0, 0, 0, 0, EADDRINUSE }, 0, 0 }, + /* If listening on IPv6 fails (and the IPv6 socket doesn’t claim to + * support IPv4), IPv4 should be used: */ + { { FALSE, 0, 0, 0, 0, 0, EADDRINUSE }, 0, 0 }, + /* If listening on both fails, fail overall: */ + { { TRUE, 0, 0, 0, 0, EADDRINUSE, EADDRINUSE }, + G_IO_ERROR, G_IO_ERROR_ADDRESS_IN_USE }, + }; + + /* Override the socket(), bind(), listen() and getsockopt() functions */ + mock_socket = listen_failures_socket; + mock_bind = listen_failures_bind; + mock_listen = listen_failures_listen; + mock_getsockopt = listen_failures_getsockopt; + + g_test_summary ("Test that adding a listening port succeeds if either " + "listening on IPv4 or IPv6 succeeds"); + + for (size_t i = 0; i < G_N_ELEMENTS (test_matrix); i++) + { + GSocketService *service = NULL; + GError *local_error = NULL; + gboolean retval; + + g_test_message ("Test %" G_GSIZE_FORMAT, i); + + /* Configure the mock socket behaviour. */ + memset (&listen_failures_mock_state, 0, sizeof (listen_failures_mock_state)); + listen_failures_mock_state.config = test_matrix[i].config; + + /* Create a GSocketService to test. */ + service = g_socket_service_new (); + retval = g_socket_listener_add_inet_port (G_SOCKET_LISTENER (service), 4321, NULL, &local_error); + + if (test_matrix[i].expected_error_domain == 0) + { + g_assert_no_error (local_error); + g_assert_true (retval); + } + else + { + g_assert_error (local_error, test_matrix[i].expected_error_domain, + test_matrix[i].expected_error_code); + g_assert_false (retval); + } + + g_clear_error (&local_error); + + g_socket_listener_close (G_SOCKET_LISTENER (service)); + g_clear_object (&service); + } + + /* Tidy up. */ + mock_socket = NULL; + mock_bind = NULL; + mock_listen = NULL; + mock_getsockopt = NULL; + memset (&listen_failures_mock_state, 0, sizeof (listen_failures_mock_state)); +#else /* if !MOCK_SOCKET_SUPPORTED */ + g_test_skip ("Mock socket not supported"); +#endif +} + int main (int argc, char *argv[]) @@ -89,6 +550,8 @@ main (int argc, g_test_init (&argc, &argv, NULL); g_test_add_func ("/socket-listener/event-signal", test_event_signal); + g_test_add_func ("/socket-listener/add-any-inet-port/listen-failures", test_add_any_inet_port_listen_failures); + g_test_add_func ("/socket-listener/add-inet-port/listen-failures", test_add_inet_port_listen_failures); return g_test_run(); } From 007c525d61929368b82788f2c84148c67a2d66b1 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 7 Feb 2025 14:45:27 +0000 Subject: [PATCH 5/6] gsocketlistener: Minor cleanups to use g_clear_object() and friends This should introduce no functional changes. Signed-off-by: Philip Withnall --- gio/gsocketlistener.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/gio/gsocketlistener.c b/gio/gsocketlistener.c index e3a2da7a3..974720008 100644 --- a/gio/gsocketlistener.c +++ b/gio/gsocketlistener.c @@ -549,10 +549,9 @@ g_socket_listener_add_inet_port (GSocketListener *listener, if (!g_socket_bind (socket4, address, TRUE, error)) { - g_object_unref (address); - g_object_unref (socket4); - if (socket6 != NULL) - g_object_unref (socket6); + g_clear_object (&address); + g_clear_object (&socket4); + g_clear_object (&socket6); g_clear_error (&socket6_listen_error); return FALSE; @@ -1137,8 +1136,7 @@ g_socket_listener_add_any_inet_port (GSocketListener *listener, if (!result || !(address = g_socket_get_local_address (socket6, error))) { - g_object_unref (socket6); - socket6 = NULL; + g_clear_object (&socket6); break; } @@ -1210,14 +1208,12 @@ g_socket_listener_add_any_inet_port (GSocketListener *listener, else /* we failed to bind to the specified port. try again. */ { - g_object_unref (socket4); - socket4 = NULL; + g_clear_object (&socket4); /* keep this open so we get a different port number */ sockets_to_close = g_slist_prepend (sockets_to_close, - socket6); + g_steal_pointer (&socket6)); candidate_port = 0; - socket6 = NULL; } } else @@ -1231,8 +1227,7 @@ g_socket_listener_add_any_inet_port (GSocketListener *listener, if (!result || !(address = g_socket_get_local_address (socket4, error))) { - g_object_unref (socket4); - socket4 = NULL; + g_clear_object (&socket4); break; } @@ -1251,12 +1246,7 @@ g_socket_listener_add_any_inet_port (GSocketListener *listener, /* should only be non-zero if we have a socket */ g_assert ((candidate_port != 0) == (socket4 || socket6)); - while (sockets_to_close) - { - g_object_unref (sockets_to_close->data); - sockets_to_close = g_slist_delete_link (sockets_to_close, - sockets_to_close); - } + g_slist_free_full (g_steal_pointer (&sockets_to_close), g_object_unref); /* now we actually listen() the sockets and add them to the listener. If * either of the listen()s fails, only return the other socket. Fail if both From 9fe6d8b5f07572e2015a33187cd54bc975019398 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 13 Mar 2025 13:15:15 +0000 Subject: [PATCH 6/6] tests: Disable socket-listener mock tests on macOS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Its symbol interposition works differently to that of Linux, so our approach using `dlsym(RTLD_NEXT)` to inject syscalls (and still allow chaining up to the version from libc) doesn’t work on macOS. See https://gitlab.gnome.org/GNOME/glib/-/jobs/4861349 for an example failure. It would be lovely to have these tests working on macOS, but I am not a macOS developer, and have spent enough time fixing this leak (#1250) already. It can wait for follow-up work. Signed-off-by: Philip Withnall Helps: #1250 --- gio/tests/socket-listener.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/gio/tests/socket-listener.c b/gio/tests/socket-listener.c index c879c8b2e..ee2d8fc63 100644 --- a/gio/tests/socket-listener.c +++ b/gio/tests/socket-listener.c @@ -52,7 +52,13 @@ * the test. The overrides are not thread-safe and will not be automatically * cleared at the end of a test. */ -#ifdef HAVE_RTLD_NEXT +/* FIXME: Not currently supported on macOS as its symbol lookup works + * differently to Linux. It will likely need something like DYLD_INTERPOSE() + * from getpwuid-preload.c here to work. At that point, this common code for + * mocking arbitrary syscalls using dlsym(RTLD_NEXT) should probably be factored + * out into a set of internal helpers, because various tests do it for various + * syscalls. */ +#if defined(HAVE_RTLD_NEXT) && !defined(__APPLE__) #define MOCK_SOCKET_SUPPORTED #endif