Make process_handle_t private and fix some unit tests

Let's *not* expose more cross-platform-compatibility structures, or
expect code to use them right.

Also, don't fclose() stdout_handle and stdin_handle until we do
tor_process_handle_destroy, or we risk a double-fclose.
This commit is contained in:
Nick Mathewson 2011-11-25 16:47:25 -05:00
parent 093e6724c7
commit d6c18c5804
4 changed files with 141 additions and 86 deletions

View File

@ -3165,7 +3165,7 @@ int
tor_terminate_process(process_handle_t *process_handle) tor_terminate_process(process_handle_t *process_handle)
{ {
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
if (tor_get_exit_code(*process_handle, 0, NULL) == PROCESS_EXIT_RUNNING) { if (tor_get_exit_code(process_handle, 0, NULL) == PROCESS_EXIT_RUNNING) {
HANDLE handle; HANDLE handle;
/* If the signal is outside of what GenerateConsoleCtrlEvent can use, /* If the signal is outside of what GenerateConsoleCtrlEvent can use,
attempt to open and terminate the process. */ attempt to open and terminate the process. */
@ -3197,6 +3197,34 @@ tor_process_get_pid(process_handle_t *process_handle)
#endif #endif
} }
#ifdef MS_WINDOWS
HANDLE
tor_process_get_stdout_pipe(process_handle_t *process_handle)
{
return process_handle->stdout_pipe;
}
#else
FILE *
tor_process_get_stdout_pipe(process_handle_t *process_handle)
{
return process_handle->stdout_handle;
}
#endif
static process_handle_t *
process_handle_new(void)
{
process_handle_t *out = tor_malloc_zero(sizeof(process_handle_t));
#ifndef MS_WINDOWS
out->stdout_pipe = -1;
out->stderr_pipe = -1;
#endif
return out;
}
/*DOCDOC*/
#define CHILD_STATE_INIT 0 #define CHILD_STATE_INIT 0
#define CHILD_STATE_PIPE 1 #define CHILD_STATE_PIPE 1
#define CHILD_STATE_MAXFD 2 #define CHILD_STATE_MAXFD 2
@ -3234,13 +3262,15 @@ tor_spawn_background(const char *const filename, const char **argv,
#else #else
const char **envp, const char **envp,
#endif #endif
process_handle_t *process_handle) process_handle_t **process_handle_out)
{ {
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
HANDLE stdout_pipe_read = NULL; HANDLE stdout_pipe_read = NULL;
HANDLE stdout_pipe_write = NULL; HANDLE stdout_pipe_write = NULL;
HANDLE stderr_pipe_read = NULL; HANDLE stderr_pipe_read = NULL;
HANDLE stderr_pipe_write = NULL; HANDLE stderr_pipe_write = NULL;
process_handle_t *process_handle;
int status;
STARTUPINFO siStartInfo; STARTUPINFO siStartInfo;
BOOL retval = FALSE; BOOL retval = FALSE;
@ -3250,30 +3280,26 @@ tor_spawn_background(const char *const filename, const char **argv,
(void)envp; // Unused on Windows (void)envp; // Unused on Windows
/* process_handle must not be NULL */
tor_assert(process_handle != NULL);
saAttr.nLength = sizeof(SECURITY_ATTRIBUTES); saAttr.nLength = sizeof(SECURITY_ATTRIBUTES);
saAttr.bInheritHandle = TRUE; saAttr.bInheritHandle = TRUE;
/* TODO: should we set explicit security attributes? (#2046, comment 5) */ /* TODO: should we set explicit security attributes? (#2046, comment 5) */
saAttr.lpSecurityDescriptor = NULL; saAttr.lpSecurityDescriptor = NULL;
/* Assume failure to start process */ /* Assume failure to start process */
memset(process_handle, 0, sizeof(process_handle_t)); status = PROCESS_STATUS_ERROR;
process_handle->status = PROCESS_STATUS_ERROR;
/* Set up pipe for stdout */ /* Set up pipe for stdout */
if (!CreatePipe(&stdout_pipe_read, &stdout_pipe_write, &saAttr, 0)) { if (!CreatePipe(&stdout_pipe_read, &stdout_pipe_write, &saAttr, 0)) {
log_warn(LD_GENERAL, log_warn(LD_GENERAL,
"Failed to create pipe for stdout communication with child process: %s", "Failed to create pipe for stdout communication with child process: %s",
format_win32_error(GetLastError())); format_win32_error(GetLastError()));
return process_handle->status; return status;
} }
if (!SetHandleInformation(stdout_pipe_read, HANDLE_FLAG_INHERIT, 0)) { if (!SetHandleInformation(stdout_pipe_read, HANDLE_FLAG_INHERIT, 0)) {
log_warn(LD_GENERAL, log_warn(LD_GENERAL,
"Failed to configure pipe for stdout communication with child " "Failed to configure pipe for stdout communication with child "
"process: %s", format_win32_error(GetLastError())); "process: %s", format_win32_error(GetLastError()));
return process_handle->status; return status;
} }
/* Set up pipe for stderr */ /* Set up pipe for stderr */
@ -3281,13 +3307,13 @@ tor_spawn_background(const char *const filename, const char **argv,
log_warn(LD_GENERAL, log_warn(LD_GENERAL,
"Failed to create pipe for stderr communication with child process: %s", "Failed to create pipe for stderr communication with child process: %s",
format_win32_error(GetLastError())); format_win32_error(GetLastError()));
return process_handle->status; return status;
} }
if (!SetHandleInformation(stderr_pipe_read, HANDLE_FLAG_INHERIT, 0)) { if (!SetHandleInformation(stderr_pipe_read, HANDLE_FLAG_INHERIT, 0)) {
log_warn(LD_GENERAL, log_warn(LD_GENERAL,
"Failed to configure pipe for stderr communication with child " "Failed to configure pipe for stderr communication with child "
"process: %s", format_win32_error(GetLastError())); "process: %s", format_win32_error(GetLastError()));
return process_handle->status; return status;
} }
/* Create the child process */ /* Create the child process */
@ -3296,6 +3322,9 @@ tor_spawn_background(const char *const filename, const char **argv,
*/ */
joined_argv = tor_join_win_cmdline(argv); joined_argv = tor_join_win_cmdline(argv);
process_handle = process_handle_new();
process_handle->status = status;
ZeroMemory(&(process_handle->pid), sizeof(PROCESS_INFORMATION)); ZeroMemory(&(process_handle->pid), sizeof(PROCESS_INFORMATION));
ZeroMemory(&siStartInfo, sizeof(STARTUPINFO)); ZeroMemory(&siStartInfo, sizeof(STARTUPINFO));
siStartInfo.cb = sizeof(STARTUPINFO); siStartInfo.cb = sizeof(STARTUPINFO);
@ -3326,22 +3355,25 @@ tor_spawn_background(const char *const filename, const char **argv,
log_warn(LD_GENERAL, log_warn(LD_GENERAL,
"Failed to create child process %s: %s", filename?filename:argv[0], "Failed to create child process %s: %s", filename?filename:argv[0],
format_win32_error(GetLastError())); format_win32_error(GetLastError()));
tor_free(process_handle);
} else { } else {
/* TODO: Close hProcess and hThread in process_handle->pid? */ /* TODO: Close hProcess and hThread in process_handle->pid? */
process_handle->stdout_pipe = stdout_pipe_read; process_handle->stdout_pipe = stdout_pipe_read;
process_handle->stderr_pipe = stderr_pipe_read; process_handle->stderr_pipe = stderr_pipe_read;
process_handle->status = PROCESS_STATUS_RUNNING; status = process_handle->status = PROCESS_STATUS_RUNNING;
} }
/* TODO: Close pipes on exit */ /* TODO: Close pipes on exit */
*process_handle_out = process_handle;
return process_handle->status; return status;
#else // MS_WINDOWS #else // MS_WINDOWS
pid_t pid; pid_t pid;
int stdout_pipe[2]; int stdout_pipe[2];
int stderr_pipe[2]; int stderr_pipe[2];
int fd, retval; int fd, retval;
ssize_t nbytes; ssize_t nbytes;
process_handle_t *process_handle;
int status;
const char *error_message = SPAWN_ERROR_MESSAGE; const char *error_message = SPAWN_ERROR_MESSAGE;
size_t error_message_length; size_t error_message_length;
@ -3354,9 +3386,7 @@ tor_spawn_background(const char *const filename, const char **argv,
static int max_fd = -1; static int max_fd = -1;
/* Assume failure to start */ status = PROCESS_STATUS_ERROR;
memset(process_handle, 0, sizeof(process_handle_t));
process_handle->status = PROCESS_STATUS_ERROR;
/* We do the strlen here because strlen() is not signal handler safe, /* We do the strlen here because strlen() is not signal handler safe,
and we are not allowed to use unsafe functions between fork and exec */ and we are not allowed to use unsafe functions between fork and exec */
@ -3370,7 +3400,7 @@ tor_spawn_background(const char *const filename, const char **argv,
log_warn(LD_GENERAL, log_warn(LD_GENERAL,
"Failed to set up pipe for stdout communication with child process: %s", "Failed to set up pipe for stdout communication with child process: %s",
strerror(errno)); strerror(errno));
return process_handle->status; return status;
} }
retval = pipe(stderr_pipe); retval = pipe(stderr_pipe);
@ -3378,7 +3408,7 @@ tor_spawn_background(const char *const filename, const char **argv,
log_warn(LD_GENERAL, log_warn(LD_GENERAL,
"Failed to set up pipe for stderr communication with child process: %s", "Failed to set up pipe for stderr communication with child process: %s",
strerror(errno)); strerror(errno));
return process_handle->status; return status;
} }
child_state = CHILD_STATE_MAXFD; child_state = CHILD_STATE_MAXFD;
@ -3468,7 +3498,7 @@ tor_spawn_background(const char *const filename, const char **argv,
_exit(255); _exit(255);
/* Never reached, but avoids compiler warning */ /* Never reached, but avoids compiler warning */
return process_handle->status; return status;
} }
/* In parent */ /* In parent */
@ -3479,9 +3509,11 @@ tor_spawn_background(const char *const filename, const char **argv,
close(stdout_pipe[1]); close(stdout_pipe[1]);
close(stderr_pipe[0]); close(stderr_pipe[0]);
close(stderr_pipe[1]); close(stderr_pipe[1]);
return process_handle->status; return status;
} }
process_handle = process_handle_new();
process_handle->status = status;
process_handle->pid = pid; process_handle->pid = pid;
/* TODO: If the child process forked but failed to exec, waitpid it */ /* TODO: If the child process forked but failed to exec, waitpid it */
@ -3505,7 +3537,7 @@ tor_spawn_background(const char *const filename, const char **argv,
strerror(errno)); strerror(errno));
} }
process_handle->status = PROCESS_STATUS_RUNNING; status = process_handle->status = PROCESS_STATUS_RUNNING;
/* Set stdout/stderr pipes to be non-blocking */ /* Set stdout/stderr pipes to be non-blocking */
fcntl(process_handle->stdout_pipe, F_SETFL, O_NONBLOCK); fcntl(process_handle->stdout_pipe, F_SETFL, O_NONBLOCK);
fcntl(process_handle->stderr_pipe, F_SETFL, O_NONBLOCK); fcntl(process_handle->stderr_pipe, F_SETFL, O_NONBLOCK);
@ -3513,6 +3545,7 @@ tor_spawn_background(const char *const filename, const char **argv,
process_handle->stdout_handle = fdopen(process_handle->stdout_pipe, "r"); process_handle->stdout_handle = fdopen(process_handle->stdout_pipe, "r");
process_handle->stderr_handle = fdopen(process_handle->stderr_pipe, "r"); process_handle->stderr_handle = fdopen(process_handle->stderr_pipe, "r");
*process_handle_out = process_handle;
return process_handle->status; return process_handle->status;
#endif // MS_WINDOWS #endif // MS_WINDOWS
} }
@ -3525,6 +3558,9 @@ void
tor_process_handle_destroy(process_handle_t *process_handle, tor_process_handle_destroy(process_handle_t *process_handle,
int also_terminate_process) int also_terminate_process)
{ {
if (!process_handle)
return;
if (also_terminate_process) { if (also_terminate_process) {
if (tor_terminate_process(process_handle) < 0) { if (tor_terminate_process(process_handle) < 0) {
log_notice(LD_GENERAL, "Failed to terminate process with PID '%d'", log_notice(LD_GENERAL, "Failed to terminate process with PID '%d'",
@ -3551,6 +3587,7 @@ tor_process_handle_destroy(process_handle_t *process_handle,
fclose(process_handle->stderr_handle); fclose(process_handle->stderr_handle);
#endif #endif
memset(process_handle, 0x0f, sizeof(process_handle_t));
tor_free(process_handle); tor_free(process_handle);
} }
@ -3565,7 +3602,7 @@ tor_process_handle_destroy(process_handle_t *process_handle,
* probably not work in Tor, because waitpid() is called in main.c to reap any * probably not work in Tor, because waitpid() is called in main.c to reap any
* terminated child processes.*/ * terminated child processes.*/
int int
tor_get_exit_code(const process_handle_t process_handle, tor_get_exit_code(const process_handle_t *process_handle,
int block, int *exit_code) int block, int *exit_code)
{ {
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
@ -3574,14 +3611,14 @@ tor_get_exit_code(const process_handle_t process_handle,
if (block) { if (block) {
/* Wait for the process to exit */ /* Wait for the process to exit */
retval = WaitForSingleObject(process_handle.pid.hProcess, INFINITE); retval = WaitForSingleObject(process_handle->pid.hProcess, INFINITE);
if (retval != WAIT_OBJECT_0) { if (retval != WAIT_OBJECT_0) {
log_warn(LD_GENERAL, "WaitForSingleObject() failed (%d): %s", log_warn(LD_GENERAL, "WaitForSingleObject() failed (%d): %s",
(int)retval, format_win32_error(GetLastError())); (int)retval, format_win32_error(GetLastError()));
return PROCESS_EXIT_ERROR; return PROCESS_EXIT_ERROR;
} }
} else { } else {
retval = WaitForSingleObject(process_handle.pid.hProcess, 0); retval = WaitForSingleObject(process_handle->pid.hProcess, 0);
if (WAIT_TIMEOUT == retval) { if (WAIT_TIMEOUT == retval) {
/* Process has not exited */ /* Process has not exited */
return PROCESS_EXIT_RUNNING; return PROCESS_EXIT_RUNNING;
@ -3593,7 +3630,7 @@ tor_get_exit_code(const process_handle_t process_handle,
} }
if (exit_code != NULL) { if (exit_code != NULL) {
success = GetExitCodeProcess(process_handle.pid.hProcess, success = GetExitCodeProcess(process_handle->pid.hProcess,
(PDWORD)exit_code); (PDWORD)exit_code);
if (!success) { if (!success) {
log_warn(LD_GENERAL, "GetExitCodeProcess() failed: %s", log_warn(LD_GENERAL, "GetExitCodeProcess() failed: %s",
@ -3605,19 +3642,19 @@ tor_get_exit_code(const process_handle_t process_handle,
int stat_loc; int stat_loc;
int retval; int retval;
retval = waitpid(process_handle.pid, &stat_loc, block?0:WNOHANG); retval = waitpid(process_handle->pid, &stat_loc, block?0:WNOHANG);
if (!block && 0 == retval) { if (!block && 0 == retval) {
/* Process has not exited */ /* Process has not exited */
return PROCESS_EXIT_RUNNING; return PROCESS_EXIT_RUNNING;
} else if (retval != process_handle.pid) { } else if (retval != process_handle->pid) {
log_warn(LD_GENERAL, "waitpid() failed for PID %d: %s", process_handle.pid, log_warn(LD_GENERAL, "waitpid() failed for PID %d: %s", process_handle->pid,
strerror(errno)); strerror(errno));
return PROCESS_EXIT_ERROR; return PROCESS_EXIT_ERROR;
} }
if (!WIFEXITED(stat_loc)) { if (!WIFEXITED(stat_loc)) {
log_warn(LD_GENERAL, "Process %d did not exit normally", log_warn(LD_GENERAL, "Process %d did not exit normally",
process_handle.pid); process_handle->pid);
return PROCESS_EXIT_ERROR; return PROCESS_EXIT_ERROR;
} }
@ -3719,7 +3756,6 @@ tor_read_all_handle(FILE *h, char *buf, size_t count,
if (NULL == retval) { if (NULL == retval) {
if (feof(h)) { if (feof(h)) {
log_debug(LD_GENERAL, "fgets() reached end of file"); log_debug(LD_GENERAL, "fgets() reached end of file");
fclose(h);
if (eof) if (eof)
*eof = 1; *eof = 1;
break; break;
@ -3732,7 +3768,6 @@ tor_read_all_handle(FILE *h, char *buf, size_t count,
} else { } else {
log_warn(LD_GENERAL, "fgets() from handle failed: %s", log_warn(LD_GENERAL, "fgets() from handle failed: %s",
strerror(errno)); strerror(errno));
fclose(h);
return -1; return -1;
} }
} }
@ -3892,12 +3927,10 @@ log_from_pipe(FILE *stream, int severity, const char *executable,
r = get_string_from_pipe(stream, buf, sizeof(buf) - 1); r = get_string_from_pipe(stream, buf, sizeof(buf) - 1);
if (r == IO_STREAM_CLOSED) { if (r == IO_STREAM_CLOSED) {
fclose(stream);
return 1; return 1;
} else if (r == IO_STREAM_EAGAIN) { } else if (r == IO_STREAM_EAGAIN) {
return 0; return 0;
} else if (r == IO_STREAM_TERM) { } else if (r == IO_STREAM_TERM) {
fclose(stream);
return -1; return -1;
} }
@ -4005,7 +4038,7 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port,
/* Static variables are initialized to zero, so child_handle.status=0 /* Static variables are initialized to zero, so child_handle.status=0
* which corresponds to it not running on startup */ * which corresponds to it not running on startup */
static process_handle_t child_handle; static process_handle_t *child_handle=NULL;
static time_t time_to_run_helper = 0; static time_t time_to_run_helper = 0;
int stdout_status, stderr_status, retval; int stdout_status, stderr_status, retval;
@ -4031,18 +4064,26 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port,
argv[9] = NULL; argv[9] = NULL;
/* Start the child, if it is not already running */ /* Start the child, if it is not already running */
if (child_handle.status != PROCESS_STATUS_RUNNING && if ((!child_handle || child_handle->status != PROCESS_STATUS_RUNNING) &&
time_to_run_helper < now) { time_to_run_helper < now) {
int status;
/* Assume tor-fw-helper will succeed, start it later*/ /* Assume tor-fw-helper will succeed, start it later*/
time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_SUCCESS; time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_SUCCESS;
if (child_handle) {
tor_process_handle_destroy(child_handle, 1);
child_handle = NULL;
}
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
/* Passing NULL as lpApplicationName makes Windows search for the .exe */ /* Passing NULL as lpApplicationName makes Windows search for the .exe */
tor_spawn_background(NULL, argv, NULL, &child_handle); status = tor_spawn_background(NULL, argv, NULL, &child_handle);
#else #else
tor_spawn_background(filename, argv, NULL, &child_handle); status = tor_spawn_background(filename, argv, NULL, &child_handle);
#endif #endif
if (PROCESS_STATUS_ERROR == child_handle.status) {
if (PROCESS_STATUS_ERROR == status) {
log_warn(LD_GENERAL, "Failed to start port forwarding helper %s", log_warn(LD_GENERAL, "Failed to start port forwarding helper %s",
filename); filename);
time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_FAIL; time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_FAIL;
@ -4051,22 +4092,22 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port,
log_info(LD_GENERAL, log_info(LD_GENERAL,
"Started port forwarding helper (%s) with pid '%d'", "Started port forwarding helper (%s) with pid '%d'",
filename, tor_process_get_pid(&child_handle)); filename, tor_process_get_pid(child_handle));
} }
/* If child is running, read from its stdout and stderr) */ /* If child is running, read from its stdout and stderr) */
if (PROCESS_STATUS_RUNNING == child_handle.status) { if (child_handle && PROCESS_STATUS_RUNNING == child_handle->status) {
/* Read from stdout/stderr and log result */ /* Read from stdout/stderr and log result */
retval = 0; retval = 0;
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
stdout_status = log_from_handle(child_handle.stdout_pipe, LOG_INFO); stdout_status = log_from_handle(child_handle->stdout_pipe, LOG_INFO);
stderr_status = log_from_handle(child_handle.stderr_pipe, LOG_WARN); stderr_status = log_from_handle(child_handle->stderr_pipe, LOG_WARN);
/* If we got this far (on Windows), the process started */ /* If we got this far (on Windows), the process started */
retval = 0; retval = 0;
#else #else
stdout_status = log_from_pipe(child_handle.stdout_handle, stdout_status = log_from_pipe(child_handle->stdout_handle,
LOG_INFO, filename, &retval); LOG_INFO, filename, &retval);
stderr_status = log_from_pipe(child_handle.stderr_handle, stderr_status = log_from_pipe(child_handle->stderr_handle,
LOG_WARN, filename, &retval); LOG_WARN, filename, &retval);
#endif #endif
if (retval) { if (retval) {
@ -4079,7 +4120,7 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port,
/* There was a failure */ /* There was a failure */
retval = -1; retval = -1;
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
else if (tor_get_exit_code(child_handle, 0, NULL) != else if (!child_handle || tor_get_exit_code(child_handle, 0, NULL) !=
PROCESS_EXIT_RUNNING) { PROCESS_EXIT_RUNNING) {
/* process has exited or there was an error */ /* process has exited or there was an error */
/* TODO: Do something with the process return value */ /* TODO: Do something with the process return value */
@ -4102,10 +4143,10 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port,
if (0 != retval) { if (0 != retval) {
if (1 == retval) { if (1 == retval) {
log_info(LD_GENERAL, "Port forwarding helper terminated"); log_info(LD_GENERAL, "Port forwarding helper terminated");
child_handle.status = PROCESS_STATUS_NOTRUNNING; child_handle->status = PROCESS_STATUS_NOTRUNNING;
} else { } else {
log_warn(LD_GENERAL, "Failed to read from port forwarding helper"); log_warn(LD_GENERAL, "Failed to read from port forwarding helper");
child_handle.status = PROCESS_STATUS_ERROR; child_handle->status = PROCESS_STATUS_ERROR;
} }
/* TODO: The child might not actually be finished (maybe it failed or /* TODO: The child might not actually be finished (maybe it failed or

View File

@ -356,14 +356,14 @@ void write_pidfile(char *filename);
void tor_check_port_forwarding(const char *filename, void tor_check_port_forwarding(const char *filename,
int dir_port, int or_port, time_t now); int dir_port, int or_port, time_t now);
typedef struct process_handle_s process_handle_t; typedef struct process_handle_t process_handle_t;
int tor_spawn_background(const char *const filename, const char **argv, int tor_spawn_background(const char *const filename, const char **argv,
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
LPVOID envp, LPVOID envp,
#else #else
const char **envp, const char **envp,
#endif #endif
process_handle_t *process_handle); process_handle_t **process_handle_out);
#define SPAWN_ERROR_MESSAGE "ERR: Failed to spawn background process - code " #define SPAWN_ERROR_MESSAGE "ERR: Failed to spawn background process - code "
@ -377,7 +377,10 @@ HANDLE load_windows_system_library(const TCHAR *library_name);
#define PROCESS_STATUS_NOTRUNNING 0 #define PROCESS_STATUS_NOTRUNNING 0
#define PROCESS_STATUS_RUNNING 1 #define PROCESS_STATUS_RUNNING 1
#define PROCESS_STATUS_ERROR -1 #define PROCESS_STATUS_ERROR -1
struct process_handle_s {
#ifdef UTIL_PRIVATE
/*DOCDOC*/
struct process_handle_t {
int status; int status;
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
HANDLE stdout_pipe; HANDLE stdout_pipe;
@ -391,12 +394,13 @@ struct process_handle_s {
pid_t pid; pid_t pid;
#endif // MS_WINDOWS #endif // MS_WINDOWS
}; };
#endif
/* Return values of tor_get_exit_code() */ /* Return values of tor_get_exit_code() */
#define PROCESS_EXIT_RUNNING 1 #define PROCESS_EXIT_RUNNING 1
#define PROCESS_EXIT_EXITED 0 #define PROCESS_EXIT_EXITED 0
#define PROCESS_EXIT_ERROR -1 #define PROCESS_EXIT_ERROR -1
int tor_get_exit_code(const process_handle_t process_handle, int tor_get_exit_code(const process_handle_t *process_handle,
int block, int *exit_code); int block, int *exit_code);
int tor_split_lines(struct smartlist_t *sl, char *buf, int len); int tor_split_lines(struct smartlist_t *sl, char *buf, int len);
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
@ -414,6 +418,11 @@ ssize_t tor_read_all_from_process_stderr(
char *tor_join_win_cmdline(const char *argv[]); char *tor_join_win_cmdline(const char *argv[]);
int tor_process_get_pid(process_handle_t *process_handle); int tor_process_get_pid(process_handle_t *process_handle);
#ifdef MS_WINDOWS
HANDLE tor_process_get_stdout_pipe(process_handle_t *process_handle);
#else
FILE *tor_process_get_stdout_pipe(process_handle_t *process_handle);
#endif
int tor_terminate_process(process_handle_t *process_handle); int tor_terminate_process(process_handle_t *process_handle);
void tor_process_handle_destroy(process_handle_t *process_handle, void tor_process_handle_destroy(process_handle_t *process_handle,

View File

@ -239,9 +239,7 @@ proxy_prepare_for_restart(managed_proxy_t *mp)
/* destroy the process handle and terminate the process. */ /* destroy the process handle and terminate the process. */
tor_process_handle_destroy(mp->process_handle, 1); tor_process_handle_destroy(mp->process_handle, 1);
mp->process_handle = NULL;
/* create process handle for the upcoming new process. */
mp->process_handle = tor_malloc_zero(sizeof(process_handle_t));
/* destroy all its old transports. we no longer use them. */ /* destroy all its old transports. we no longer use them. */
SMARTLIST_FOREACH_BEGIN(mp->transports, const char *, t_name) { SMARTLIST_FOREACH_BEGIN(mp->transports, const char *, t_name) {
@ -274,7 +272,7 @@ launch_managed_proxy(managed_proxy_t *mp)
/* Passing NULL as lpApplicationName makes Windows search for the .exe */ /* Passing NULL as lpApplicationName makes Windows search for the .exe */
retval = tor_spawn_background(NULL, (const char **)mp->argv, envp, retval = tor_spawn_background(NULL, (const char **)mp->argv, envp,
mp->process_handle); &mp->process_handle);
tor_free(envp); tor_free(envp);
@ -291,7 +289,7 @@ launch_managed_proxy(managed_proxy_t *mp)
} }
retval = tor_spawn_background(mp->argv[0], (const char **)mp->argv, retval = tor_spawn_background(mp->argv[0], (const char **)mp->argv,
(const char **)envp, mp->process_handle); (const char **)envp, &mp->process_handle);
/* free the memory allocated by set_managed_proxy_environment(). */ /* free the memory allocated by set_managed_proxy_environment(). */
free_execve_args(envp); free_execve_args(envp);
@ -371,8 +369,9 @@ configure_proxy(managed_proxy_t *mp)
} }
tor_assert(mp->conf_state != PT_PROTO_INFANT); tor_assert(mp->conf_state != PT_PROTO_INFANT);
tor_assert(mp->process_handle);
pos = tor_read_all_handle(mp->process_handle->stdout_pipe, pos = tor_read_all_handle(tor_process_get_stdout_pipe(mp->process_handle),
stdout_buf, sizeof(stdout_buf) - 1, NULL); stdout_buf, sizeof(stdout_buf) - 1, NULL);
if (pos < 0) { if (pos < 0) {
log_notice(LD_GENERAL, "Failed to read data from managed proxy"); log_notice(LD_GENERAL, "Failed to read data from managed proxy");
@ -426,9 +425,10 @@ configure_proxy(managed_proxy_t *mp)
} }
tor_assert(mp->conf_state != PT_PROTO_INFANT); tor_assert(mp->conf_state != PT_PROTO_INFANT);
tor_assert(mp->process_handle);
while (1) { while (1) {
r = get_string_from_pipe(mp->process_handle->stdout_handle, r = get_string_from_pipe(tor_process_get_stdout_pipe(mp->process_handle),
stdout_buf, sizeof(stdout_buf) - 1); stdout_buf, sizeof(stdout_buf) - 1);
if (r == IO_STREAM_OKAY) { /* got a line; handle it! */ if (r == IO_STREAM_OKAY) { /* got a line; handle it! */
@ -550,6 +550,7 @@ managed_proxy_destroy(managed_proxy_t *mp,
free_execve_args(mp->argv); free_execve_args(mp->argv);
tor_process_handle_destroy(mp->process_handle, also_terminate_process); tor_process_handle_destroy(mp->process_handle, also_terminate_process);
mp->process_handle = NULL;
tor_free(mp); tor_free(mp);
} }
@ -1119,8 +1120,6 @@ managed_proxy_create(const smartlist_t *transport_list,
SMARTLIST_FOREACH(transport_list, const char *, transport, SMARTLIST_FOREACH(transport_list, const char *, transport,
add_transport_to_proxy(transport, mp)); add_transport_to_proxy(transport, mp));
mp->process_handle = tor_malloc_zero(sizeof(process_handle_t));
/* register the managed proxy */ /* register the managed proxy */
if (!managed_proxy_list) if (!managed_proxy_list)
managed_proxy_list = smartlist_create(); managed_proxy_list = smartlist_create();

View File

@ -1388,27 +1388,29 @@ run_util_spawn_background(const char *argv[], const char *expected_out,
{ {
int retval, exit_code; int retval, exit_code;
ssize_t pos; ssize_t pos;
process_handle_t process_handle; process_handle_t *process_handle=NULL;
char stdout_buf[100], stderr_buf[100]; char stdout_buf[100], stderr_buf[100];
int status;
/* Start the program */ /* Start the program */
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
tor_spawn_background(NULL, argv, NULL, &process_handle); status = tor_spawn_background(NULL, argv, NULL, &process_handle);
#else #else
tor_spawn_background(argv[0], argv, NULL, &process_handle); status = tor_spawn_background(argv[0], argv, NULL, &process_handle);
#endif #endif
tt_int_op(process_handle.status, ==, expected_status); tt_int_op(status, ==, expected_status);
if (status == PROCESS_STATUS_ERROR)
/* If the process failed to start, don't bother continuing */
if (process_handle.status == PROCESS_STATUS_ERROR)
return; return;
tt_int_op(process_handle.stdout_pipe, >, 0); tt_assert(process_handle != NULL);
tt_int_op(process_handle.stderr_pipe, >, 0); tt_int_op(process_handle->status, ==, expected_status);
tt_int_op(process_handle->stdout_pipe, >, 0);
tt_int_op(process_handle->stderr_pipe, >, 0);
/* Check stdout */ /* Check stdout */
pos = tor_read_all_from_process_stdout(&process_handle, stdout_buf, pos = tor_read_all_from_process_stdout(process_handle, stdout_buf,
sizeof(stdout_buf) - 1); sizeof(stdout_buf) - 1);
tt_assert(pos >= 0); tt_assert(pos >= 0);
stdout_buf[pos] = '\0'; stdout_buf[pos] = '\0';
@ -1422,7 +1424,7 @@ run_util_spawn_background(const char *argv[], const char *expected_out,
// TODO: Make test-child exit with something other than 0 // TODO: Make test-child exit with something other than 0
/* Check stderr */ /* Check stderr */
pos = tor_read_all_from_process_stderr(&process_handle, stderr_buf, pos = tor_read_all_from_process_stderr(process_handle, stderr_buf,
sizeof(stderr_buf) - 1); sizeof(stderr_buf) - 1);
tt_assert(pos >= 0); tt_assert(pos >= 0);
stderr_buf[pos] = '\0'; stderr_buf[pos] = '\0';
@ -1430,7 +1432,8 @@ run_util_spawn_background(const char *argv[], const char *expected_out,
tt_int_op(pos, ==, strlen(expected_err)); tt_int_op(pos, ==, strlen(expected_err));
done: done:
; if (process_handle)
tor_process_handle_destroy(process_handle, 1);
} }
/** Check that we can launch a process and read the output */ /** Check that we can launch a process and read the output */
@ -1489,7 +1492,8 @@ test_util_spawn_background_partial_read(void *ptr)
int retval, exit_code; int retval, exit_code;
ssize_t pos = -1; ssize_t pos = -1;
process_handle_t process_handle; process_handle_t *process_handle=NULL;
int status;
char stdout_buf[100], stderr_buf[100]; char stdout_buf[100], stderr_buf[100];
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
const char *argv[] = {"test-child.exe", "--test", NULL}; const char *argv[] = {"test-child.exe", "--test", NULL};
@ -1510,21 +1514,23 @@ test_util_spawn_background_partial_read(void *ptr)
/* Start the program */ /* Start the program */
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
tor_spawn_background(NULL, argv, NULL, &process_handle); status = tor_spawn_background(NULL, argv, NULL, &process_handle);
#else #else
tor_spawn_background(argv[0], argv, NULL, &process_handle); status = tor_spawn_background(argv[0], argv, NULL, &process_handle);
#endif #endif
tt_int_op(process_handle.status, ==, expected_status); tt_int_op(status, ==, expected_status);
tt_assert(process_handle);
tt_int_op(process_handle->status, ==, expected_status);
/* Check stdout */ /* Check stdout */
for (expected_out_ctr =0; expected_out[expected_out_ctr] != NULL;) { for (expected_out_ctr = 0; expected_out[expected_out_ctr] != NULL;) {
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
pos = tor_read_all_handle(process_handle.stdout_pipe, stdout_buf, pos = tor_read_all_handle(process_handle->stdout_pipe, stdout_buf,
sizeof(stdout_buf) - 1, NULL); sizeof(stdout_buf) - 1, NULL);
#else #else
/* Check that we didn't read the end of file last time */ /* Check that we didn't read the end of file last time */
tt_assert(!eof); tt_assert(!eof);
pos = tor_read_all_handle(process_handle.stdout_handle, stdout_buf, pos = tor_read_all_handle(process_handle->stdout_handle, stdout_buf,
sizeof(stdout_buf) - 1, NULL, &eof); sizeof(stdout_buf) - 1, NULL, &eof);
#endif #endif
log_info(LD_GENERAL, "tor_read_all_handle() returned %d", (int)pos); log_info(LD_GENERAL, "tor_read_all_handle() returned %d", (int)pos);
@ -1542,16 +1548,16 @@ test_util_spawn_background_partial_read(void *ptr)
/* The process should have exited without writing more */ /* The process should have exited without writing more */
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
pos = tor_read_all_handle(process_handle.stdout_pipe, stdout_buf, pos = tor_read_all_handle(process_handle->stdout_pipe, stdout_buf,
sizeof(stdout_buf) - 1, sizeof(stdout_buf) - 1,
&process_handle); process_handle);
tt_int_op(pos, ==, 0); tt_int_op(pos, ==, 0);
#else #else
if (!eof) { if (!eof) {
/* We should have got all the data, but maybe not the EOF flag */ /* We should have got all the data, but maybe not the EOF flag */
pos = tor_read_all_handle(process_handle.stdout_handle, stdout_buf, pos = tor_read_all_handle(process_handle->stdout_handle, stdout_buf,
sizeof(stdout_buf) - 1, sizeof(stdout_buf) - 1,
&process_handle, &eof); process_handle, &eof);
tt_int_op(pos, ==, 0); tt_int_op(pos, ==, 0);
tt_assert(eof); tt_assert(eof);
} }
@ -1566,7 +1572,7 @@ test_util_spawn_background_partial_read(void *ptr)
// TODO: Make test-child exit with something other than 0 // TODO: Make test-child exit with something other than 0
/* Check stderr */ /* Check stderr */
pos = tor_read_all_from_process_stderr(&process_handle, stderr_buf, pos = tor_read_all_from_process_stderr(process_handle, stderr_buf,
sizeof(stderr_buf) - 1); sizeof(stderr_buf) - 1);
tt_assert(pos >= 0); tt_assert(pos >= 0);
stderr_buf[pos] = '\0'; stderr_buf[pos] = '\0';
@ -1574,7 +1580,7 @@ test_util_spawn_background_partial_read(void *ptr)
tt_int_op(pos, ==, strlen(expected_err)); tt_int_op(pos, ==, strlen(expected_err));
done: done:
; tor_process_handle_destroy(process_handle, 1);
} }
/** /**