From f6422af80ab54eddf5f1344bec7ddab9ab240924 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 21 Dec 2013 20:12:35 -0800 Subject: [PATCH 1/6] osx: Fix a possible segfault in uv__io_poll In our build infrastructure, I've seen a lot of segfaults recently that were all only happening on OSX. Upon inspecting the coredumps, it appearded that all segfaults happened at the same instruction, and upon translating the assembly back to the source, I found that an array could be indexed with a -1 index before the index was checked to be not -1. As concrete evidence, here is the situation that I found caused the segfault. The instruction in question along with the relevant register values was: mov (%r8,%r15,8),%r12 r8 = 0x7fb0ba800000 r15 = 0xffffffffffffffff r8 + r15 * 8 == 0x7fb0ba7ffff8 It appears that the base of loop->watchers was page aligned, and by going back one word I guess that the page wasn't mapped, causing our segfaults. --- src/unix/kqueue.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/unix/kqueue.c b/src/unix/kqueue.c index a80b7d14..dc642034 100644 --- a/src/unix/kqueue.c +++ b/src/unix/kqueue.c @@ -168,11 +168,10 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { for (i = 0; i < nfds; i++) { ev = events + i; fd = ev->ident; - w = loop->watchers[fd]; - /* Skip invalidated events, see uv__platform_invalidate_fd */ if (fd == -1) continue; + w = loop->watchers[fd]; if (w == NULL) { /* File descriptor that we've stopped watching, disarm it. */ From f526c90eeff271d9323a9107b9a64a4671fd3103 Mon Sep 17 00:00:00 2001 From: Timothy J Fontaine Date: Tue, 7 Jan 2014 14:03:15 -0800 Subject: [PATCH 2/6] 2014.01.08, Version 0.10.22 (Stable) Changes since version 0.10.21: * windows: avoid assertion failure when pipe server is closed (Bert Belder) --- ChangeLog | 7 +++++++ src/version.c | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 229b7d72..d55fd009 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2014.01.08, Version 0.10.22 (Stable) + +Changes since version 0.10.21: + +* windows: avoid assertion failure when pipe server is closed (Bert Belder) + + 2013.12.19, Version 0.10.21 (Stable), 375ebce068555f0ca8151b562edb5f1b263022db Changes since version 0.10.20: diff --git a/src/version.c b/src/version.c index c03e2b36..6786ec77 100644 --- a/src/version.c +++ b/src/version.c @@ -35,7 +35,7 @@ #define UV_VERSION_MAJOR 0 #define UV_VERSION_MINOR 10 #define UV_VERSION_PATCH 22 -#define UV_VERSION_IS_RELEASE 0 +#define UV_VERSION_IS_RELEASE 1 #define UV_VERSION ((UV_VERSION_MAJOR << 16) | \ From 97eda7fd6226be651216ec19ba6146807186bd36 Mon Sep 17 00:00:00 2001 From: Timothy J Fontaine Date: Tue, 7 Jan 2014 14:03:18 -0800 Subject: [PATCH 3/6] Now working on v0.10.23 --- ChangeLog | 2 +- src/version.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index d55fd009..0857e9a0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,4 @@ -2014.01.08, Version 0.10.22 (Stable) +2014.01.08, Version 0.10.22 (Stable), f526c90eeff271d9323a9107b9a64a4671fd3103 Changes since version 0.10.21: diff --git a/src/version.c b/src/version.c index 6786ec77..2c08c822 100644 --- a/src/version.c +++ b/src/version.c @@ -34,8 +34,8 @@ #define UV_VERSION_MAJOR 0 #define UV_VERSION_MINOR 10 -#define UV_VERSION_PATCH 22 -#define UV_VERSION_IS_RELEASE 1 +#define UV_VERSION_PATCH 23 +#define UV_VERSION_IS_RELEASE 0 #define UV_VERSION ((UV_VERSION_MAJOR << 16) | \ From 993151bc409c273409dbbaaac192091c864f3823 Mon Sep 17 00:00:00 2001 From: Luca Bruno Date: Mon, 20 Jan 2014 13:56:26 +0100 Subject: [PATCH 4/6] linux: relax assumption on /proc/stat parsing CPU entries in /proc/stat are not guaranteed to be monotonically increasing, asserting on this assumption can break in cases such as the UltraSparc II machine shown in #1080. Signed-off-by: Luca Bruno --- src/unix/linux-core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/unix/linux-core.c b/src/unix/linux-core.c index 7c34ea16..74294eb2 100644 --- a/src/unix/linux-core.c +++ b/src/unix/linux-core.c @@ -601,9 +601,9 @@ static int read_times(unsigned int numcpus, uv_cpu_info_t* ci) { /* skip "cpu " marker */ { - unsigned int n = num; + unsigned int n; + assert(sscanf(buf, "cpu%u ", &n) == 1); for (len = sizeof("cpu0"); n /= 10; len++); - assert(sscanf(buf, "cpu%u ", &n) == 1 && n == num); } /* Line contains user, nice, system, idle, iowait, irq, softirq, steal, @@ -630,6 +630,7 @@ static int read_times(unsigned int numcpus, uv_cpu_info_t* ci) { ci[num++].cpu_times = ts; } fclose(fp); + assert(num == numcpus); return 0; } From 8bc29b6f5fff71c69987c44e3cfbb3b79b882398 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 21 Jan 2014 15:05:39 +0400 Subject: [PATCH 5/6] openbsd: fix obvious bug in uv_cpu_info `int which[]` should not be static here, as the function itself is changing it fix joyent/node#6878 --- src/unix/openbsd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/unix/openbsd.c b/src/unix/openbsd.c index 4dfcd76b..ec83a818 100644 --- a/src/unix/openbsd.c +++ b/src/unix/openbsd.c @@ -223,7 +223,7 @@ uv_err_t uv_cpu_info(uv_cpu_info_t** cpu_infos, int* count) { uint64_t info[CPUSTATES]; char model[512]; int numcpus = 1; - static int which[] = {CTL_HW,HW_MODEL,0}; + int which[] = {CTL_HW,HW_MODEL,0}; size_t size; int i; uv_cpu_info_t* cpu_info; From e403a2c486f46c0f5eda8814ff13d4ed4521e5a6 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Mon, 20 Jan 2014 19:30:08 +0400 Subject: [PATCH 6/6] process: close stdio after dup2'ing it Thus allow passing the same file descriptor as the source of multiple stdios. Committed with the help and code parts from: * Sam Roberts fix #1074 --- src/unix/process.c | 41 +++++++++++------- test/test-list.h | 2 + test/test-spawn.c | 102 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 16 deletions(-) diff --git a/src/unix/process.c b/src/unix/process.c index 389817f2..ca029b1c 100644 --- a/src/unix/process.c +++ b/src/unix/process.c @@ -290,32 +290,41 @@ static void uv__process_child_init(uv_process_options_t options, close_fd = pipes[fd][0]; use_fd = pipes[fd][1]; - if (use_fd >= 0) - close(close_fd); - else if (fd >= 3) - continue; - else { - /* redirect stdin, stdout and stderr to /dev/null even if UV_IGNORE is - * set - */ - use_fd = open("/dev/null", fd == 0 ? O_RDONLY : O_RDWR); + if (use_fd < 0) { + if (fd >= 3) + continue; + else { + /* redirect stdin, stdout and stderr to /dev/null even if UV_IGNORE is + * set + */ + use_fd = open("/dev/null", fd == 0 ? O_RDONLY : O_RDWR); + close_fd = use_fd; - if (use_fd == -1) { - uv__write_int(error_fd, errno); - perror("failed to open stdio"); - _exit(127); + if (use_fd == -1) { + uv__write_int(error_fd, errno); + perror("failed to open stdio"); + _exit(127); + } } } if (fd == use_fd) uv__cloexec(use_fd, 0); - else { + else dup2(use_fd, fd); - close(use_fd); - } if (fd <= 2) uv__nonblock(fd, 0); + + if (close_fd != -1) + close(close_fd); + } + + for (fd = 0; fd < stdio_count; fd++) { + use_fd = pipes[fd][1]; + + if (use_fd >= 0 && fd != use_fd) + close(use_fd); } if (options.cwd && chdir(options.cwd)) { diff --git a/test/test-list.h b/test/test-list.h index 11fc4983..06c56fca 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -160,6 +160,7 @@ TEST_DECLARE (spawn_preserve_env) TEST_DECLARE (spawn_setuid_fails) TEST_DECLARE (spawn_setgid_fails) TEST_DECLARE (spawn_stdout_to_file) +TEST_DECLARE (spawn_stdout_and_stderr_to_file) TEST_DECLARE (spawn_auto_unref) TEST_DECLARE (fs_poll) TEST_DECLARE (kill) @@ -435,6 +436,7 @@ TASK_LIST_START TEST_ENTRY (spawn_setuid_fails) TEST_ENTRY (spawn_setgid_fails) TEST_ENTRY (spawn_stdout_to_file) + TEST_ENTRY (spawn_stdout_and_stderr_to_file) TEST_ENTRY (spawn_auto_unref) TEST_ENTRY (fs_poll) TEST_ENTRY (kill) diff --git a/test/test-spawn.c b/test/test-spawn.c index 56eca9c5..43889878 100644 --- a/test/test-spawn.c +++ b/test/test-spawn.c @@ -263,6 +263,61 @@ TEST_IMPL(spawn_stdout_to_file) { } +TEST_IMPL(spawn_stdout_and_stderr_to_file) { + int r; + uv_file file; + uv_fs_t fs_req; + uv_stdio_container_t stdio[3]; + + /* Setup. */ + unlink("stdout_file"); + + init_process_options("spawn_helper6", exit_cb); + + r = uv_fs_open(uv_default_loop(), &fs_req, "stdout_file", O_CREAT | O_RDWR, + S_IREAD | S_IWRITE, NULL); + ASSERT(r != -1); + uv_fs_req_cleanup(&fs_req); + + file = r; + + options.stdio = stdio; + options.stdio[0].flags = UV_IGNORE; + options.stdio[1].flags = UV_INHERIT_FD; + options.stdio[1].data.fd = file; + options.stdio[2].flags = UV_INHERIT_FD; + options.stdio[2].data.fd = file; + options.stdio_count = 3; + + r = uv_spawn(uv_default_loop(), &process, options); + ASSERT(r == 0); + + r = uv_run(uv_default_loop(), UV_RUN_DEFAULT); + ASSERT(r == 0); + + ASSERT(exit_cb_called == 1); + ASSERT(close_cb_called == 1); + + r = uv_fs_read(uv_default_loop(), &fs_req, file, output, sizeof(output), + 0, NULL); + ASSERT(r == 27); + uv_fs_req_cleanup(&fs_req); + + r = uv_fs_close(uv_default_loop(), &fs_req, file, NULL); + ASSERT(r == 0); + uv_fs_req_cleanup(&fs_req); + + printf("output is: %s", output); + ASSERT(strcmp("hello world\nhello errworld\n", output) == 0); + + /* Cleanup. */ + unlink("stdout_file"); + + MAKE_VALGRIND_HAPPY(); + return 0; +} + + TEST_IMPL(spawn_stdin) { int r; uv_pipe_t out; @@ -562,6 +617,53 @@ TEST_IMPL(spawn_and_ping) { } +TEST_IMPL(spawn_same_stdout_stderr) { + uv_write_t write_req; + uv_pipe_t in, out; + uv_buf_t buf; + uv_stdio_container_t stdio[3]; + int r; + + init_process_options("spawn_helper3", exit_cb); + buf = uv_buf_init("TEST", 4); + + uv_pipe_init(uv_default_loop(), &out, 0); + uv_pipe_init(uv_default_loop(), &in, 0); + options.stdio = stdio; + options.stdio[0].flags = UV_CREATE_PIPE | UV_READABLE_PIPE; + options.stdio[0].data.stream = (uv_stream_t*)∈ + options.stdio[1].flags = UV_CREATE_PIPE | UV_WRITABLE_PIPE; + options.stdio[1].data.stream = (uv_stream_t*)&out; + options.stdio_count = 2; + + r = uv_spawn(uv_default_loop(), &process, options); + ASSERT(r == 0); + + /* Sending signum == 0 should check if the + * child process is still alive, not kill it. + */ + r = uv_process_kill(&process, 0); + ASSERT(r == 0); + + r = uv_write(&write_req, (uv_stream_t*)&in, &buf, 1, write_cb); + ASSERT(r == 0); + + r = uv_read_start((uv_stream_t*)&out, on_alloc, on_read); + ASSERT(r == 0); + + ASSERT(exit_cb_called == 0); + + r = uv_run(uv_default_loop(), UV_RUN_DEFAULT); + ASSERT(r == 0); + + ASSERT(exit_cb_called == 1); + ASSERT(strcmp(output, "TEST") == 0); + + MAKE_VALGRIND_HAPPY(); + return 0; +} + + TEST_IMPL(kill) { int r; uv_err_t err;