From 1c0281e3e210aa97f7faaa5725cb95538f1c97fd Mon Sep 17 00:00:00 2001 From: Santiago Gimeno Date: Tue, 29 Mar 2016 18:55:14 +0200 Subject: [PATCH] process: fix uv_spawn edge-case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It can happen that the `parent` end of the `signal_pipe` is given a STDIO file descriptor, so closing it using `uv__close` fails. This problem is happening when running the `spawn_closed_process_io` test in `SmartOS`. The reason being that when creating a socketpair in `uv__process_init_stdio`, the `Illumos` implementation uses 3 sockets: one is used as a listener, and the other 2 represent both ends of the pipe. The listener socket is closed once the pipe is created. In the test, the listener socket is assigned to the `0` fd, as it is the first free fd in the system. So the fd `0` remained free after the call to `socketpair`. Afterwards, when creating the `signal_pipe`, the fd `0` is being assigned again, so closing it with `uv__close` made the test fail. This issue is not happening in the other unixes because `socketpair` doesn't use 3 fd's, but only 2. To solve the issue, a new `uv__close__nocheckstdio()` function has been added and used. PR-URL: https://github.com/libuv/libuv/pull/796 Reviewed-By: Ben Noordhuis Reviewed-By: Saúl Ibarra Corretgé --- src/unix/core.c | 9 +++++++-- src/unix/internal.h | 1 + src/unix/process.c | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/unix/core.c b/src/unix/core.c index b7f7c25f..9aaca841 100644 --- a/src/unix/core.c +++ b/src/unix/core.c @@ -498,12 +498,11 @@ skip: } -int uv__close(int fd) { +int uv__close_nocheckstdio(int fd) { int saved_errno; int rc; assert(fd > -1); /* Catch uninitialized io_watcher.fd bugs. */ - assert(fd > STDERR_FILENO); /* Catch stdio close bugs. */ saved_errno = errno; rc = close(fd); @@ -518,6 +517,12 @@ int uv__close(int fd) { } +int uv__close(int fd) { + assert(fd > STDERR_FILENO); /* Catch stdio close bugs. */ + return uv__close_nocheckstdio(fd); +} + + #if defined(__linux__) || defined(__FreeBSD__) || defined(__APPLE__) || \ defined(_AIX) || defined(__DragonFly__) diff --git a/src/unix/internal.h b/src/unix/internal.h index 328c6632..938e76f1 100644 --- a/src/unix/internal.h +++ b/src/unix/internal.h @@ -177,6 +177,7 @@ struct uv__stream_queued_fds_s { /* core */ int uv__nonblock(int fd, int set); int uv__close(int fd); +int uv__close_nocheckstdio(int fd); int uv__cloexec(int fd, int set); int uv__socket(int domain, int type, int protocol); int uv__dup(int fd); diff --git a/src/unix/process.c b/src/unix/process.c index 571f8cd7..02468e6e 100644 --- a/src/unix/process.c +++ b/src/unix/process.c @@ -498,7 +498,7 @@ int uv_spawn(uv_loop_t* loop, } else abort(); - uv__close(signal_pipe[0]); + uv__close_nocheckstdio(signal_pipe[0]); for (i = 0; i < options->stdio_count; i++) { err = uv__process_open_stream(options->stdio + i, pipes[i], i == 0);