From 8b0cf41d88ada0b9b8034d7596b318012635065d Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 23 Mar 2023 00:29:02 +0000 Subject: gsocket: Use SOCK_NONBLOCK to avoid a fcntl() syscall where possible If the libc and kernel support `SOCK_NONBLOCK`, we can specify that in the `socket()` flags, and avoid a subsequent call to `fcntl()` to set `O_NONBLOCK`. For modern Linux distributions, this will save a syscall when creating a socket. Signed-off-by: Philip Withnall --- gio/gsocket.c | 89 ++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 51 insertions(+), 38 deletions(-) diff --git a/gio/gsocket.c b/gio/gsocket.c index 2dc005b63..045833915 100644 --- a/gio/gsocket.c +++ b/gio/gsocket.c @@ -587,7 +587,38 @@ g_socket_details_from_fd (GSocket *socket) socket_strerror (errsv)); } -/* Wrapper around socket() that is shared with gnetworkmonitornetlink.c */ +static void +socket_set_nonblock (int fd) +{ +#ifndef G_OS_WIN32 + GError *error = NULL; +#else + gulong arg; +#endif + + /* Always use native nonblocking sockets, as Windows sets sockets to + * nonblocking automatically in certain operations. This way we make + * things work the same on all platforms. + */ +#ifndef G_OS_WIN32 + if (!g_unix_set_fd_nonblocking (fd, TRUE, &error)) + { + g_warning ("Error setting socket nonblocking: %s", error->message); + g_clear_error (&error); + } +#else + arg = TRUE; + + if (ioctlsocket (fd, FIONBIO, &arg) == SOCKET_ERROR) + { + int errsv = get_socket_errno (); + g_warning ("Error setting socket status flags: %s", socket_strerror (errsv)); + } +#endif +} + +/* Wrapper around socket() that is shared with gnetworkmonitornetlink.c. + * It always sets SOCK_CLOEXEC | SOCK_NONBLOCK. */ gint g_socket (gint domain, gint type, @@ -596,13 +627,13 @@ g_socket (gint domain, { int fd, errsv; -#ifdef SOCK_CLOEXEC - fd = socket (domain, type | SOCK_CLOEXEC, protocol); +#if defined(SOCK_CLOEXEC) && defined(SOCK_NONBLOCK) + fd = socket (domain, type | SOCK_CLOEXEC | SOCK_NONBLOCK, protocol); errsv = errno; if (fd != -1) return fd; - /* It's possible that libc has SOCK_CLOEXEC but the kernel does not */ + /* It's possible that libc has SOCK_CLOEXEC and/or SOCK_NONBLOCK but the kernel does not */ if (fd < 0 && (errsv == EINVAL || errsv == EPROTOTYPE)) #endif fd = socket (domain, type, protocol); @@ -644,9 +675,13 @@ g_socket (gint domain, } #endif + /* Ensure the socket is non-blocking. */ + socket_set_nonblock (fd); + return fd; } +/* Returned socket has SOCK_CLOEXEC | SOCK_NONBLOCK set. */ static gint g_socket_create_socket (GSocketFamily family, GSocketType type, @@ -696,44 +731,22 @@ g_socket_constructed (GObject *object) GSocket *socket = G_SOCKET (object); if (socket->priv->fd >= 0) - /* create socket->priv info from the fd */ - g_socket_details_from_fd (socket); - + { + /* create socket->priv info from the fd and ensure it’s non-blocking */ + g_socket_details_from_fd (socket); + socket_set_nonblock (socket->priv->fd); + } else - /* create the fd from socket->priv info */ - socket->priv->fd = g_socket_create_socket (socket->priv->family, - socket->priv->type, - socket->priv->protocol, - &socket->priv->construct_error); + { + /* create the fd from socket->priv info; this sets it non-blocking by construction */ + socket->priv->fd = g_socket_create_socket (socket->priv->family, + socket->priv->type, + socket->priv->protocol, + &socket->priv->construct_error); + } if (socket->priv->fd != -1) { -#ifndef G_OS_WIN32 - GError *error = NULL; -#else - gulong arg; -#endif - - /* Always use native nonblocking sockets, as Windows sets sockets to - * nonblocking automatically in certain operations. This way we make - * things work the same on all platforms. - */ -#ifndef G_OS_WIN32 - if (!g_unix_set_fd_nonblocking (socket->priv->fd, TRUE, &error)) - { - g_warning ("Error setting socket nonblocking: %s", error->message); - g_clear_error (&error); - } -#else - arg = TRUE; - - if (ioctlsocket (socket->priv->fd, FIONBIO, &arg) == SOCKET_ERROR) - { - int errsv = get_socket_errno (); - g_warning ("Error setting socket status flags: %s", socket_strerror (errsv)); - } -#endif - #ifdef SO_NOSIGPIPE /* See note about SIGPIPE below. */ g_socket_set_option (socket, SOL_SOCKET, SO_NOSIGPIPE, TRUE, NULL); -- cgit v1.2.1 From 56c013cb6e8a9d5a6e42fc9846ecc24f289834e1 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 23 Mar 2023 00:52:02 +0000 Subject: inotify: Use IN_NONBLOCK to avoid a fcntl() syscall where possible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `inotify_init1()` API has supported this flag for a long time (possibly since it was first introduced, although I haven’t bothered doing the archaeology). This saves a syscall when first connecting to inotify. Signed-off-by: Philip Withnall --- gio/inotify/inotify-kernel.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/gio/inotify/inotify-kernel.c b/gio/inotify/inotify-kernel.c index 92d61fc31..7733d398e 100644 --- a/gio/inotify/inotify-kernel.c +++ b/gio/inotify/inotify-kernel.c @@ -377,6 +377,7 @@ ik_source_new (gboolean (* callback) (ik_event_t *event)) }; InotifyKernelSource *iks; GSource *source; + gboolean should_set_nonblock = FALSE; source = g_source_new (&source_funcs, sizeof (InotifyKernelSource)); iks = (InotifyKernelSource *) source; @@ -384,17 +385,23 @@ ik_source_new (gboolean (* callback) (ik_event_t *event)) g_source_set_static_name (source, "inotify kernel source"); iks->unmatched_moves = g_hash_table_new (NULL, NULL); - iks->fd = inotify_init1 (IN_CLOEXEC); + iks->fd = inotify_init1 (IN_CLOEXEC | IN_NONBLOCK); if (iks->fd < 0) - iks->fd = inotify_init (); + { + should_set_nonblock = TRUE; + iks->fd = inotify_init (); + } if (iks->fd >= 0) { GError *error = NULL; - g_unix_set_fd_nonblocking (iks->fd, TRUE, &error); - g_assert_no_error (error); + if (should_set_nonblock) + { + g_unix_set_fd_nonblocking (iks->fd, TRUE, &error); + g_assert_no_error (error); + } iks->fd_tag = g_source_add_unix_fd (source, iks->fd, G_IO_IN); } -- cgit v1.2.1 From 5c65437d73d36a92d89493eede707b0e78b9799e Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 23 Mar 2023 00:55:00 +0000 Subject: glib-unix: Add O_NONBLOCK support to g_unix_open_pipe() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add support for it, but don’t use it anywhere yet — this is an API addition, but currently doesn’t cause any functional changes. It’ll be used in the next commit. Signed-off-by: Philip Withnall --- gio/gtestdbus.c | 2 +- glib/gbacktrace.c | 4 ++-- glib/glib-unix.c | 20 +++++++++++------- glib/glib-unixprivate.h | 56 +++++++++++++++++++++++++++++++++++++------------ 4 files changed, 59 insertions(+), 23 deletions(-) diff --git a/gio/gtestdbus.c b/gio/gtestdbus.c index 34cead176..8c5de3355 100644 --- a/gio/gtestdbus.c +++ b/gio/gtestdbus.c @@ -249,7 +249,7 @@ watcher_init (void) gint pipe_fds[2]; /* fork a child to clean up when we are killed */ - if (!g_unix_open_pipe_internal (pipe_fds, TRUE)) + if (!g_unix_open_pipe_internal (pipe_fds, TRUE, FALSE)) { errsv = errno; g_warning ("pipe() failed: %s", g_strerror (errsv)); diff --git a/glib/gbacktrace.c b/glib/gbacktrace.c index b708b1636..0f81502e5 100644 --- a/glib/gbacktrace.c +++ b/glib/gbacktrace.c @@ -398,8 +398,8 @@ stack_trace (const char * const *args) stack_trace_done = FALSE; signal (SIGCHLD, stack_trace_sigchld); - if (!g_unix_open_pipe_internal (in_fd, TRUE) || - !g_unix_open_pipe_internal (out_fd, TRUE)) + if (!g_unix_open_pipe_internal (in_fd, TRUE, FALSE) || + !g_unix_open_pipe_internal (out_fd, TRUE, FALSE)) { perror ("unable to open pipe"); _exit (0); diff --git a/glib/glib-unix.c b/glib/glib-unix.c index 1c9f12599..ef0d1fbfb 100644 --- a/glib/glib-unix.c +++ b/glib/glib-unix.c @@ -74,11 +74,16 @@ g_unix_set_error_from_errno (GError **error, * * Similar to the UNIX pipe() call, but on modern systems like Linux * uses the pipe2() system call, which atomically creates a pipe with - * the configured flags. The only supported flag currently is - * %FD_CLOEXEC. If for example you want to configure %O_NONBLOCK, that - * must still be done separately with fcntl(). + * the configured flags. * - * This function does not take %O_CLOEXEC, it takes %FD_CLOEXEC as if + * As of GLib 2.78, the supported flags are `FD_CLOEXEC` and `O_NONBLOCK`. Prior + * to GLib 2.78, only `FD_CLOEXEC` was supported — if you wanted to configure + * `O_NONBLOCK` then that had to be done separately with `fcntl()`. + * + * It is a programmer error to call this function with unsupported flags, and a + * critical warning will be raised. + * + * This function does not take `O_CLOEXEC`, it takes `FD_CLOEXEC` as if * for fcntl(); these are different on Linux/glibc. * * Returns: %TRUE on success, %FALSE if not (and errno will be set). @@ -90,11 +95,12 @@ g_unix_open_pipe (int *fds, int flags, GError **error) { - /* We only support FD_CLOEXEC */ - g_return_val_if_fail ((flags & (FD_CLOEXEC)) == flags, FALSE); + /* We only support FD_CLOEXEC and O_NONBLOCK */ + g_return_val_if_fail ((flags & (FD_CLOEXEC | O_NONBLOCK)) == flags, FALSE); if (!g_unix_open_pipe_internal (fds, - (flags & FD_CLOEXEC) != 0)) + (flags & FD_CLOEXEC) != 0, + (flags & O_NONBLOCK) != 0)) return g_unix_set_error_from_errno (error, errno); return TRUE; diff --git a/glib/glib-unixprivate.h b/glib/glib-unixprivate.h index d4c64bd1e..fa13fe861 100644 --- a/glib/glib-unixprivate.h +++ b/glib/glib-unixprivate.h @@ -41,15 +41,22 @@ G_BEGIN_DECLS static inline gboolean g_unix_open_pipe_internal (int *fds, - gboolean close_on_exec) + gboolean close_on_exec, + gboolean nonblock) { #ifdef HAVE_PIPE2 do { int ecode; + int flags = 0; + + if (close_on_exec) + flags |= O_CLOEXEC; + if (nonblock) + flags |= O_NONBLOCK; /* Atomic */ - ecode = pipe2 (fds, close_on_exec ? O_CLOEXEC : 0); + ecode = pipe2 (fds, flags); if (ecode == -1 && errno != ENOSYS) return FALSE; else if (ecode == 0) @@ -62,21 +69,44 @@ g_unix_open_pipe_internal (int *fds, if (pipe (fds) == -1) return FALSE; - if (!close_on_exec) - return TRUE; + if (close_on_exec) + { + if (fcntl (fds[0], F_SETFD, FD_CLOEXEC) == -1 || + fcntl (fds[1], F_SETFD, FD_CLOEXEC) == -1) + { + int saved_errno = errno; + + close (fds[0]); + close (fds[1]); + fds[0] = -1; + fds[1] = -1; + + errno = saved_errno; + return FALSE; + } + } - if (fcntl (fds[0], F_SETFD, FD_CLOEXEC) == -1 || - fcntl (fds[1], F_SETFD, FD_CLOEXEC) == -1) + if (nonblock) { - int saved_errno = errno; +#ifdef O_NONBLOCK + int flags = O_NONBLOCK; +#else + int flags = O_NDELAY; +#endif + + if (fcntl (fds[0], F_SETFL, flags) == -1 || + fcntl (fds[1], F_SETFL, flags) == -1) + { + int saved_errno = errno; - close (fds[0]); - close (fds[1]); - fds[0] = -1; - fds[1] = -1; + close (fds[0]); + close (fds[1]); + fds[0] = -1; + fds[1] = -1; - errno = saved_errno; - return FALSE; + errno = saved_errno; + return FALSE; + } } return TRUE; -- cgit v1.2.1 From aab9cea326d51d6720c9d644531ac75cdfa357c6 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 23 Mar 2023 00:55:59 +0000 Subject: gwakeup: Use O_NONBLOCK to avoid a fcntl() syscall where possible Create the wakeup pipe using the `O_NONBLOCK` flag for `pipe2()`, where possible. This saves a couple of `fcntl()` syscalls every time a wakeup pipe is created, which is at least once per `GMainContext`. This uses the `O_NONBLOCK` support added to `g_unix_open_pipe()` in the previous commit. Signed-off-by: Philip Withnall --- glib/gwakeup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glib/gwakeup.c b/glib/gwakeup.c index 24d85b669..a3283a3ff 100644 --- a/glib/gwakeup.c +++ b/glib/gwakeup.c @@ -160,7 +160,7 @@ g_wakeup_new (void) /* for any failure, try a pipe instead */ #endif - if (!g_unix_open_pipe (wakeup->fds, FD_CLOEXEC, &error)) + if (!g_unix_open_pipe (wakeup->fds, FD_CLOEXEC | O_NONBLOCK, &error)) g_error ("Creating pipes for GWakeup: %s", error->message); if (!g_unix_set_fd_nonblocking (wakeup->fds[0], TRUE, &error) || -- cgit v1.2.1 From 0387c1561492e808160b52e5f35f2d29ce7a12a6 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 23 Mar 2023 14:08:41 +0000 Subject: gsocket: Improve wording in a warning message slightly Signed-off-by: Philip Withnall --- gio/gsocket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gio/gsocket.c b/gio/gsocket.c index 045833915..22a8dd7ec 100644 --- a/gio/gsocket.c +++ b/gio/gsocket.c @@ -603,7 +603,7 @@ socket_set_nonblock (int fd) #ifndef G_OS_WIN32 if (!g_unix_set_fd_nonblocking (fd, TRUE, &error)) { - g_warning ("Error setting socket nonblocking: %s", error->message); + g_warning ("Error setting socket to nonblocking mode: %s", error->message); g_clear_error (&error); } #else -- cgit v1.2.1