From 4ed03965a5a412bf58540e678f48f6c331ad30d9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 9 Apr 2014 13:45:27 -0400 Subject: New waitpid-handler functions to run callbacks when a child exits. Also, move 'procmon' into libor_event library, since it uses libevent. --- src/common/include.am | 9 ++- src/common/procmon.c | 1 + src/common/util_process.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++ src/common/util_process.h | 25 ++++++++ 4 files changed, 189 insertions(+), 3 deletions(-) create mode 100644 src/common/util_process.c create mode 100644 src/common/util_process.h (limited to 'src/common') diff --git a/src/common/include.am b/src/common/include.am index 7b2465cd8..c9dcedd52 100644 --- a/src/common/include.am +++ b/src/common/include.am @@ -57,9 +57,9 @@ LIBOR_A_SOURCES = \ src/common/log.c \ src/common/memarea.c \ src/common/mempool.c \ - src/common/procmon.c \ src/common/util.c \ src/common/util_codedigest.c \ + src/common/util_process.c \ src/common/sandbox.c \ src/ext/csiphash.c \ $(libor_extra_source) @@ -72,7 +72,9 @@ LIBOR_CRYPTO_A_SOURCES = \ src/common/tortls.c \ $(libcrypto_extra_source) -LIBOR_EVENT_A_SOURCES = src/common/compat_libevent.c +LIBOR_EVENT_A_SOURCES = \ + src/common/compat_libevent.c \ + src/common/procmon.c src_common_libor_a_SOURCES = $(LIBOR_A_SOURCES) src_common_libor_crypto_a_SOURCES = $(LIBOR_CRYPTO_A_SOURCES) @@ -111,7 +113,8 @@ COMMONHEADERS = \ src/common/torint.h \ src/common/torlog.h \ src/common/tortls.h \ - src/common/util.h + src/common/util.h \ + src/common/util_process.h noinst_HEADERS+= $(COMMONHEADERS) diff --git a/src/common/procmon.c b/src/common/procmon.c index 0a49689e3..7c9b7c3c8 100644 --- a/src/common/procmon.c +++ b/src/common/procmon.c @@ -162,6 +162,7 @@ tor_validate_process_specifier(const char *process_spec, return parse_process_specifier(process_spec, &ppspec, msg); } +/* XXXX we should use periodic_timer_new() for this stuff */ #ifdef HAVE_EVENT2_EVENT_H #define PERIODIC_TIMER_FLAGS EV_PERSIST #else diff --git a/src/common/util_process.c b/src/common/util_process.c new file mode 100644 index 000000000..8ccf7f38f --- /dev/null +++ b/src/common/util_process.c @@ -0,0 +1,157 @@ +/* Copyright (c) 2003-2004, Roger Dingledine + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2013, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file util_process.c + * \brief utility functions for launching processes and checking their + * status. These functions are kept separately from procmon so that they + * won't require linking against libevent. + **/ + +#include "orconfig.h" + +#ifdef HAVE_SYS_TYPES_H +#include +#endif +#ifdef HAVE_SYS_WAIT_H +#include +#endif + +#include "compat.h" +#include "util.h" +#include "torlog.h" +#include "util_process.h" +#include "ht.h" + +/* ================================================== */ +/* Convenience structures for handlers for waitpid(). + * + * The tor_process_monitor*() code above doesn't use them, since it is for + * monitoring a non-child process. + */ + +#ifndef _WIN32 + +/** Mapping from a PID to a userfn/userdata pair. */ +struct waitpid_callback_t { + HT_ENTRY(waitpid_callback_t) node; + pid_t pid; + + void (*userfn)(int, void *userdata); + void *userdata; + + unsigned running; +}; + +static INLINE unsigned int +process_map_entry_hash_(const waitpid_callback_t *ent) +{ + return (unsigned) ent->pid; +} + +static INLINE unsigned int +process_map_entries_eq_(const waitpid_callback_t *a, const waitpid_callback_t *b) +{ + return a->pid == b->pid; +} + +static HT_HEAD(process_map, waitpid_callback_t) process_map = HT_INITIALIZER(); + +HT_PROTOTYPE(process_map, waitpid_callback_t, node, process_map_entry_hash_, + process_map_entries_eq_); +HT_GENERATE(process_map, waitpid_callback_t, node, process_map_entry_hash_, + process_map_entries_eq_, 0.6, malloc, realloc, free); + +/** + * Begin monitoring the child pid pid to see if we get a SIGCHLD for + * it. If we eventually do, call fn, passing it the exit status (as + * yielded by waitpid) and the pointer arg. + * + * To cancel this, or clean up after it has triggered, call + * clear_waitpid_callback(). + */ +waitpid_callback_t * +set_waitpid_callback(pid_t pid, void (*fn)(int, void *), void *arg) +{ + waitpid_callback_t *old_ent; + waitpid_callback_t *ent = tor_malloc_zero(sizeof(waitpid_callback_t)); + ent->pid = pid; + ent->userfn = fn; + ent->userdata = arg; + ent->running = 1; + + old_ent = HT_REPLACE(process_map, &process_map, ent); + if (old_ent) { + log_warn(LD_BUG, "Replaced a waitpid monitor on pid %u. That should be " + "impossible.", (unsigned) pid); + old_ent->running = 0; + } + + return ent; +} + +/** + * Cancel a waitpid_callback_t, or clean up after one has triggered. Releases + * all storage held by ent. + */ +void +clear_waitpid_callback(waitpid_callback_t *ent) +{ + waitpid_callback_t *old_ent; + if (ent == NULL) + return; + + if (ent->running) { + old_ent = HT_REMOVE(process_map, &process_map, ent); + if (old_ent != ent) { + log_warn(LD_BUG, "Couldn't remove waitpid monitor for pid %u.", + (unsigned) ent->pid); + return; + } + } + + tor_free(ent); +} + +/** Helper: find the callack for pid; if there is one, run it, + * reporting the exit status as status. */ +static void +notify_waitpid_callback_by_pid(pid_t pid, int status) +{ + waitpid_callback_t search, *ent; + + search.pid = pid; + ent = HT_REMOVE(process_map, &process_map, &search); + if (!ent || !ent->running) { + log_info(LD_GENERAL, "Child process %u has exited; no callback was " + "registered", (unsigned)pid); + return; + } + + log_info(LD_GENERAL, "Child process %u has exited; running callback.", + (unsigned)pid); + + ent->running = 0; + ent->userfn(status, ent->userdata); +} + +/** Use waitpid() to wait for all children that have exited, and invoke any + * callbacks registered for them. */ +void +notify_pending_waitpid_callbacks(void) +{ + /* I was going to call this function reap_zombie_children(), but + * that makes it sound way more exciting than it really is. */ + pid_t child; + int status = 0; + + while ((child = waitpid(-1, &status, WNOHANG)) > 0) { + notify_waitpid_callback_by_pid(child, status); + status = 0; /* should be needless */ + } +} + +#endif + diff --git a/src/common/util_process.h b/src/common/util_process.h new file mode 100644 index 000000000..877702c68 --- /dev/null +++ b/src/common/util_process.h @@ -0,0 +1,25 @@ +/* Copyright (c) 2011-2013, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file util_process.h + * \brief Headers for util_process.c + **/ + +#ifndef TOR_UTIL_PROCESS_H +#define TOR_UTIL_PROCESS_H + +#ifndef _WIN32 +/** A callback structure waiting for us to get a SIGCHLD informing us that a + * PID has been closed. Created by set_waitpid_callback. Cancelled or cleaned- + * up from clear_waitpid_callback(). Do not access outside of the main thread; + * do not access from inside a signal handler. */ +typedef struct waitpid_callback_t waitpid_callback_t; + +waitpid_callback_t *set_waitpid_callback(pid_t pid, + void (*fn)(int, void *), void *arg); +void clear_waitpid_callback(waitpid_callback_t *ent); +void notify_pending_waitpid_callbacks(void); +#endif + +#endif -- cgit v1.2.3 From f8344c2d28be2489c8abadd694b5b96fe18efc02 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 10 Apr 2014 11:06:10 -0400 Subject: Use waitpid code to learn when a controlled process dies This lets us avoid sending SIGTERM to something that has already died, since we realize it has already died, and is a fix for the unix version of #8746. --- src/common/util.c | 46 +++++++++++++++++++++++++++++++++++++++++++--- src/common/util.h | 9 ++++++++- 2 files changed, 51 insertions(+), 4 deletions(-) (limited to 'src/common') diff --git a/src/common/util.c b/src/common/util.c index 56235aa66..0a1410139 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -26,6 +26,7 @@ #include "address.h" #include "sandbox.h" #include "backtrace.h" +#include "util_process.h" #ifdef _WIN32 #include @@ -3642,7 +3643,10 @@ tor_terminate_process(process_handle_t *process_handle) return 0; } #else /* Unix */ - return kill(process_handle->pid, SIGTERM); + if (process_handle->waitpid_cb) { + /* We haven't got a waitpid yet, so we can just kill off the process. */ + return kill(process_handle->pid, SIGTERM); + } #endif return -1; @@ -3691,6 +3695,23 @@ process_handle_new(void) return out; } +#ifndef _WIN32 +/** Invoked when a process that we've launched via tor_spawn_background() has + * been found to have terminated. + */ +static void +process_handle_waitpid_cb(int status, void *arg) +{ + process_handle_t *process_handle = arg; + + process_handle->waitpid_exit_status = status; + clear_waitpid_callback(process_handle->waitpid_cb); + if (process_handle->status == PROCESS_STATUS_RUNNING) + process_handle->status = PROCESS_STATUS_NOTRUNNING; + process_handle->waitpid_cb = 0; +} +#endif + /** * @name child-process states * @@ -4007,6 +4028,10 @@ tor_spawn_background(const char *const filename, const char **argv, strerror(errno)); } + process_handle->waitpid_cb = set_waitpid_callback(pid, + process_handle_waitpid_cb, + process_handle); + process_handle->stderr_pipe = stderr_pipe[0]; retval = close(stderr_pipe[1]); @@ -4071,6 +4096,8 @@ tor_process_handle_destroy,(process_handle_t *process_handle, if (process_handle->stderr_handle) fclose(process_handle->stderr_handle); + + clear_waitpid_callback(process_handle->waitpid_cb); #endif memset(process_handle, 0x0f, sizeof(process_handle_t)); @@ -4088,7 +4115,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 * terminated child processes.*/ int -tor_get_exit_code(const process_handle_t *process_handle, +tor_get_exit_code(process_handle_t *process_handle, int block, int *exit_code) { #ifdef _WIN32 @@ -4128,7 +4155,20 @@ tor_get_exit_code(const process_handle_t *process_handle, int stat_loc; int retval; - retval = waitpid(process_handle->pid, &stat_loc, block?0:WNOHANG); + if (process_handle->waitpid_cb) { + /* We haven't processed a SIGCHLD yet. */ + retval = waitpid(process_handle->pid, &stat_loc, block?0:WNOHANG); + if (retval == process_handle->pid) { + clear_waitpid_callback(process_handle->waitpid_cb); + process_handle->waitpid_cb = NULL; + process_handle->waitpid_exit_status = stat_loc; + } + } else { + /* We already got a SIGCHLD for this process, and handled it. */ + retval = process_handle->pid; + stat_loc = process_handle->waitpid_exit_status; + } + if (!block && 0 == retval) { /* Process has not exited */ return PROCESS_EXIT_RUNNING; diff --git a/src/common/util.h b/src/common/util.h index 18dc20639..97367a9a7 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -446,6 +446,7 @@ void set_environment_variable_in_smartlist(struct smartlist_t *env_vars, #define PROCESS_STATUS_ERROR -1 #ifdef UTIL_PRIVATE +struct waitpid_callback_t; /** Structure to represent the state of a process with which Tor is * communicating. The contents of this structure are private to util.c */ struct process_handle_t { @@ -461,6 +462,12 @@ struct process_handle_t { FILE *stdout_handle; FILE *stderr_handle; pid_t pid; + /** If the process has not given us a SIGCHLD yet, this has the + * waitpid_callback_t that gets invoked once it has. Otherwise this + * contains NULL. */ + struct waitpid_callback_t *waitpid_cb; + /** The exit status reported by waitpid. */ + int waitpid_exit_status; #endif // _WIN32 }; #endif @@ -469,7 +476,7 @@ struct process_handle_t { #define PROCESS_EXIT_RUNNING 1 #define PROCESS_EXIT_EXITED 0 #define PROCESS_EXIT_ERROR -1 -int tor_get_exit_code(const process_handle_t *process_handle, +int tor_get_exit_code(process_handle_t *process_handle, int block, int *exit_code); int tor_split_lines(struct smartlist_t *sl, char *buf, int len); #ifdef _WIN32 -- cgit v1.2.3 From 34f8723dc784142b30d92bbbdeb37089ae7a3bc5 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 10 Apr 2014 11:16:42 -0400 Subject: On Windows, terminate processes by handle, not pid When we create a process yourself with CreateProcess, we get a handle to the process in the PROCESS_INFO output structure. But instead of using that handle, we were manually looking up a _new_ handle based on the process ID, which is a poor idea, since the process ID might refer to a new process later on, but the handle can't. --- src/common/util.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'src/common') diff --git a/src/common/util.c b/src/common/util.c index 0a1410139..f79e72bea 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3629,13 +3629,7 @@ tor_terminate_process(process_handle_t *process_handle) { #ifdef _WIN32 if (tor_get_exit_code(process_handle, 0, NULL) == PROCESS_EXIT_RUNNING) { - HANDLE handle; - /* If the signal is outside of what GenerateConsoleCtrlEvent can use, - attempt to open and terminate the process. */ - handle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, - process_handle->pid.dwProcessId); - if (!handle) - return -1; + HANDLE handle = process_handle->pid.hProcess; if (!TerminateProcess(handle, 0)) return -1; -- cgit v1.2.3 From e2e588175eac4ebe8fb47c0540954d0f78525cce Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 30 Apr 2014 12:48:46 -0400 Subject: New testing-only tor_sleep_msec function In the unit tests I want to loop with a delay, but I want less than a 1 second delay. This, sadly, requires compatibility code. --- configure.ac | 2 ++ src/common/compat.c | 26 ++++++++++++++++++++++++++ src/common/compat.h | 4 ++++ 3 files changed, 32 insertions(+) (limited to 'src/common') diff --git a/configure.ac b/configure.ac index c0c818757..c662a9fc1 100644 --- a/configure.ac +++ b/configure.ac @@ -355,6 +355,7 @@ AC_CHECK_FUNCS( sysconf \ sysctl \ uname \ + usleep \ vasprintf \ _vscprintf ) @@ -885,6 +886,7 @@ AC_CHECK_HEADERS( sys/param.h \ sys/prctl.h \ sys/resource.h \ + sys/select.h \ sys/socket.h \ sys/sysctl.h \ sys/syslimits.h \ diff --git a/src/common/compat.c b/src/common/compat.c index 1ba264a0c..1c460b6ae 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -114,6 +114,12 @@ /* Only use the linux prctl; the IRIX prctl is totally different */ #include #endif +#ifdef TOR_UNIT_TESTS +#if !defined(HAVE_USLEEP) && defined(HAVE_SYS_SELECT_H) +/* as fallback implementation for tor_sleep_msec */ +#include +#endif +#endif #include "torlog.h" #include "util.h" @@ -3450,3 +3456,23 @@ get_total_system_memory(size_t *mem_out) return -1; } +#ifdef TOR_UNIT_TESTS +/** Delay for msec milliseconds. Only used in tests. */ +void +tor_sleep_msec(int msec) +{ +#ifdef _WIN32 + Sleep(msec); +#elif defined(HAVE_USLEEP) + sleep(msec / 1000); + /* Some usleep()s hate sleeping more than 1 sec */ + usleep((msec % 1000) * 1000); +#elif defined(HAVE_SYS_SELECT_H) + struct timeval tv = { msec / 1000, (msec % 1000) * 1000}; + select(0, NULL, NULL, NULL, &tv); +#else + sleep(CEIL_DIV(msec, 1000)); +#endif +} +#endif + diff --git a/src/common/compat.h b/src/common/compat.h index 314b1aa00..d723448fd 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -744,6 +744,10 @@ char *format_win32_error(DWORD err); #endif +#ifdef TOR_UNIT_TESTS +void tor_sleep_msec(int msec); +#endif + #ifdef COMPAT_PRIVATE #if !defined(HAVE_SOCKETPAIR) || defined(_WIN32) || defined(TOR_UNIT_TESTS) #define NEED_ERSATZ_SOCKETPAIR -- cgit v1.2.3