diff --git a/include/uv-unix.h b/include/uv-unix.h index 965fbafb..45006092 100644 --- a/include/uv-unix.h +++ b/include/uv-unix.h @@ -287,7 +287,6 @@ typedef struct { #define UV_PROCESS_PRIVATE_FIELDS \ void* queue[2]; \ - int errorno; \ int status; \ #define UV_FS_PRIVATE_FIELDS \ diff --git a/include/uv-win.h b/include/uv-win.h index 512d7f89..e4e1f839 100644 --- a/include/uv-win.h +++ b/include/uv-win.h @@ -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; \ diff --git a/include/uv.h b/include/uv.h index e6b290bc..c3ba2502 100644 --- a/include/uv.h +++ b/include/uv.h @@ -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); diff --git a/src/unix/process.c b/src/unix/process.c index 42990ce5..6854ac11 100644 --- a/src/unix/process.c +++ b/src/unix/process.c @@ -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) { diff --git a/src/win/process.c b/src/win/process.c index 218ea8f2..a5bb7437 100644 --- a/src/win/process.c +++ b/src/win/process.c @@ -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. */ diff --git a/test/test-spawn.c b/test/test-spawn.c index 454c1816..0a355af0 100644 --- a/test/test-spawn.c +++ b/test/test-spawn.c @@ -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;