June 2014 - tor-commits - lists.torproject.org

[tor/master] More unit tests for process spawning
by nickm＠torproject.org 14 Jun '14

14 Jun '14

commit e3833193af11dc4252e90565c3919889cc4130b3 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Apr 30 12:50:00 2014 -0400 More unit tests for process spawning Try killing a running process; try noticing that a process has exited without checking its output; verify that waitpid_cb (when present) is set to NULL when you would expect it to be. --- src/test/test-child.c | 40 +++++++++++----- src/test/test_util.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 146 insertions(+), 18 deletions(-) diff --git a/src/test/test-child.c b/src/test/test-child.c index ef10fbb..756782e 100644 --- a/src/test/test-child.c +++ b/src/test/test-child.c @@ -9,6 +9,13 @@ #else #include <unistd.h> #endif +#include <string.h> + +#ifdef _WIN32 +#define SLEEP(sec) Sleep((sec)*1000) +#else +#define SLEEP(sec) sleep(sec) +#endif /** Trivial test program which prints out its command line arguments so we can * check if tor_spawn_background() works */ @@ -16,27 +23,38 @@ int main(int argc, char **argv) { int i; + int delay = 1; + int fast = 0; + + if (argc > 1) { + if (!strcmp(argv[1], "--hang")) { + delay = 60; + } else if (!strcmp(argv[1], "--fast")) { + fast = 1; + delay = 0; + } + } fprintf(stdout, "OUT\n"); fprintf(stderr, "ERR\n"); for (i = 1; i < argc; i++) fprintf(stdout, "%s\n", argv[i]); - fprintf(stdout, "SLEEPING\n"); + if (!fast) + fprintf(stdout, "SLEEPING\n"); /* We need to flush stdout so that test_util_spawn_background_partial_read() succeed. Otherwise ReadFile() will get the entire output in one */ // XXX: Can we make stdio flush on newline? fflush(stdout); -#ifdef _WIN32 - Sleep(1000); -#else - sleep(1); -#endif + if (!fast) + SLEEP(1); fprintf(stdout, "DONE\n"); -#ifdef _WIN32 - Sleep(1000); -#else - sleep(1); -#endif + fflush(stdout); + if (fast) + return 0; + + while (--delay) { + SLEEP(1); + } return 0; } diff --git a/src/test/test_util.c b/src/test/test_util.c index 6d6b6db..029e88e 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -14,6 +14,7 @@ #include "test.h" #include "mempool.h" #include "memarea.h" +#include "util_process.h" #ifdef _WIN32 #include <tchar.h> @@ -2529,6 +2530,10 @@ test_util_fgets_eagain(void *ptr) } #endif +#ifdef _WIN32 +#define notify_pending_waitpid_callbacks() STMT_NIL +#endif + /** Helper function for testing tor_spawn_background */ static void run_util_spawn_background(const char *argv[], const char *expected_out, @@ -2548,19 +2553,28 @@ run_util_spawn_background(const char *argv[], const char *expected_out, status = tor_spawn_background(argv[0], argv, NULL, &process_handle); #endif + notify_pending_waitpid_callbacks(); + test_eq(expected_status, status); - if (status == PROCESS_STATUS_ERROR) + if (status == PROCESS_STATUS_ERROR) { + tt_ptr_op(process_handle, ==, NULL); return; + } test_assert(process_handle != NULL); test_eq(expected_status, process_handle->status); +#ifndef _WIN32 + notify_pending_waitpid_callbacks(); + tt_ptr_op(process_handle->waitpid_cb, !=, NULL); +#endif + #ifdef _WIN32 test_assert(process_handle->stdout_pipe != INVALID_HANDLE_VALUE); test_assert(process_handle->stderr_pipe != INVALID_HANDLE_VALUE); #else - test_assert(process_handle->stdout_pipe > 0); - test_assert(process_handle->stderr_pipe > 0); + test_assert(process_handle->stdout_pipe >= 0); + test_assert(process_handle->stderr_pipe >= 0); #endif /* Check stdout */ @@ -2571,12 +2585,19 @@ run_util_spawn_background(const char *argv[], const char *expected_out, test_eq(strlen(expected_out), pos); test_streq(expected_out, stdout_buf); + notify_pending_waitpid_callbacks(); + /* Check it terminated correctly */ retval = tor_get_exit_code(process_handle, 1, &exit_code); test_eq(PROCESS_EXIT_EXITED, retval); test_eq(expected_exit, exit_code); // TODO: Make test-child exit with something other than 0 +#ifndef _WIN32 + notify_pending_waitpid_callbacks(); + tt_ptr_op(process_handle->waitpid_cb, ==, NULL); +#endif + /* Check stderr */ pos = tor_read_all_from_process_stderr(process_handle, stderr_buf, sizeof(stderr_buf) - 1); @@ -2585,6 +2606,8 @@ run_util_spawn_background(const char *argv[], const char *expected_out, test_streq(expected_err, stderr_buf); test_eq(strlen(expected_err), pos); + notify_pending_waitpid_callbacks(); + done: if (process_handle) tor_process_handle_destroy(process_handle, 1); @@ -2607,7 +2630,7 @@ test_util_spawn_background_ok(void *ptr) (void)ptr; run_util_spawn_background(argv, expected_out, expected_err, 0, - PROCESS_STATUS_RUNNING); + PROCESS_STATUS_RUNNING); } /** Check that failing to find the executable works as expected */ @@ -2637,13 +2660,13 @@ test_util_spawn_background_fail(void *ptr) "ERR: Failed to spawn background process - code %s\n", code); run_util_spawn_background(argv, expected_out, expected_err, 255, - expected_status); + expected_status); } /** Test that reading from a handle returns a partial read rather than * blocking */ static void -test_util_spawn_background_partial_read(void *ptr) +test_util_spawn_background_partial_read_impl(int exit_early) { const int expected_exit = 0; const int expected_status = PROCESS_STATUS_RUNNING; @@ -2668,7 +2691,16 @@ test_util_spawn_background_partial_read(void *ptr) int eof = 0; #endif int expected_out_ctr; - (void)ptr; + + + if (exit_early) { + argv[1] = "--hang"; +#ifdef _WIN32 + expected_out[0] = "OUT\r\n--hang\r\nSLEEPING\r\n"; +#else + expected_out[0] = "OUT\n--hang\nSLEEPING\n"; +#endif + } /* Start the program */ #ifdef _WIN32 @@ -2704,6 +2736,12 @@ test_util_spawn_background_partial_read(void *ptr) expected_out_ctr++; } + if (exit_early) { + tor_process_handle_destroy(process_handle, 1); + process_handle = NULL; + goto done; + } + /* The process should have exited without writing more */ #ifdef _WIN32 pos = tor_read_all_handle(process_handle->stdout_pipe, stdout_buf, @@ -2741,6 +2779,76 @@ test_util_spawn_background_partial_read(void *ptr) tor_process_handle_destroy(process_handle, 1); } +static void +test_util_spawn_background_partial_read(void *arg) +{ + (void)arg; + test_util_spawn_background_partial_read_impl(0); +} + +static void +test_util_spawn_background_exit_early(void *arg) +{ + (void)arg; + test_util_spawn_background_partial_read_impl(1); +} + +static void +test_util_spawn_background_waitpid_notify(void *arg) +{ + int retval, exit_code; + process_handle_t *process_handle=NULL; + int status; + int ms_timer; + +#ifdef _WIN32 + const char *argv[] = {"test-child.exe", "--fast", NULL}; +#else + const char *argv[] = {BUILDDIR "/src/test/test-child", "--fast", NULL}; +#endif + + (void) arg; + +#ifdef _WIN32 + status = tor_spawn_background(NULL, argv, NULL, &process_handle); +#else + status = tor_spawn_background(argv[0], argv, NULL, &process_handle); +#endif + + tt_int_op(status, ==, PROCESS_STATUS_RUNNING); + tt_ptr_op(process_handle, !=, NULL); + + /* We're not going to look at the stdout/stderr output this time. Instead, + * we're testing whether notify_pending_waitpid_calbacks() can report the + * process exit (on unix) and/or whether tor_get_exit_code() can notice it + * (on windows) */ + +#ifndef _WIN32 + ms_timer = 30*1000; + tt_ptr_op(process_handle->waitpid_cb, !=, NULL); + while (process_handle->waitpid_cb && ms_timer > 0) { + tor_sleep_msec(100); + ms_timer -= 100; + notify_pending_waitpid_callbacks(); + } + tt_int_op(ms_timer, >, 0); + tt_ptr_op(process_handle->waitpid_cb, ==, NULL); +#endif + + ms_timer = 30*1000; + while (((retval = tor_get_exit_code(process_handle, 0, &exit_code)) + == PROCESS_EXIT_RUNNING) && ms_timer > 0) { + tor_sleep_msec(100); + ms_timer -= 100; + } + tt_int_op(ms_timer, >, 0); + + tt_int_op(retval, ==, PROCESS_EXIT_EXITED); + + done: + tor_process_handle_destroy(process_handle, 1); +} + /** * Test for format_hex_number_sigsafe() */ @@ -3682,6 +3790,8 @@ struct testcase_t util_tests[] = { UTIL_TEST(spawn_background_ok, 0), UTIL_TEST(spawn_background_fail, 0), UTIL_TEST(spawn_background_partial_read, 0), + UTIL_TEST(spawn_background_exit_early, 0), + UTIL_TEST(spawn_background_waitpid_notify, 0), UTIL_TEST(format_hex_number, 0), UTIL_TEST(format_dec_number, 0), UTIL_TEST(join_win_cmdline, 0),

1 0

[tor/master] Use waitpid code to learn when a controlled process dies
by nickm＠torproject.org 14 Jun '14

14 Jun '14

commit f8344c2d28be2489c8abadd694b5b96fe18efc02 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Apr 10 11:06:10 2014 -0400 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(-) diff --git a/src/common/util.c b/src/common/util.c index 56235aa..0a14101 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 <io.h> @@ -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 18dc206..97367a9 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

1 0

[tor/master] On Windows, terminate processes by handle, not pid
by nickm＠torproject.org 14 Jun '14

14 Jun '14

commit 34f8723dc784142b30d92bbbdeb37089ae7a3bc5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Apr 10 11:16:42 2014 -0400 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(-) diff --git a/src/common/util.c b/src/common/util.c index 0a14101..f79e72b 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;

1 0

[tor/master] Merge branch 'bug8746_v2_squashed'
by nickm＠torproject.org 14 Jun '14

14 Jun '14

commit a7cafb1ea9307864c94ad3e019af93d09d48350e Merge: a58d94f e07d328 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Jun 14 11:46:38 2014 -0400 Merge branch 'bug8746_v2_squashed' Conflicts: src/common/include.am changes/bug8746 | 4 ++ configure.ac | 2 + src/common/compat.c | 26 ++++++++ src/common/compat.h | 4 ++ src/common/include.am | 7 +- src/common/procmon.c | 1 + src/common/util.c | 54 ++++++++++++--- src/common/util.h | 9 ++- src/common/util_process.c | 157 ++++++++++++++++++++++++++++++++++++++++++++ src/common/util_process.h | 25 +++++++ src/or/main.c | 4 +- src/or/or.h | 3 - src/test/test-child.c | 40 ++++++++---- src/test/test_util.c | 160 ++++++++++++++++++++++++++++++++++++--------- 14 files changed, 437 insertions(+), 59 deletions(-) diff --cc src/common/compat.c index 111070c,1c460b6..e25ecc4 --- a/src/common/compat.c +++ b/src/common/compat.c @@@ -3553,6 -3453,26 +3559,26 @@@ get_total_system_memory(size_t *mem_out *mem_out = mem_cached = (size_t) m; - return -1; + return 0; } + #ifdef TOR_UNIT_TESTS + /** Delay for <b>msec</b> 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 --cc src/common/include.am index 61a90cd,c9dcedd..68e0110 --- a/src/common/include.am +++ b/src/common/include.am @@@ -64,13 -56,13 +64,13 @@@ LIBOR_A_SOURCES = src/common/di_ops.c \ src/common/log.c \ src/common/memarea.c \ - src/common/procmon.c \ - src/common/mempool.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) + $(libor_extra_source) \ + $(libor_mempool_source) LIBOR_CRYPTO_A_SOURCES = \ src/common/aes.c \ @@@ -119,7 -114,7 +121,8 @@@ COMMONHEADERS = src/common/torlog.h \ src/common/tortls.h \ src/common/util.h \ - src/common/util_process.h ++ src/common/util_process.h \ + $(libor_mempool_header) noinst_HEADERS+= $(COMMONHEADERS) diff --cc src/test/test_util.c index c7fa141,656b77b..151ec69 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@@ -12,10 -12,9 +12,11 @@@ #include "config.h" #include "control.h" #include "test.h" +#ifdef ENABLE_MEMPOOLS #include "mempool.h" +#endif /* ENABLE_MEMPOOLS */ #include "memarea.h" + #include "util_process.h" #ifdef _WIN32 #include <tchar.h>

1 0

[tor/master] refactor win/nix handling for test_spawn_background*()
by nickm＠torproject.org 14 Jun '14

14 Jun '14

commit a5c092b34b1505051071e05a54353225dd6cba5e Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Apr 30 13:00:54 2014 -0400 refactor win/nix handling for test_spawn_background*() Instead of having a #if ... for every function, just define TEST_CHILD to the right patch and EOL to the expected line terminator. --- src/test/test_util.c | 58 +++++++++++++++++++++----------------------------- 1 file changed, 24 insertions(+), 34 deletions(-) diff --git a/src/test/test_util.c b/src/test/test_util.c index 029e88e..656b77b 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -2530,8 +2530,17 @@ test_util_fgets_eagain(void *ptr) } #endif +#ifndef BUILDDIR +#define BUILDDIR "." +#endif + #ifdef _WIN32 #define notify_pending_waitpid_callbacks() STMT_NIL +#define TEST_CHILD "test-child.exe" +#define EOL "\r\n" +#else +#define TEST_CHILD (BUILDDIR "/src/test/test-child") +#define EOL "\n" #endif /** Helper function for testing tor_spawn_background */ @@ -2617,15 +2626,9 @@ run_util_spawn_background(const char *argv[], const char *expected_out, static void test_util_spawn_background_ok(void *ptr) { -#ifdef _WIN32 - const char *argv[] = {"test-child.exe", "--test", NULL}; - const char *expected_out = "OUT\r\n--test\r\nSLEEPING\r\nDONE\r\n"; - const char *expected_err = "ERR\r\n"; -#else - const char *argv[] = {BUILDDIR "/src/test/test-child", "--test", NULL}; - const char *expected_out = "OUT\n--test\nSLEEPING\nDONE\n"; - const char *expected_err = "ERR\n"; -#endif + const char *argv[] = {TEST_CHILD, "--test", NULL}; + const char *expected_out = "OUT"EOL "--test"EOL "SLEEPING"EOL "DONE" EOL; + const char *expected_err = "ERR"EOL; (void)ptr; @@ -2637,9 +2640,6 @@ test_util_spawn_background_ok(void *ptr) static void test_util_spawn_background_fail(void *ptr) { -#ifndef BUILDDIR -#define BUILDDIR "." -#endif const char *argv[] = {BUILDDIR "/src/test/no-such-file", "--test", NULL}; const char *expected_err = ""; char expected_out[1024]; @@ -2676,30 +2676,21 @@ test_util_spawn_background_partial_read_impl(int exit_early) process_handle_t *process_handle=NULL; int status; char stdout_buf[100], stderr_buf[100]; -#ifdef _WIN32 - const char *argv[] = {"test-child.exe", "--test", NULL}; - const char *expected_out[] = { "OUT\r\n--test\r\nSLEEPING\r\n", - "DONE\r\n", - NULL }; - const char *expected_err = "ERR\r\n"; -#else - const char *argv[] = {BUILDDIR "/src/test/test-child", "--test", NULL}; - const char *expected_out[] = { "OUT\n--test\nSLEEPING\n", - "DONE\n", + + const char *argv[] = {TEST_CHILD, "--test", NULL}; + const char *expected_out[] = { "OUT" EOL "--test" EOL "SLEEPING" EOL, + "DONE" EOL, NULL }; - const char *expected_err = "ERR\n"; + const char *expected_err = "ERR" EOL; + +#ifndef _WIN32 int eof = 0; #endif int expected_out_ctr; - if (exit_early) { argv[1] = "--hang"; -#ifdef _WIN32 - expected_out[0] = "OUT\r\n--hang\r\nSLEEPING\r\n"; -#else - expected_out[0] = "OUT\n--hang\nSLEEPING\n"; -#endif + expected_out[0] = "OUT"EOL "--hang"EOL "SLEEPING" EOL; } /* Start the program */ @@ -2801,11 +2792,7 @@ test_util_spawn_background_waitpid_notify(void *arg) int status; int ms_timer; -#ifdef _WIN32 - const char *argv[] = {"test-child.exe", "--fast", NULL}; -#else - const char *argv[] = {BUILDDIR "/src/test/test-child", "--fast", NULL}; -#endif + const char *argv[] = {TEST_CHILD, "--fast", NULL}; (void) arg; @@ -2849,6 +2836,9 @@ test_util_spawn_background_waitpid_notify(void *arg) tor_process_handle_destroy(process_handle, 1); } +#undef TEST_CHILD +#undef EOL + /** * Test for format_hex_number_sigsafe() */

1 0

[tor/master] changes file for 8746
by nickm＠torproject.org 14 Jun '14

14 Jun '14

commit e07d328457b26babe89abdcc1d5a550ee8273462 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Apr 30 13:13:38 2014 -0400 changes file for 8746 --- changes/bug8746 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/changes/bug8746 b/changes/bug8746 new file mode 100644 index 0000000..b6e52ca --- /dev/null +++ b/changes/bug8746 @@ -0,0 +1,4 @@ + o Major bugfixes: + - When managing pluggable transports, use OS notification facilities to + learn if they have crashed, and do not attempt to kill any process + that has already exited. Fix for bug 8746; bugfix on 0.2.3.6-alpha.

1 0

[tor/master] Add ATTR_UNUSED to HT_FIND
by nickm＠torproject.org 14 Jun '14

14 Jun '14

commit 9b10d7a7dd822af0584474285d00c8228fbdbb82 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Jun 14 11:46:54 2014 -0400 Add ATTR_UNUSED to HT_FIND The fix for bug 8746 added a hashtable instance that never actually invoked HT_FIND. This caused a warning, since we didn't mark HT_FIND as okay-not-to-use. --- src/ext/ht.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ext/ht.h b/src/ext/ht.h index 871f5bb..8387107 100644 --- a/src/ext/ht.h +++ b/src/ext/ht.h @@ -160,7 +160,7 @@ ht_string_hash(const char *s) } \ /* Return a pointer to the element in the table 'head' matching 'elm', \ * or NULL if no such element exists */ \ - static INLINE struct type * \ + ATTR_UNUSED static INLINE struct type * \ name##_HT_FIND(const struct name *head, struct type *elm) \ { \ struct type **p; \

1 0

[tor/master] Merge branch 'bug12184_diagnostic_squashed'
by nickm＠torproject.org 14 Jun '14

14 Jun '14

commit a58d94fb7c6c304ecba930eaa7ebbade1d0686ab Merge: cfca2a6 8f3e327 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Jun 14 11:01:04 2014 -0400 Merge branch 'bug12184_diagnostic_squashed' changes/bug12184_diagnostic | 4 +++ src/or/circuitbuild.c | 70 ++++++++++++++++++++++++++++++++++++++----- src/or/circuitlist.c | 29 +++++++++++++++++- src/or/circuitlist.h | 2 ++ src/or/circuitmux.c | 35 ++++++++++++++++++++++ src/or/circuitmux.h | 4 +++ src/or/relay.c | 11 +++++++ src/or/relay.h | 2 ++ 8 files changed, 149 insertions(+), 8 deletions(-)

1 0

[tor/master] Try to diagnose bug 12184
by nickm＠torproject.org 14 Jun '14

14 Jun '14

commit 8f3e3279c198dd13c11ffd739ddf08dbe8b07762 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jun 10 13:35:45 2014 -0400 Try to diagnose bug 12184 Check for consistency between the queued destroy cells and the marked circuit IDs. Check for consistency in the count of queued destroy cells in several ways. Check to see whether any of the marked circuit IDs have somehow been marked longer than the channel has existed. --- changes/bug12184_diagnostic | 4 +++ src/or/circuitbuild.c | 70 ++++++++++++++++++++++++++++++++++++++----- src/or/circuitlist.c | 29 +++++++++++++++++- src/or/circuitlist.h | 2 ++ src/or/circuitmux.c | 35 ++++++++++++++++++++++ src/or/circuitmux.h | 4 +++ src/or/relay.c | 11 +++++++ src/or/relay.h | 2 ++ 8 files changed, 149 insertions(+), 8 deletions(-) diff --git a/changes/bug12184_diagnostic b/changes/bug12184_diagnostic new file mode 100644 index 0000000..3c783b1 --- /dev/null +++ b/changes/bug12184_diagnostic @@ -0,0 +1,4 @@ + o Minor features (diagnostic): + - Add significant new logging code to attempt to diagnose bug 12184, + where relays seem to run out of available circuit IDs. + diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 9d06759..e743a25 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -96,10 +96,12 @@ get_unique_circ_id_by_chan(channel_t *chan) * N=256. */ #define MAX_CIRCID_ATTEMPTS 64 int in_use; - unsigned n_with_circ = 0, n_pending_destroy = 0; + unsigned n_with_circ = 0, n_pending_destroy = 0, n_weird_pending_destroy = 0; circid_t test_circ_id; circid_t attempts=0; circid_t high_bit, max_range, mask; + int64_t pending_destroy_time_total = 0; + int64_t pending_destroy_time_max = 0; tor_assert(chan); @@ -130,15 +132,57 @@ get_unique_circ_id_by_chan(channel_t *chan) * whole circuit ID space every time we extend a circuit, which is * not so great either. */ - log_fn_ratelim(&chan->last_warned_circ_ids_exhausted, LOG_WARN, - LD_CIRC,"No unused circIDs found on channel %s wide " + int64_t queued_destroys; + char *m = rate_limit_log(&chan->last_warned_circ_ids_exhausted, + approx_time()); + if (m == NULL) + return 0; /* This message has been rate-limited away. */ + if (n_pending_destroy) + pending_destroy_time_total /= n_pending_destroy; + log_warn(LD_CIRC,"No unused circIDs found on channel %s wide " "circID support, with %u inbound and %u outbound circuits. " "Found %u circuit IDs in use by circuits, and %u with " - "pending destroy cells." - "Failing a circuit.", + "pending destroy cells. (%u of those were marked bogusly.) " + "The ones with pending destroy cells " + "have been marked unusable for an average of %ld seconds " + "and a maximum of %ld seconds. This channel is %ld seconds " + "old. Failing a circuit.%s", chan->wide_circ_ids ? "with" : "without", chan->num_p_circuits, chan->num_n_circuits, - n_with_circ, n_pending_destroy); + n_with_circ, n_pending_destroy, n_weird_pending_destroy, + (long)pending_destroy_time_total, + (long)pending_destroy_time_max, + (long)(approx_time() - chan->timestamp_created), + m); + tor_free(m); + + if (!chan->cmux) { + /* This warning should be impossible. */ + log_warn(LD_BUG, " This channel somehow has no cmux on it!"); + return 0; + } + + /* analysis so far on 12184 suggests that we're running out of circuit + IDs because it looks like we have too many pending destroy + cells. Let's see how many we really have pending. + */ + queued_destroys = circuitmux_count_queued_destroy_cells(chan, + chan->cmux); + + log_warn(LD_CIRC, " Circuitmux on this channel has %u circuits, " + "of which %u are active. It says it has "I64_FORMAT + " destroy cells queued.", + circuitmux_num_circuits(chan->cmux), + circuitmux_num_active_circuits(chan->cmux), + I64_PRINTF_ARG(queued_destroys)); + + /* Change this into "if (1)" in order to get more information about + * possible failure modes here. You'll need to know how to use gdb with + * Tor: this will make Tor exit with an assertion failure if the cmux is + * corrupt. */ + if (0) + circuitmux_assert_okay(chan->cmux); + return 0; } @@ -152,8 +196,20 @@ get_unique_circ_id_by_chan(channel_t *chan) in_use = circuit_id_in_use_on_channel(test_circ_id, chan); if (in_use == 1) ++n_with_circ; - else if (in_use == 2) + else if (in_use == 2) { + time_t since_when; ++n_pending_destroy; + since_when = + circuit_id_when_marked_unusable_on_channel(test_circ_id, chan); + if (since_when) { + time_t waiting = approx_time() - since_when; + pending_destroy_time_total += waiting; + if (waiting > pending_destroy_time_max) + pending_destroy_time_max = waiting; + } else { + ++n_weird_pending_destroy; + } + } } while (in_use); return test_circ_id; } diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 6238e08..ba16bb1 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -59,6 +59,8 @@ typedef struct chan_circid_circuit_map_t { channel_t *chan; circid_t circ_id; circuit_t *circuit; + /* For debugging 12184: when was this placeholder item added? */ + time_t made_placeholder_at; } chan_circid_circuit_map_t; /** Helper for hash tables: compare the channel and circuit ID for a and @@ -184,6 +186,7 @@ circuit_set_circid_chan_helper(circuit_t *circ, int direction, found = HT_FIND(chan_circid_map, &chan_circid_map, &search); if (found) { found->circuit = circ; + found->made_placeholder_at = 0; } else { found = tor_malloc_zero(sizeof(chan_circid_circuit_map_t)); found->circ_id = id; @@ -241,11 +244,14 @@ channel_mark_circid_unusable(channel_t *chan, circid_t id) "a circuit there.", (unsigned)id, chan); } else if (ent) { /* It's already marked. */ + if (!ent->made_placeholder_at) + ent->made_placeholder_at = approx_time(); } else { ent = tor_malloc_zero(sizeof(chan_circid_circuit_map_t)); ent->chan = chan; ent->circ_id = id; - /* leave circuit at NULL */ + /* leave circuit at NULL. */ + ent->made_placeholder_at = approx_time(); HT_INSERT(chan_circid_map, &chan_circid_map, ent); } } @@ -1090,6 +1096,27 @@ circuit_id_in_use_on_channel(circid_t circ_id, channel_t *chan) return 0; } +/** Helper for debugging 12184. Returns the time since which 'circ_id' has + * been marked unusable on 'chan'. */ +time_t +circuit_id_when_marked_unusable_on_channel(circid_t circ_id, channel_t *chan) +{ + chan_circid_circuit_map_t search; + chan_circid_circuit_map_t *found; + + memset(&search, 0, sizeof(search)); + search.circ_id = circ_id; + search.chan = chan; + + found = HT_FIND(chan_circid_map, &chan_circid_map, &search); + + if (! found || found->circuit) + return 0; + + return found->made_placeholder_at; +} + + /** Return the circuit that a given edge connection is using. */ circuit_t * circuit_get_by_edge_conn(edge_connection_t *conn) diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h index 916afba..d48d7c3 100644 --- a/src/or/circuitlist.h +++ b/src/or/circuitlist.h @@ -28,6 +28,8 @@ void circuit_set_n_circid_chan(circuit_t *circ, circid_t id, channel_t *chan); void channel_mark_circid_unusable(channel_t *chan, circid_t id); void channel_mark_circid_usable(channel_t *chan, circid_t id); +time_t circuit_id_when_marked_unusable_on_channel(circid_t circ_id, + channel_t *chan); void circuit_set_state(circuit_t *circ, uint8_t state); void circuit_close_all_marked(void); int32_t circuit_initial_package_window(void); diff --git a/src/or/circuitmux.c b/src/or/circuitmux.c index 52ebfef..563166e 100644 --- a/src/or/circuitmux.c +++ b/src/or/circuitmux.c @@ -1909,3 +1909,38 @@ circuitmux_append_destroy_cell(channel_t *chan, } } +/*DOCDOC; for debugging 12184. This runs slowly. */ +int64_t +circuitmux_count_queued_destroy_cells(const channel_t *chan, + const circuitmux_t *cmux) +{ + int64_t n_destroy_cells = cmux->destroy_ctr; + int64_t destroy_queue_size = cmux->destroy_cell_queue.n; + + int64_t manual_total = 0; + int64_t manual_total_in_map = 0; + packed_cell_t *cell; + + TOR_SIMPLEQ_FOREACH(cell, &cmux->destroy_cell_queue.head, next) { + circid_t id; + ++manual_total; + + id = packed_cell_get_circid(cell, chan->wide_circ_ids); + if (circuit_id_in_use_on_channel(id, (channel_t*)chan)) + ++manual_total_in_map; + } + + if (n_destroy_cells != destroy_queue_size || + n_destroy_cells != manual_total || + n_destroy_cells != manual_total_in_map) { + log_warn(LD_BUG, " Discrepancy in counts for queued destroy cells on " + "circuitmux. n="I64_FORMAT". queue_size="I64_FORMAT". " + "manual_total="I64_FORMAT". manual_total_in_map="I64_FORMAT".", + I64_PRINTF_ARG(n_destroy_cells), + I64_PRINTF_ARG(destroy_queue_size), + I64_PRINTF_ARG(manual_total), + I64_PRINTF_ARG(manual_total_in_map)); + } + + return n_destroy_cells; +} diff --git a/src/or/circuitmux.h b/src/or/circuitmux.h index c4c0649..2b5fb7e 100644 --- a/src/or/circuitmux.h +++ b/src/or/circuitmux.h @@ -121,6 +121,10 @@ unsigned int circuitmux_num_cells(circuitmux_t *cmux); unsigned int circuitmux_num_circuits(circuitmux_t *cmux); unsigned int circuitmux_num_active_circuits(circuitmux_t *cmux); +/* Debuging interface - slow. */ +int64_t circuitmux_count_queued_destroy_cells(const channel_t *chan, + const circuitmux_t *cmux); + /* Channel interface */ circuit_t * circuitmux_get_first_active_circuit(circuitmux_t *cmux, cell_queue_t **destroy_queue_out); diff --git a/src/or/relay.c b/src/or/relay.c index 509d7ce..58d6db2 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -2563,6 +2563,17 @@ packed_cell_get_command(const packed_cell_t *cell, int wide_circ_ids) } } +/** Extract the circuit ID from a packed cell. */ +circid_t +packed_cell_get_circid(const packed_cell_t *cell, int wide_circ_ids) +{ + if (wide_circ_ids) { + return ntohl(get_uint32(cell->body)); + } else { + return ntohs(get_uint16(cell->body)); + } +} + /** Pull as many cells as possible (but no more than <b>max</b>) from the * queue of the first active circuit on <b>chan</b>, and write them to * <b>chan</b>->outbuf. Return the number of cells written. Advance diff --git a/src/or/relay.h b/src/or/relay.h index 479d474..969c6fb 100644 --- a/src/or/relay.h +++ b/src/or/relay.h @@ -82,6 +82,8 @@ void stream_choice_seed_weak_rng(void); int relay_crypt(circuit_t *circ, cell_t *cell, cell_direction_t cell_direction, crypt_path_t **layer_hint, char *recognized); +circid_t packed_cell_get_circid(const packed_cell_t *cell, int wide_circ_ids); + #ifdef RELAY_PRIVATE STATIC int connected_cell_parse(const relay_header_t *rh, const cell_t *cell, tor_addr_t *addr_out, int *ttl_out);

1 0

r26837: {website} Provide Webroot users with an actual solution. (website/trunk/docs/en)
by Matt Pagan 14 Jun '14

14 Jun '14

Author: mttp Date: 2014-06-14 14:00:40 +0000 (Sat, 14 Jun 2014) New Revision: 26837 Modified: website/trunk/docs/en/faq.wml Log: Provide Webroot users with an actual solution. Modified: website/trunk/docs/en/faq.wml =================================================================== --- website/trunk/docs/en/faq.wml 2014-06-14 11:51:00 UTC (rev 26836) +++ website/trunk/docs/en/faq.wml 2014-06-14 14:00:40 UTC (rev 26837) @@ -83,8 +83,8 @@ </a></li> <li><a href="#SophosOnMac">I'm using the Sophos anti-virus software on my Mac, and Tor starts but I can't browse anywhere.</a></li> - <li><a href="#XPCOMError">When I open the Tor Browser Bundle I get an -error message from the browser: "Cannot load XPCOM".</a></li> + <li><a href="#XPCOMError">When I start Tor Browser I get an +error message: "Cannot load XPCOM".</a></li> <li><a href="#TBBOtherExtensions">Can I install other Firefox extensions? Which extensions should I avoid using?</a></li> <li><a href="#TBBJavaScriptEnabled">Why is NoScript configured to @@ -1193,16 +1193,17 @@ <hr> <a id="XPCOMError"></a> -<h3><a class="anchor" href="#XPCOMError">When I open the Tor Browser Bundle -I get an error message from the browser: "Cannot load XPCOM".</a></h3> +<h3><a class="anchor" href="#XPCOMError">When I start Tor Browser I get an +error message: "Cannot load XPCOM".</a></h3> <p> -This problem is caused by the Webroot SecureAnywhere Antivirus software. -Consider switching to a different antivirus program. We encourage affected -Webroot users to contact Webroot support about this issue. -<a href="https://trac.torproject.org/projects/tor/ticket/10789">Ticket -10789</a> has some suggestions for other workarounds; please let us know -if you find better fixes! +This <a +href="https://trac.torproject.org/projects/tor/ticket/10789">problem</a> is +specifically caused by the Webroot SecureAnywhere Antivirus software. +From the Webroot control panel, go to Identity Protection → Application +Protection, and set all the files in your Tor Browser folder to 'Allow'. +We encourage affected Webroot users to contact Webroot support about this +issue. </p> <hr>

1 0