Merge branch 'fix/unix-stream-pollable-detection' into 'master'

gunix: Fix {Input,Output}Stream pollable detection

Closes #1180

See merge request GNOME/glib!1846

gio/giounix-private.c

@@ -0,0 +1,145 @@
+/*
+ * Copyright © 2021 Ole André Vadla Ravnås
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "config.h"
+
+#include <errno.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#if defined (HAVE_EPOLL_CREATE)
+#include <sys/epoll.h>
+#elif defined (HAVE_KQUEUE)
+#include <sys/event.h>
+#include <sys/time.h>
+#endif
+
+#include "giounix-private.h"
+
+#define G_TEMP_FAILURE_RETRY(expression)      \
+  ({                                          \
+    gssize __result;                          \
+                                              \
+    do                                        \
+      __result = (gssize) (expression);       \
+    while (__result == -1 && errno == EINTR); \
+                                              \
+    __result;                                 \
+  })
+
+static gboolean g_fd_is_regular_file (int fd) G_GNUC_UNUSED;
+
+gboolean
+_g_fd_is_pollable (int fd)
+{
+  /*
+   * Determining whether a file-descriptor (FD) is pollable turns out to be
+   * quite hard.
+   *
+   * We used to detect this by attempting to lseek() and check if it failed with
+   * ESPIPE, and if so we'd consider the FD pollable. But this turned out to not
+   * work on e.g. PTYs and other devices that are pollable.
+   *
+   * Another approach that was considered was to call fstat() and if it failed
+   * we'd assume that the FD is pollable, and if it succeeded we'd consider it
+   * pollable as long as it's not a regular file. This seemed to work alright
+   * except for FDs backed by simple devices, such as /dev/null.
+   *
+   * There are however OS-specific methods that allow us to figure this out with
+   * absolute certainty:
+   */
+
+#if defined (HAVE_EPOLL_CREATE)
+  /*
+   * Linux
+   *
+   * The answer we seek is provided by the kernel's file_can_poll():
+   * https://github.com/torvalds/linux/blob/2ab38c17aac10bf55ab3efde4c4db3893d8691d2/include/linux/poll.h#L81-L84
+   * But we cannot probe that by using poll() as the returned events for
+   * non-pollable FDs are always IN | OUT.
+   *
+   * The best option then seems to be using epoll, as it will refuse to add FDs
+   * where file_can_poll() returns FALSE.
+   */
+
+  int efd;
+  struct epoll_event ev = { 0, };
+  gboolean add_succeeded;
+
+  efd = epoll_create (1);
+  if (efd == -1)
+    g_error ("epoll_create () failed: %s", g_strerror (errno));
+
+  ev.events = EPOLLIN;
+
+  add_succeeded = epoll_ctl (efd, EPOLL_CTL_ADD, fd, &ev) == 0;
+
+  close (efd);
+
+  return add_succeeded;
+#elif defined (HAVE_KQUEUE)
+  /*
+   * Apple OSes and BSDs
+   *
+   * Like on Linux, we cannot use poll() to do the probing, but kqueue does
+   * the trick as it will refuse to add non-pollable FDs. (Except for regular
+   * files, which we need to special-case. Even though kqueue does support them,
+   * poll() does not.)
+   */
+
+  int kfd;
+  struct kevent ev;
+  gboolean add_succeeded;
+
+  if (g_fd_is_regular_file (fd))
+    return FALSE;
+
+  kfd = kqueue ();
+  if (kfd == -1)
+    g_error ("kqueue () failed: %s", g_strerror (errno));
+
+  EV_SET (&ev, fd, EVFILT_READ, EV_ADD, 0, 0, NULL);
+
+  add_succeeded =
+      G_TEMP_FAILURE_RETRY (kevent (kfd, &ev, 1, NULL, 0, NULL)) == 0;
+
+  close (kfd);
+
+  return add_succeeded;
+#else
+  /*
+   * Other UNIXes (AIX, QNX, Solaris, etc.)
+   *
+   * We can rule out regular files, but devices such as /dev/null will be
+   * reported as pollable even though they're not. This is hopefully good
+   * enough for most use-cases, but easy to expand on later if needed.
+   */
+
+  return !g_fd_is_regular_file (fd);
+#endif
+}
+
+static gboolean
+g_fd_is_regular_file (int fd)
+{
+  struct stat st;
+
+  if (G_TEMP_FAILURE_RETRY (fstat (fd, &st)) == -1)
+    return FALSE;
+
+  return S_ISREG (st.st_mode);
+}

gio/giounix-private.h

@@ -0,0 +1,26 @@
+/*
+ * Copyright © 2021 Ole André Vadla Ravnås
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "gio.h"
+
+G_BEGIN_DECLS
+
+gboolean _g_fd_is_pollable (int fd);
+
+G_END_DECLS

gio/gunixinputstream.c

@@ -20,12 +20,9 @@
 
 #include "config.h"
 
-#include <sys/types.h>
-#include <sys/stat.h>
 #include <unistd.h>
 #include <errno.h>
 #include <stdio.h>
-#include <fcntl.h>
 
 #include <glib.h>
 #include <glib/gstdio.h>
@@ -36,6 +33,7 @@
 #include "gasynchelper.h"
 #include "gfiledescriptorbased.h"
 #include "glibintl.h"
+#include "giounix-private.h"
 
 
 /**
@@ -64,7 +62,7 @@ enum {
 struct _GUnixInputStreamPrivate {
   int fd;
   guint close_fd : 1;
-  guint is_pipe_or_socket : 1;
+  guint can_poll : 1;
 };
 
 static void g_unix_input_stream_pollable_iface_init (GPollableInputStreamInterface *iface);
@@ -186,10 +184,7 @@ g_unix_input_stream_set_property (GObject         *object,
     {
     case PROP_FD:
       unix_stream->priv->fd = g_value_get_int (value);
-      if (lseek (unix_stream->priv->fd, 0, SEEK_CUR) == -1 && errno == ESPIPE)
-	unix_stream->priv->is_pipe_or_socket = TRUE;
-      else
-	unix_stream->priv->is_pipe_or_socket = FALSE;
+      unix_stream->priv->can_poll = _g_fd_is_pollable (unix_stream->priv->fd);
       break;
     case PROP_CLOSE_FD:
       unix_stream->priv->close_fd = g_value_get_boolean (value);
@@ -337,7 +332,7 @@ g_unix_input_stream_read (GInputStream  *stream,
 
   poll_fds[0].fd = unix_stream->priv->fd;
   poll_fds[0].events = G_IO_IN;
-  if (unix_stream->priv->is_pipe_or_socket &&
+  if (unix_stream->priv->can_poll &&
       g_cancellable_make_pollfd (cancellable, &poll_fds[1]))
     nfds = 2;
   else
@@ -445,7 +440,7 @@ g_unix_input_stream_skip_finish  (GInputStream  *stream,
 static gboolean
 g_unix_input_stream_pollable_can_poll (GPollableInputStream *stream)
 {
-  return G_UNIX_INPUT_STREAM (stream)->priv->is_pipe_or_socket;
+  return G_UNIX_INPUT_STREAM (stream)->priv->can_poll;
 }
 
 static gboolean

gio/gunixoutputstream.c

@@ -20,12 +20,9 @@
 
 #include "config.h"
 
-#include <sys/types.h>
-#include <sys/stat.h>
 #include <unistd.h>
 #include <errno.h>
 #include <stdio.h>
-#include <fcntl.h>
 #include <sys/uio.h>
 
 #include <glib.h>
@@ -38,6 +35,7 @@
 #include "gfiledescriptorbased.h"
 #include "glibintl.h"
 #include "gioprivate.h"
+#include "giounix-private.h"
 
 
 /**
@@ -66,7 +64,7 @@ enum {
 struct _GUnixOutputStreamPrivate {
   int fd;
   guint close_fd : 1;
-  guint is_pipe_or_socket : 1;
+  guint can_poll : 1;
 };
 
 static void g_unix_output_stream_pollable_iface_init (GPollableOutputStreamInterface *iface);
@@ -186,10 +184,7 @@ g_unix_output_stream_set_property (GObject         *object,
     {
     case PROP_FD:
       unix_stream->priv->fd = g_value_get_int (value);
-      if (lseek (unix_stream->priv->fd, 0, SEEK_CUR) == -1 && errno == ESPIPE)
-	unix_stream->priv->is_pipe_or_socket = TRUE;
-      else
-	unix_stream->priv->is_pipe_or_socket = FALSE;
+      unix_stream->priv->can_poll = _g_fd_is_pollable (unix_stream->priv->fd);
       break;
     case PROP_CLOSE_FD:
       unix_stream->priv->close_fd = g_value_get_boolean (value);
@@ -339,7 +334,7 @@ g_unix_output_stream_write (GOutputStream  *stream,
   poll_fds[0].events = G_IO_OUT;
   nfds++;
 
-  if (unix_stream->priv->is_pipe_or_socket &&
+  if (unix_stream->priv->can_poll &&
       g_cancellable_make_pollfd (cancellable, &poll_fds[1]))
     nfds++;
 
@@ -446,7 +441,7 @@ g_unix_output_stream_writev (GOutputStream        *stream,
   poll_fds[0].events = G_IO_OUT;
   nfds++;
 
-  if (unix_stream->priv->is_pipe_or_socket &&
+  if (unix_stream->priv->can_poll &&
       g_cancellable_make_pollfd (cancellable, &poll_fds[1]))
     nfds++;
 
@@ -532,7 +527,7 @@ g_unix_output_stream_close (GOutputStream  *stream,
 static gboolean
 g_unix_output_stream_pollable_can_poll (GPollableOutputStream *stream)
 {
-  return G_UNIX_OUTPUT_STREAM (stream)->priv->is_pipe_or_socket;
+  return G_UNIX_OUTPUT_STREAM (stream)->priv->can_poll;
 }
 
 static gboolean

gio/meson.build

@@ -361,6 +361,7 @@ gdbus_daemon_sources = [
 if host_system != 'windows'
   unix_sources = files(
     'gfiledescriptorbased.c',
+    'giounix-private.c',
     'gunixconnection.c',
     'gunixcredentialsmessage.c',
     'gunixfdlist.c',

gio/tests/meson.build

@@ -59,7 +59,7 @@ gio_tests = {
   'network-monitor' : {},
   'network-monitor-race' : {},
   'permission' : {},
-  'pollable' : {},
+  'pollable' : {'dependencies' : [libdl_dep]},
   'proxy-test' : {},
   'readwrite' : {},
   'simple-async-result' : {},

gio/tests/pollable.c

@@ -16,10 +16,13 @@
  * Public License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "config.h"
+
 #include <gio/gio.h>
 #include <glib/gstdio.h>
 
 #ifdef G_OS_UNIX
+#include <dlfcn.h>
 #include <fcntl.h>
 #include <gio/gunixinputstream.h>
 #include <gio/gunixoutputstream.h>
@@ -146,10 +149,26 @@ test_streams (void)
 }
 
 #ifdef G_OS_UNIX
+
+#define g_assert_not_pollable(fd) \
+  G_STMT_START {                                                        \
+    in = G_POLLABLE_INPUT_STREAM (g_unix_input_stream_new (fd, FALSE)); \
+    out = g_unix_output_stream_new (fd, FALSE);                         \
+                                                                        \
+    g_assert (!g_pollable_input_stream_can_poll (in));                  \
+    g_assert (!g_pollable_output_stream_can_poll (                      \
+        G_POLLABLE_OUTPUT_STREAM (out)));                               \
+                                                                        \
+    g_clear_object (&in);                                               \
+    g_clear_object (&out);                                              \
+  } G_STMT_END
+
 static void
-test_pollable_unix (void)
+test_pollable_unix_pipe (void)
 {
-  int pipefds[2], status, fd;
+  int pipefds[2], status;
+
+  g_test_summary ("Test that pipes are considered pollable, just like sockets");
 
   status = pipe (pipefds);
   g_assert_cmpint (status, ==, 0);
@@ -161,21 +180,86 @@ test_pollable_unix (void)
 
   g_object_unref (in);
   g_object_unref (out);
+}
 
-  /* Non-pipe/socket unix streams are not pollable */
-  fd = g_open ("/dev/null", O_RDWR, 0);
-  g_assert_cmpint (fd, !=, -1);
-  in = G_POLLABLE_INPUT_STREAM (g_unix_input_stream_new (fd, FALSE));
-  out = g_unix_output_stream_new (fd, FALSE);
+static void
+test_pollable_unix_pty (void)
+{
+  int (*openpty_impl) (int *, int *, char *, void *, void *);
+  int a, b, status;
 
-  g_assert (!g_pollable_input_stream_can_poll (in));
-  g_assert (!g_pollable_output_stream_can_poll (G_POLLABLE_OUTPUT_STREAM (out)));
+  g_test_summary ("Test that PTYs are considered pollable");
+
+#ifdef __linux__
+  dlopen ("libutil.so", RTLD_GLOBAL | RTLD_LAZY);
+#endif
+
+  openpty_impl = dlsym (RTLD_DEFAULT, "openpty");
+  if (openpty_impl == NULL)
+    {
+      g_test_skip ("System does not support openpty()");
+      return;
+    }
+
+  status = openpty_impl (&a, &b, NULL, NULL, NULL);
+  if (status == -1)
+    {
+      g_test_skip ("Unable to open PTY");
+      return;
+    }
+
+  in = G_POLLABLE_INPUT_STREAM (g_unix_input_stream_new (a, TRUE));
+  out = g_unix_output_stream_new (b, TRUE);
+
+  test_streams ();
 
   g_object_unref (in);
   g_object_unref (out);
+
+  close (a);
+  close (b);
+}
+
+static void
+test_pollable_unix_file (void)
+{
+  int fd;
+
+  g_test_summary ("Test that regular files are not considered pollable");
+
+  fd = g_open ("/etc/hosts", O_RDONLY, 0);
+  if (fd == -1)
+    {
+      g_test_skip ("Unable to open /etc/hosts");
+      return;
+    }
+
+  g_assert_not_pollable (fd);
+
   close (fd);
 }
 
+static void
+test_pollable_unix_nulldev (void)
+{
+  int fd;
+
+  g_test_summary ("Test that /dev/null is not considered pollable, but only if "
+                  "on a system where we are able to tell it apart from devices "
+                  "that actually implement poll");
+
+#if defined (HAVE_EPOLL_CREATE) || defined (HAVE_KQUEUE)
+  fd = g_open ("/dev/null", O_RDWR, 0);
+  g_assert_cmpint (fd, !=, -1);
+
+  g_assert_not_pollable (fd);
+
+  close (fd);
+#else
+  g_test_skip ("Cannot detect /dev/null as non-pollable on this system");
+#endif
+}
+
 static void
 test_pollable_converter (void)
 {
@@ -285,7 +369,10 @@ main (int   argc,
   g_test_init (&argc, &argv, NULL);
 
 #ifdef G_OS_UNIX
-  g_test_add_func ("/pollable/unix", test_pollable_unix);
+  g_test_add_func ("/pollable/unix/pipe", test_pollable_unix_pipe);
+  g_test_add_func ("/pollable/unix/pty", test_pollable_unix_pty);
+  g_test_add_func ("/pollable/unix/file", test_pollable_unix_file);
+  g_test_add_func ("/pollable/unix/nulldev", test_pollable_unix_nulldev);
   g_test_add_func ("/pollable/converter", test_pollable_converter);
 #endif
   g_test_add_func ("/pollable/socket", test_pollable_socket);

meson.build

@@ -485,6 +485,7 @@ functions = [
   'close_range',
   'endmntent',
   'endservent',
+  'epoll_create',
   'fallocate',
   'fchmod',
   'fchown',