From 1552184238a49e748a04d0cb2cdf2d34c2d3c7cb Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Thu, 14 Aug 2014 16:38:56 -0700 Subject: [PATCH] unix: read on stream even when UV__POLLHUP set. This fixes a SmartOS specific issue that happens when reading from a stream that is the reading end of a pipe that has been closed by the parent process. In this case, a UV__POLLHUP event would be set on the stream and would prevent the event loop from closing it. As a result, the event loop would think there are stil handles open, and leave the process hanging. Fixes #1419. --- Makefile.am | 1 + src/unix/stream.c | 2 +- test/test-list.h | 6 ++ test/test-pipe-close-stdout-read-stdin.c | 103 +++++++++++++++++++++++ uv.gyp | 1 + 5 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 test/test-pipe-close-stdout-read-stdin.c diff --git a/Makefile.am b/Makefile.am index 420c526a..f72cb9ea 100644 --- a/Makefile.am +++ b/Makefile.am @@ -163,6 +163,7 @@ test_run_tests_SOURCES = test/blackhole-server.c \ test/test-pipe-getsockname.c \ test/test-pipe-sendmsg.c \ test/test-pipe-server-close.c \ + test/test-pipe-close-stdout-read-stdin.c \ test/test-platform-output.c \ test/test-poll-close.c \ test/test-poll-closesocket.c \ diff --git a/src/unix/stream.c b/src/unix/stream.c index 50bd85aa..cd85d2fc 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -1162,7 +1162,7 @@ static 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 it it's set, there may still be data to read. */ - if (events & (UV__POLLIN | UV__POLLERR)) + if (events & (UV__POLLIN | UV__POLLERR | UV__POLLHUP)) uv__read(stream); if (uv__stream_fd(stream) == -1) diff --git a/test/test-list.h b/test/test-list.h index de8d340c..a5aa2103 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -156,6 +156,9 @@ TEST_DECLARE (pipe_ref) TEST_DECLARE (pipe_ref2) TEST_DECLARE (pipe_ref3) TEST_DECLARE (pipe_ref4) +#ifndef _WIN32 +TEST_DECLARE (pipe_close_stdout_read_stdin) +#endif TEST_DECLARE (process_ref) TEST_DECLARE (has_ref) TEST_DECLARE (active) @@ -314,6 +317,9 @@ TASK_LIST_START TEST_ENTRY (pipe_connect_to_file) TEST_ENTRY (pipe_server_close) +#ifndef _WIN32 + TEST_ENTRY (pipe_close_stdout_read_stdin) +#endif TEST_ENTRY (tty) TEST_ENTRY (stdio_over_pipes) TEST_ENTRY (ip6_pton) diff --git a/test/test-pipe-close-stdout-read-stdin.c b/test/test-pipe-close-stdout-read-stdin.c new file mode 100644 index 00000000..26a1ee76 --- /dev/null +++ b/test/test-pipe-close-stdout-read-stdin.c @@ -0,0 +1,103 @@ +/* Copyright Joyent, Inc. and other Node contributors. All rights reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#ifndef _WIN32 + +#include +#include +#include +#include + +#include "uv.h" +#include "task.h" + +void alloc_buffer(uv_handle_t *handle, size_t suggested_size, uv_buf_t* buf) +{ + static char buffer[1024]; + + buf->base = buffer; + buf->len = sizeof(buffer); +} + +void read_stdin(uv_stream_t *stream, ssize_t nread, const uv_buf_t* buf) +{ + if (nread < 0) { + uv_close((uv_handle_t*)stream, NULL); + return; + } +} + +/* + * This test is a reproduction of joyent/libuv#1419 . + */ +TEST_IMPL(pipe_close_stdout_read_stdin) { + int r = -1; + int pid; + int fd[2]; + int status; + + pipe(fd); + + if ((pid = fork()) == 0) { + /* + * Make the read side of the pipe our stdin. + * The write side will be closed by the parent process. + */ + close(fd[1]); + close(0); + dup(fd[0]); + + /* Create a stream that reads from the pipe. */ + uv_pipe_t stdin_pipe; + + r = uv_pipe_init(uv_default_loop(), (uv_pipe_t *)&stdin_pipe, 0); + ASSERT(r == 0); + + r = uv_pipe_open((uv_pipe_t *)&stdin_pipe, 0); + ASSERT(r == 0); + + r = uv_read_start((uv_stream_t *)&stdin_pipe, alloc_buffer, read_stdin); + ASSERT(r == 0); + + /* + * Because the other end of the pipe was closed, there should + * be no event left to process after one run of the event loop. + * Otherwise, it means that events were not processed correctly. + */ + ASSERT(uv_run(uv_default_loop(), UV_RUN_NOWAIT) == 0); + } else { + /* + * Close both ends of the pipe so that the child + * get a POLLHUP event when it tries to read from + * the other end. + */ + close(fd[1]); + close(fd[0]); + + waitpid(pid, &status, 0); + ASSERT(WIFEXITED(status) && WEXITSTATUS(status) == 0); + } + + MAKE_VALGRIND_HAPPY(); + return 0; +} + +#endif /* ifndef _WIN32 */ diff --git a/uv.gyp b/uv.gyp index f32f8b42..b9edcafa 100644 --- a/uv.gyp +++ b/uv.gyp @@ -346,6 +346,7 @@ 'test/test-pipe-getsockname.c', 'test/test-pipe-sendmsg.c', 'test/test-pipe-server-close.c', + 'test/test-pipe-close-stdout-read-stdin.c', 'test/test-platform-output.c', 'test/test-poll.c', 'test/test-poll-close.c',