diff --git a/src/unix/stream.c b/src/unix/stream.c index 725558662..660d520a7 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -1030,8 +1030,6 @@ static void uv__read(uv_stream_t* stream) { int err; int is_ipc; - stream->flags &= ~UV_HANDLE_READ_PARTIAL; - /* Prevent loop starvation when the data comes in as fast as (or faster than) * we can read it. XXX Need to rearm fd if we switch to edge-triggered I/O. */ @@ -1146,11 +1144,15 @@ static void uv__read(uv_stream_t* stream) { #endif stream->read_cb(stream, nread, &buf); - /* Return if we didn't fill the buffer, there is no more data to read. */ - if (nread < buflen) { - stream->flags |= UV_HANDLE_READ_PARTIAL; + /* Save a system call and return if we didn't fill the buffer + * completely, on the assumption the next read() will fail with EOF. + * + * Devices like PTYs sometimes operate in a packet-like mode where + * they don't return all available data in a single read but we'll + * catch it on the next read because of level-triggered I/O. + */ + if (nread < buflen) return; - } } } } @@ -1202,22 +1204,23 @@ void uv__stream_io(uv_loop_t* loop, uv__io_t* w, unsigned int events) { assert(uv__stream_fd(stream) >= 0); - /* Ignore POLLHUP here. Even if it's set, there may still be data to read. */ - if (events & (POLLIN | POLLERR | POLLHUP)) + if (events & (POLLIN | POLLERR)) uv__read(stream); if (uv__stream_fd(stream) == -1) return; /* read_cb closed stream. */ /* Short-circuit iff POLLHUP is set, the user is still interested in read - * events and uv__read() reported a partial read but not EOF. If the EOF - * flag is set, uv__read() called read_cb with err=UV_EOF and we don't - * have to do anything. If the partial read flag is not set, we can't - * report the EOF yet because there is still data to read. + * events and uv__read() didn't see EOF. If the EOF flag is set, uv__read() + * called read_cb with err=UV_EOF and we don't have to do anything. + * + * POLLIN should not be set because, at least on Linux and possibly other + * operating systems, devices like PTYs sometimes produce partial reads even + * when more data is available. */ if ((events & POLLHUP) && + !(events & POLLIN) && (stream->flags & UV_HANDLE_READING) && - (stream->flags & UV_HANDLE_READ_PARTIAL) && !(stream->flags & UV_HANDLE_READ_EOF)) { uv_buf_t buf = { NULL, 0 }; uv__stream_eof(stream, &buf); diff --git a/src/uv-common.h b/src/uv-common.h index b9a8e976e..5452a5493 100644 --- a/src/uv-common.h +++ b/src/uv-common.h @@ -90,7 +90,6 @@ enum { UV_HANDLE_LISTENING = 0x00000040, UV_HANDLE_CONNECTION = 0x00000080, UV_HANDLE_SHUT = 0x00000200, - UV_HANDLE_READ_PARTIAL = 0x00000400, UV_HANDLE_READ_EOF = 0x00000800, /* Used by streams and UDP handles. */ diff --git a/test/test-list.h b/test/test-list.h index f1dd7ef51..651fc578e 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -77,6 +77,7 @@ TEST_DECLARE (tty_escape_sequence_processing) #endif TEST_DECLARE (tty_file) TEST_DECLARE (tty_pty) +TEST_DECLARE (tty_pty_partial) TEST_DECLARE (stdio_over_pipes) TEST_DECLARE (stdio_emulate_iocp) TEST_DECLARE (ip6_pton) @@ -664,6 +665,7 @@ TASK_LIST_START #endif TEST_ENTRY (tty_file) TEST_ENTRY (tty_pty) + TEST_ENTRY (tty_pty_partial) TEST_ENTRY (stdio_over_pipes) TEST_ENTRY (stdio_emulate_iocp) TEST_ENTRY (ip6_pton) diff --git a/test/test-tty.c b/test/test-tty.c index abb76f348..bd2a707ff 100644 --- a/test/test-tty.c +++ b/test/test-tty.c @@ -460,3 +460,76 @@ TEST_IMPL(tty_pty) { #endif return 0; } + +#ifndef _WIN32 +static int tty_pty_partial_read_count; + +static void tty_pty_partial_feeder(void *arg) { + static char buf[1<<13]; + ssize_t n; + ssize_t r; + int fd; + int i; + + fd = *(int *)arg; + memset(buf, 'x', sizeof(buf)); + for (i = 0; i < 8; i++) { + for (n = 0; n < (int) sizeof(buf); n += r) { + do + r = write(fd, &buf[n], sizeof(buf) - n); + while (r == -1 && errno == EINTR); + ASSERT_GT(r, 0); + } + } + ASSERT_OK(close(fd)); +} + +static void tty_pty_partial_alloc_cb(uv_handle_t* handle, + size_t suggested_size, + uv_buf_t *buf) { + static char slab[1<<16]; + *buf = uv_buf_init(slab, sizeof(slab)); +} + +static void tty_pty_partial_read_cb(uv_stream_t* stream, + ssize_t nread, + const uv_buf_t *buf) { + if (nread > 0) + tty_pty_partial_read_count += nread; + else + uv_close((uv_handle_t*) stream, NULL); +} +#endif /* !_WIN32 */ + +TEST_IMPL(tty_pty_partial) { +#ifndef _WIN32 + int master_fd, slave_fd; + uv_tty_t master_tty; + uv_thread_t tid; + uv_loop_t loop; + int i; + + /* This test is not 100% deterministic. If the bug it is testing for is + * present, then it fails about 1 in 3 times, that's why it runs in a loop. + */ + for (i = 0; i < 10; i++) { + if (openpty(&master_fd, &slave_fd, NULL, NULL, NULL)) + RETURN_SKIP("No pty available, skipping."); + + tty_pty_partial_read_count = 0; + ASSERT_OK(uv_loop_init(&loop)); + ASSERT_OK(uv_tty_init(&loop, &master_tty, master_fd, 0)); + ASSERT_OK(uv_read_start((uv_stream_t*) &master_tty, + tty_pty_partial_alloc_cb, + tty_pty_partial_read_cb)); + ASSERT(uv_is_readable((uv_stream_t*) &master_tty)); + ASSERT(uv_is_writable((uv_stream_t*) &master_tty)); + ASSERT_OK(uv_thread_create(&tid, tty_pty_partial_feeder, &slave_fd)); + ASSERT_OK(uv_run(&loop, UV_RUN_DEFAULT)); + ASSERT_OK(uv_thread_join(&tid)); + ASSERT_EQ(tty_pty_partial_read_count, 65536); + MAKE_VALGRIND_HAPPY(&loop); + } +#endif + return 0; +}