Skip to content

Commit

Permalink
stream: autodetect direction
Browse files Browse the repository at this point in the history
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.

Fix libuv#1936
  • Loading branch information
vtjnash committed Sep 5, 2018
1 parent d1af5a3 commit ef84cbb
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 38 deletions.
2 changes: 1 addition & 1 deletion docs/code/tty-gravity/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
2 changes: 1 addition & 1 deletion docs/code/tty/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions docs/src/guide/utilities.rst
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,9 @@ 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.

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
Expand Down
11 changes: 5 additions & 6 deletions docs/src/tty.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -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
Expand Down
20 changes: 17 additions & 3 deletions src/unix/pipe.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}


Expand Down
1 change: 1 addition & 0 deletions src/unix/stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
37 changes: 16 additions & 21 deletions src/unix/tty.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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;
}
Expand All @@ -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);

Expand All @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion src/win/tty.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)) {
Expand Down
6 changes: 5 additions & 1 deletion test/test-handle-fileno.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions test/test-spawn.c
Original file line number Diff line number Diff line change
Expand Up @@ -1751,6 +1751,14 @@ 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_stdin_child));
ASSERT(!uv_is_readable((uv_stream_t*) &pipe_stdout_child));
ASSERT(uv_is_writable((uv_stream_t*) &pipe_stdout_child));
ASSERT(!uv_is_readable((uv_stream_t*) &pipe_stdin_parent));
ASSERT(uv_is_writable((uv_stream_t*) &pipe_stdin_parent));
ASSERT(uv_is_readable((uv_stream_t*) &pipe_stdout_parent));
ASSERT(!uv_is_writable((uv_stream_t*) &pipe_stdout_parent));

child_stdio[0].flags = UV_INHERIT_STREAM;
child_stdio[0].data.stream = (uv_stream_t *)&pipe_stdin_child;
Expand Down
45 changes: 43 additions & 2 deletions test/test-tty.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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));
Expand All @@ -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));

Expand Down Expand Up @@ -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.
*/
Expand Down

0 comments on commit ef84cbb

Please sign in to comment.