stream: autodetect direction

Previously, we required the user to specify the expected read/write
flags for a pipe or tty. But we've already been asking the OS to tell us
what they actually are (fcntl F_GETFL), so we can hopefully just use
that information directly.

Fixes: #1936
PR-URL: #1964
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>

docs/code/tty-gravity/main.c

@@ -32,7 +32,7 @@ void update(uv_timer_t *req) {
 int main() {
     loop = uv_default_loop();
 
-    uv_tty_init(loop, &tty, 1, 0);
+    uv_tty_init(loop, &tty, STDOUT_FILENO, 0);
     uv_tty_set_mode(&tty, 0);
 
     if (uv_tty_get_winsize(&tty, &width, &height)) {

docs/code/tty/main.c

@@ -8,7 +8,7 @@ uv_tty_t tty;
 int main() {
     loop = uv_default_loop();
 
-    uv_tty_init(loop, &tty, 1, 0);
+    uv_tty_init(loop, &tty, STDOUT_FILENO, 0);
     uv_tty_set_mode(&tty, UV_TTY_MODE_NORMAL);
 
     if (uv_guess_handle(1) == UV_TTY) {

docs/src/guide/utilities.rst

@@ -370,9 +370,10 @@ terminal information.
 The first thing to do is to initialize a ``uv_tty_t`` with the file descriptor
 it reads/writes from. This is achieved with::
 
-    int uv_tty_init(uv_loop_t*, uv_tty_t*, uv_file fd, int readable)
+    int uv_tty_init(uv_loop_t*, uv_tty_t*, uv_file fd, int unused)
 
-Set ``readable`` to true if you plan to use ``uv_read_start()`` on the stream.
+The ``unused`` parameter is now auto-detected and ignored. It previously needed
+to be set to use ``uv_read_start()`` on the stream.
 
 It is then best to use ``uv_tty_set_mode`` to set the mode to *normal*
 which enables most TTY formatting, flow-control and other settings. Other_ modes
@@ -423,6 +424,9 @@ can try `ncurses`_.
 
 .. _ncurses: http://www.gnu.org/software/ncurses/ncurses.html
 
+.. versionchanged:: 1.23.1: the `readable` parameter is now unused and ignored.
+                    The appropriate value will now be auto-detected from the kernel.
+
 ----
 
 .. [#] I was first introduced to the term baton in this context, in Konstantin

docs/src/tty.rst

@@ -46,7 +46,7 @@ N/A
 API
 ---
 
-.. c:function:: int uv_tty_init(uv_loop_t* loop, uv_tty_t* handle, uv_file fd, int readable)
+.. c:function:: int uv_tty_init(uv_loop_t* loop, uv_tty_t* handle, uv_file fd, int unused)
 
     Initialize a new TTY stream with the given file descriptor. Usually the
     file descriptor will be:
@@ -55,9 +55,6 @@ API
     * 1 = stdout
     * 2 = stderr
 
-    `readable`, specifies if you plan on calling :c:func:`uv_read_start` with
-    this stream. stdin is readable, stdout is not.
-
     On Unix this function will determine the path of the fd of the terminal
     using :man:`ttyname_r(3)`, open it, and use it if the passed file descriptor
     refers to a TTY. This lets libuv put the tty in non-blocking mode without
@@ -67,8 +64,10 @@ API
     ioctl TIOCGPTN or TIOCPTYGNAME, for instance OpenBSD and Solaris.
 
     .. note::
-        If reopening the TTY fails, libuv falls back to blocking writes for
-        non-readable TTY streams.
+        If reopening the TTY fails, libuv falls back to blocking writes.
+
+    .. versionchanged:: 1.23.1: the `readable` parameter is now unused and ignored.
+                        The correct value will now be auto-detected from the kernel.
 
     .. versionchanged:: 1.9.0: the path of the TTY is determined by
                         :man:`ttyname_r(3)`. In earlier versions libuv opened

src/unix/pipe.c

@@ -132,11 +132,21 @@ void uv__pipe_close(uv_pipe_t* handle) {
 
 
 int uv_pipe_open(uv_pipe_t* handle, uv_file fd) {
+  int flags;
+  int mode;
   int err;
+  flags = 0;
 
   if (uv__fd_exists(handle->loop, fd))
     return UV_EEXIST;
 
+  do
+    mode = fcntl(fd, F_GETFL);
+  while (mode == -1 && errno == EINTR);
+
+  if (mode == -1)
+    return UV__ERR(errno); /* according to docs, must be EBADF */
+
   err = uv__nonblock(fd, 1);
   if (err)
     return err;
@@ -147,9 +157,13 @@ int uv_pipe_open(uv_pipe_t* handle, uv_file fd) {
     return err;
 #endif /* defined(__APPLE__) */
 
-  return uv__stream_open((uv_stream_t*)handle,
-                         fd,
-                         UV_HANDLE_READABLE | UV_HANDLE_WRITABLE);
+  mode &= O_ACCMODE;
+  if (mode != O_WRONLY)
+    flags |= UV_HANDLE_READABLE;
+  if (mode != O_RDONLY)
+    flags |= UV_HANDLE_WRITABLE;
+
+  return uv__stream_open((uv_stream_t*)handle, fd, flags);
 }
 
 

src/unix/stream.c

@@ -1676,6 +1676,7 @@ void uv__stream_close(uv_stream_t* handle) {
   uv__io_close(handle->loop, &handle->io_watcher);
   uv_read_stop(handle);
   uv__handle_stop(handle);
+  handle->flags &= ~(UV_HANDLE_READABLE | UV_HANDLE_WRITABLE);
 
   if (handle->io_watcher.fd != -1) {
     /* Don't close stdio file descriptors.  Nothing good comes from it. */

src/unix/tty.c

@@ -92,13 +92,15 @@ static int uv__tty_is_slave(const int fd) {
   return result;
 }
 
-int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, int fd, int readable) {
+int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, int fd, int unused) {
   uv_handle_type type;
   int flags;
   int newfd;
   int r;
   int saved_flags;
+  int mode;
   char path[256];
+  (void)unused; /* deprecated parameter is no longer needed */
 
   /* File descriptors that refer to files cannot be monitored with epoll.
    * That restriction also applies to character devices like /dev/random
@@ -111,6 +113,15 @@ int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, int fd, int readable) {
   flags = 0;
   newfd = -1;
 
+  /* Save the fd flags in case we need to restore them due to an error. */
+  do
+    saved_flags = fcntl(fd, F_GETFL);
+  while (saved_flags == -1 && errno == EINTR);
+
+  if (saved_flags == -1)
+    return UV__ERR(errno);
+  mode = saved_flags & O_ACCMODE;
+
   /* Reopen the file descriptor when it refers to a tty. This lets us put the
    * tty in non-blocking mode without affecting other processes that share it
    * with us.
@@ -128,13 +139,13 @@ int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, int fd, int readable) {
      * slave device.
      */
     if (uv__tty_is_slave(fd) && ttyname_r(fd, path, sizeof(path)) == 0)
-      r = uv__open_cloexec(path, O_RDWR);
+      r = uv__open_cloexec(path, mode);
     else
       r = -1;
 
     if (r < 0) {
       /* fallback to using blocking writes */
-      if (!readable)
+      if (mode != O_RDONLY)
         flags |= UV_HANDLE_BLOCKING_WRITES;
       goto skip;
     }
@@ -154,22 +165,6 @@ int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, int fd, int readable) {
     fd = newfd;
   }
 
-#if defined(__APPLE__)
-  /* Save the fd flags in case we need to restore them due to an error. */
-  do
-    saved_flags = fcntl(fd, F_GETFL);
-  while (saved_flags == -1 && errno == EINTR);
-
-  if (saved_flags == -1) {
-    if (newfd != -1)
-      uv__close(newfd);
-    return UV__ERR(errno);
-  }
-#endif
-
-  /* Pacify the compiler. */
-  (void) &saved_flags;
-
 skip:
   uv__stream_init(loop, (uv_stream_t*) tty, UV_TTY);
 
@@ -194,9 +189,9 @@ int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, int fd, int readable) {
   }
 #endif
 
-  if (readable)
+  if (mode != O_WRONLY)
     flags |= UV_HANDLE_READABLE;
-  else
+  if (mode != O_RDONLY)
     flags |= UV_HANDLE_WRITABLE;
 
   uv__stream_open((uv_stream_t*) tty, fd, flags);

src/win/tty.c

@@ -172,9 +172,12 @@ void uv_console_init(void) {
 }
 
 
-int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, uv_file fd, int readable) {
+int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, uv_file fd, int unused) {
+  BOOL readable;
+  DWORD NumberOfEvents;
   HANDLE handle;
   CONSOLE_SCREEN_BUFFER_INFO screen_buffer_info;
+  (void)unused;
 
   uv__once_init();
   handle = (HANDLE) uv__get_osfhandle(fd);
@@ -199,6 +202,7 @@ int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, uv_file fd, int readable) {
     fd = -1;
   }
 
+  readable = GetNumberOfConsoleInputEvents(handle, &NumberOfEvents);
   if (!readable) {
     /* Obtain the screen buffer info with the output handle. */
     if (!GetConsoleScreenBufferInfo(handle, &screen_buffer_info)) {

test/test-handle-fileno.c

@@ -27,7 +27,7 @@ static int get_tty_fd(void) {
   /* Make sure we have an FD that refers to a tty */
 #ifdef _WIN32
   HANDLE handle;
-  handle = CreateFileA("conout$",
+  handle = CreateFileA("conin$",
                        GENERIC_READ | GENERIC_WRITE,
                        FILE_SHARE_READ | FILE_SHARE_WRITE,
                        NULL,
@@ -107,11 +107,15 @@ TEST_IMPL(handle_fileno) {
   } else {
     r = uv_tty_init(loop, &tty, tty_fd, 0);
     ASSERT(r == 0);
+    ASSERT(uv_is_readable((uv_stream_t*) &tty));
+    ASSERT(!uv_is_writable((uv_stream_t*) &tty));
     r = uv_fileno((uv_handle_t*) &tty, &fd);
     ASSERT(r == 0);
     uv_close((uv_handle_t*) &tty, NULL);
     r = uv_fileno((uv_handle_t*) &tty, &fd);
     ASSERT(r == UV_EBADF);
+    ASSERT(!uv_is_readable((uv_stream_t*) &tty));
+    ASSERT(!uv_is_writable((uv_stream_t*) &tty));
   }
 
   uv_run(loop, UV_RUN_DEFAULT);

test/test-spawn.c

@@ -1733,6 +1733,7 @@ TEST_IMPL(spawn_inherit_streams) {
   uv_buf_t buf;
   unsigned int i;
   int r;
+  int bidir;
   uv_write_t write_req;
   uv_loop_t* loop;
 
@@ -1751,6 +1752,15 @@ TEST_IMPL(spawn_inherit_streams) {
   ASSERT(uv_pipe_open(&pipe_stdout_child, fds_stdout[1]) == 0);
   ASSERT(uv_pipe_open(&pipe_stdin_parent, fds_stdin[1]) == 0);
   ASSERT(uv_pipe_open(&pipe_stdout_parent, fds_stdout[0]) == 0);
+  ASSERT(uv_is_readable((uv_stream_t*) &pipe_stdin_child));
+  ASSERT(uv_is_writable((uv_stream_t*) &pipe_stdout_child));
+  ASSERT(uv_is_writable((uv_stream_t*) &pipe_stdin_parent));
+  ASSERT(uv_is_readable((uv_stream_t*) &pipe_stdout_parent));
+  /* Some systems (SVR4) open a bidirectional pipe, most don't. */
+  bidir = uv_is_writable((uv_stream_t*) &pipe_stdin_child);
+  ASSERT(uv_is_readable((uv_stream_t*) &pipe_stdout_child) == bidir);
+  ASSERT(uv_is_readable((uv_stream_t*) &pipe_stdin_parent) == bidir);
+  ASSERT(uv_is_writable((uv_stream_t*) &pipe_stdout_parent) == bidir);
 
   child_stdio[0].flags = UV_INHERIT_STREAM;
   child_stdio[0].data.stream = (uv_stream_t *)&pipe_stdin_child;

test/test-tty.c

@@ -96,9 +96,13 @@ TEST_IMPL(tty) {
 
   r = uv_tty_init(uv_default_loop(), &tty_in, ttyin_fd, 1);  /* Readable. */
   ASSERT(r == 0);
+  ASSERT(uv_is_readable((uv_stream_t*) &tty_in));
+  ASSERT(!uv_is_writable((uv_stream_t*) &tty_in));
 
   r = uv_tty_init(uv_default_loop(), &tty_out, ttyout_fd, 0);  /* Writable. */
   ASSERT(r == 0);
+  ASSERT(!uv_is_readable((uv_stream_t*) &tty_out));
+  ASSERT(uv_is_writable((uv_stream_t*) &tty_out));
 
   r = uv_tty_get_winsize(&tty_out, &width, &height);
   ASSERT(r == 0);
@@ -186,6 +190,8 @@ TEST_IMPL(tty_raw) {
 
   r = uv_tty_init(uv_default_loop(), &tty_in, ttyin_fd, 1);  /* Readable. */
   ASSERT(r == 0);
+  ASSERT(uv_is_readable((uv_stream_t*) &tty_in));
+  ASSERT(!uv_is_writable((uv_stream_t*) &tty_in));
 
   r = uv_read_start((uv_stream_t*)&tty_in, tty_raw_alloc, tty_raw_read);
   ASSERT(r == 0);
@@ -242,6 +248,8 @@ TEST_IMPL(tty_empty_write) {
 
   r = uv_tty_init(uv_default_loop(), &tty_out, ttyout_fd, 0);  /* Writable. */
   ASSERT(r == 0);
+  ASSERT(!uv_is_readable((uv_stream_t*) &tty_out));
+  ASSERT(uv_is_writable((uv_stream_t*) &tty_out));
 
   bufs[0].len = 0;
   bufs[0].base = &dummy[0];
@@ -309,6 +317,8 @@ TEST_IMPL(tty_file) {
 #ifndef _WIN32
   uv_loop_t loop;
   uv_tty_t tty;
+  uv_tty_t tty_ro;
+  uv_tty_t tty_wo;
   int fd;
 
   ASSERT(0 == uv_loop_init(&loop));
@@ -334,13 +344,40 @@ TEST_IMPL(tty_file) {
     ASSERT(0 == close(fd));
   }
 
-  fd = open("/dev/tty", O_RDONLY);
+  fd = open("/dev/tty", O_RDWR);
   if (fd != -1) {
     ASSERT(0 == uv_tty_init(&loop, &tty, fd, 1));
-    ASSERT(0 == close(fd));
+    ASSERT(0 == close(fd)); /* TODO: it's indeterminate who owns fd now */
+    ASSERT(uv_is_readable((uv_stream_t*) &tty));
+    ASSERT(uv_is_writable((uv_stream_t*) &tty));
     uv_close((uv_handle_t*) &tty, NULL);
+    ASSERT(!uv_is_readable((uv_stream_t*) &tty));
+    ASSERT(!uv_is_writable((uv_stream_t*) &tty));
+  }
+
+  fd = open("/dev/tty", O_RDONLY);
+  if (fd != -1) {
+    ASSERT(0 == uv_tty_init(&loop, &tty_ro, fd, 1));
+    ASSERT(0 == close(fd)); /* TODO: it's indeterminate who owns fd now */
+    ASSERT(uv_is_readable((uv_stream_t*) &tty_ro));
+    ASSERT(!uv_is_writable((uv_stream_t*) &tty_ro));
+    uv_close((uv_handle_t*) &tty_ro, NULL);
+    ASSERT(!uv_is_readable((uv_stream_t*) &tty_ro));
+    ASSERT(!uv_is_writable((uv_stream_t*) &tty_ro));
   }
 
+  fd = open("/dev/tty", O_WRONLY);
+  if (fd != -1) {
+    ASSERT(0 == uv_tty_init(&loop, &tty_wo, fd, 0));
+    ASSERT(0 == close(fd)); /* TODO: it's indeterminate who owns fd now */
+    ASSERT(!uv_is_readable((uv_stream_t*) &tty_wo));
+    ASSERT(uv_is_writable((uv_stream_t*) &tty_wo));
+    uv_close((uv_handle_t*) &tty_wo, NULL);
+    ASSERT(!uv_is_readable((uv_stream_t*) &tty_wo));
+    ASSERT(!uv_is_writable((uv_stream_t*) &tty_wo));
+  }
+
+
   ASSERT(0 == uv_run(&loop, UV_RUN_DEFAULT));
   ASSERT(0 == uv_loop_close(&loop));
 
@@ -370,6 +407,10 @@ TEST_IMPL(tty_pty) {
 
   ASSERT(0 == uv_tty_init(&loop, &slave_tty, slave_fd, 0));
   ASSERT(0 == uv_tty_init(&loop, &master_tty, master_fd, 0));
+  ASSERT(uv_is_readable((uv_stream_t*) &slave_tty));
+  ASSERT(uv_is_writable((uv_stream_t*) &slave_tty));
+  ASSERT(uv_is_readable((uv_stream_t*) &master_tty));
+  ASSERT(uv_is_writable((uv_stream_t*) &master_tty));
   /* Check if the file descriptor was reopened. If it is,
    * UV_HANDLE_BLOCKING_WRITES (value 0x100000) isn't set on flags.
    */