luo980/libuv
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

unix: return exec errors from uv_spawn, not async

Browse Source 
If spawning a process fails due to an exec() failure (but it succeeded
in forking), then this should be considered a spawn failure instead of
an asynchronous termination of the process. This allows to check for
common exec() failure conditions such as a bad path quickly instead of
having to rely on keeping track of the async callback.

Additionally, the meaning of the two fields returned in the callback are
now exactly what they advertise to be. The process exit argument is not
one of two values depending on what happened to the child.

Fixes #978.
This commit is contained in:
Alex Crichton 2013-10-31 11:52:55 -07:00 committed by Ben Noordhuis
parent 2445467810
commit f764bff699
6 changed files with 43 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/uv-unix.h
View File

@ -287,7 +287,6 @@ typedef struct {
#define UV_PROCESS_PRIVATE_FIELDS \
void* queue[2]; \
int errorno; \
int status; \
#define UV_FS_PRIVATE_FIELDS \

1
include/uv-win.h
View File

@ -533,7 +533,6 @@ RB_HEAD(uv_timer_tree_s, uv_timer_s);
UV_REQ_FIELDS \
} exit_req; \
BYTE* child_stdio_buffer; \
int spawn_error; \
int exit_signal; \
HANDLE wait_handle; \
HANDLE process_handle; \

12
include/uv.h
View File

@ -1489,7 +1489,17 @@ struct uv_process_s {
UV_PROCESS_PRIVATE_FIELDS
};
/* Initializes uv_process_t and starts the process. */
/*
* Initializes the uv_process_t and starts the process. If the process is
* successfully spawned, then this function will return 0. Otherwise, the
* negative error code corresponding to the reason it couldn't spawn is
* returned.
*
* Possible reasons for failing to spawn would include (but not be limited to)
* the file to execute not existing, not having permissions to use the setuid or
* setgid specified, or not having enough memory to allocate for the new
* process.
*/
UV_EXTERN int uv_spawn(uv_loop_t* loop,
uv_process_t* handle,
const uv_process_options_t* options);

21
src/unix/process.c
View File

@ -109,9 +109,6 @@ static void uv__chld(uv_signal_t* handle, int signum) {
if (WIFSIGNALED(process->status))
term_signal = WTERMSIG(process->status);
if (process->errorno != 0)
exit_status = process->errorno; /* execve() failed */
process->exit_cb(process, exit_status, term_signal);
}
}
@ -359,6 +356,7 @@ int uv_spawn(uv_loop_t* loop,
ssize_t r;
pid_t pid;
int err;
int exec_errorno;
int i;
assert(options->file != NULL);
@ -434,14 +432,14 @@ int uv_spawn(uv_loop_t* loop,
uv__close(signal_pipe[1]);
process->status = 0;
process->errorno = 0;
exec_errorno = 0;
do
r = read(signal_pipe[0], &process->errorno, sizeof(process->errorno));
r = read(signal_pipe[0], &exec_errorno, sizeof(exec_errorno));
while (r == -1 && errno == EINTR);
if (r == 0)
; /* okay, EOF */
else if (r == sizeof(process->errorno))
else if (r == sizeof(exec_errorno))
; /* okay, read errorno */
else if (r == -1 && errno == EPIPE)
; /* okay, got EPIPE */
@ -461,15 +459,18 @@ int uv_spawn(uv_loop_t* loop,
goto error;
}
q = uv__process_queue(loop, pid);
QUEUE_INSERT_TAIL(q, &process->queue);
/* Only activate this handle if exec() happened successfully */
if (exec_errorno == 0) {
q = uv__process_queue(loop, pid);
QUEUE_INSERT_TAIL(q, &process->queue);
uv__handle_start(process);
}
process->pid = pid;
process->exit_cb = options->exit_cb;
uv__handle_start(process);
free(pipes);
return 0;
return exec_errorno;
error:
if (pipes != NULL) {

23
src/win/process.c
View File

@ -127,7 +127,6 @@ static void uv_process_init(uv_loop_t* loop, uv_process_t* handle) {
uv__handle_init(loop, (uv_handle_t*) handle, UV_PROCESS);
handle->exit_cb = NULL;
handle->pid = 0;
handle->spawn_error = 0;
handle->exit_signal = 0;
handle->wait_handle = INVALID_HANDLE_VALUE;
handle->process_handle = INVALID_HANDLE_VALUE;
@ -752,10 +751,7 @@ void uv_process_proc_exit(uv_loop_t* loop, uv_process_t* handle) {
/* callback.*/
uv__handle_stop(handle);
if (handle->spawn_error) {
/* Spawning failed. */
exit_code = uv_translate_sys_error(handle->spawn_error);
} else if (GetExitCodeProcess(handle->process_handle, &status)) {
if (GetExitCodeProcess(handle->process_handle, &status)) {
exit_code = status;
} else {
/* Unable to to obtain the exit code. This should never happen. */
@ -1025,25 +1021,20 @@ int uv_spawn(uv_loop_t* loop,
free(env);
free(path);
process->spawn_error = err;
if (process->child_stdio_buffer != NULL) {
/* Clean up child stdio handles. */
uv__stdio_destroy(process->child_stdio_buffer);
process->child_stdio_buffer = NULL;
}
/* Make the handle active. It will remain active until the exit callback */
/* is made or the handle is closed, whichever happens first. */
uv__handle_start(process);
/* If an error happened, queue the exit req. */
if (err) {
process->exit_cb_pending = 1;
uv_insert_pending_req(loop, (uv_req_t*) &process->exit_req);
/* Make the handle active, but only if an error didn't happen. It will */
/* remain active until the exit callback is made or the handle is closed, */
/* whichever happens first. */
if (err == 0) {
uv__handle_start(process);
}
return 0;
return err;
/* This code path is taken when we run into an error that we want to */
/* report immediately. */

50
test/test-spawn.c
View File

@ -51,7 +51,6 @@ static void close_cb(uv_handle_t* handle) {
close_cb_called++;
}
static void exit_cb(uv_process_t* process,
int64_t exit_status,
int term_signal) {
@ -63,29 +62,10 @@ static void exit_cb(uv_process_t* process,
}
static void expect(uv_process_t* process,
int64_t exit_status,
int term_signal,
int err) {
printf("exit_cb\n");
exit_cb_called++;
ASSERT(exit_status == err);
ASSERT(term_signal == 0);
uv_close((uv_handle_t*)process, close_cb);
}
static void exit_cb_expect_enoent(uv_process_t* process,
int64_t exit_status,
int term_signal) {
expect(process, exit_status, term_signal, UV_ENOENT);
}
static void exit_cb_expect_eperm(uv_process_t* process,
int64_t exit_status,
int term_signal) {
expect(process, exit_status, term_signal, UV_EPERM);
static void fail_cb(uv_process_t* process,
int64_t exit_status,
int term_signal) {
ASSERT(0 && "fail_cb called");
}
@ -166,12 +146,12 @@ static void timer_cb(uv_timer_t* handle, int status) {
TEST_IMPL(spawn_fails) {
init_process_options("", exit_cb_expect_enoent);
init_process_options("", fail_cb);
options.file = options.args[0] = "program-that-had-better-not-exist";
ASSERT(0 == uv_spawn(uv_default_loop(), &process, &options));
ASSERT(1 == uv_is_active((uv_handle_t*) &process));
ASSERT(UV_ENOENT == uv_spawn(uv_default_loop(), &process, &options));
ASSERT(0 == uv_is_active((uv_handle_t*) &process));
ASSERT(0 == uv_run(uv_default_loop(), UV_RUN_DEFAULT));
ASSERT(1 == exit_cb_called);
MAKE_VALGRIND_HAPPY();
return 0;
@ -851,19 +831,18 @@ TEST_IMPL(spawn_setuid_fails) {
ASSERT(0 == setuid(pw->pw_uid));
}
init_process_options("spawn_helper1", exit_cb_expect_eperm);
init_process_options("spawn_helper1", fail_cb);
options.flags |= UV_PROCESS_SETUID;
options.uid = 0;
r = uv_spawn(uv_default_loop(), &process, &options);
ASSERT(r == 0);
ASSERT(r == UV_EPERM);
r = uv_run(uv_default_loop(), UV_RUN_DEFAULT);
ASSERT(r == 0);
ASSERT(exit_cb_called == 1);
ASSERT(close_cb_called == 1);
ASSERT(close_cb_called == 0);
MAKE_VALGRIND_HAPPY();
return 0;
@ -883,19 +862,18 @@ TEST_IMPL(spawn_setgid_fails) {
ASSERT(0 == setuid(pw->pw_uid));
}
init_process_options("spawn_helper1", exit_cb_expect_eperm);
init_process_options("spawn_helper1", fail_cb);
options.flags |= UV_PROCESS_SETGID;
options.gid = 0;
r = uv_spawn(uv_default_loop(), &process, &options);
ASSERT(r == 0);
ASSERT(r == UV_EPERM);
r = uv_run(uv_default_loop(), UV_RUN_DEFAULT);
ASSERT(r == 0);
ASSERT(exit_cb_called == 1);
ASSERT(close_cb_called == 1);
ASSERT(close_cb_called == 0);
MAKE_VALGRIND_HAPPY();
return 0;