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

[tor/maint-0.2.3] Resolve crash caused by format_helper_exit_status changes in #5557
by nickm＠torproject.org 25 Jun '12

25 Jun '12

commit b1ad1a1d0266a20bb0dac15e65abe7b65a74e8a0 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Jun 23 15:30:01 2012 -0400 Resolve crash caused by format_helper_exit_status changes in #5557 Because the string output was no longer equal in length to HEX_ERRNO_SIZE, the write() call would add some extra spaces and maybe a NUL, and the NUL would trigger an assert in get_string_from_pipe. Fixes bug 6225; bug not in any released version of Tor. --- src/common/util.c | 29 ++++++++++++++++++++--------- src/common/util.h | 4 ++-- src/test/test_util.c | 16 +++++++++++----- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index cb8ff85..8cb013e 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3264,8 +3264,11 @@ format_hex_number_for_helper_exit_status(unsigned int x, char *buf, * in the processs of starting the child process did the failure occur (see * CHILD_STATE_* macros for definition), and SAVED_ERRNO is the value of * errno when the failure occurred. + * + * On success return the number of characters added to hex_errno, not counting + * the terminating NUL; return -1 on error. */ -void +int format_helper_exit_status(unsigned char child_state, int saved_errno, char *hex_errno) { @@ -3273,6 +3276,7 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, int written, left; char *cur; size_t i; + int res = -1; /* Fill hex_errno with spaces, and a trailing newline (memset may not be signal handler safe, so we can't use it) */ @@ -3343,6 +3347,8 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, *cur++ = '\n'; *cur++ = '\0'; + res = cur - hex_errno - 1; + goto done; err: @@ -3353,7 +3359,7 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, *hex_errno = '\0'; done: - return; + return res; } /* Maximum number of file descriptors, if we cannot get it via sysconf() */ @@ -3695,15 +3701,20 @@ tor_spawn_background(const char *const filename, const char **argv, child_state = CHILD_STATE_FAILEXEC; error: - /* XXX: are we leaking fds from the pipe? */ + { + /* XXX: are we leaking fds from the pipe? */ + int n; - format_helper_exit_status(child_state, errno, hex_errno); + n = format_helper_exit_status(child_state, errno, hex_errno); - /* Write the error message. GCC requires that we check the return - value, but there is nothing we can do if it fails */ - /* TODO: Don't use STDOUT, use a pipe set up just for this purpose */ - nbytes = write(STDOUT_FILENO, error_message, error_message_length); - nbytes = write(STDOUT_FILENO, hex_errno, sizeof(hex_errno)); + if (n >= 0) { + /* Write the error message. GCC requires that we check the return + value, but there is nothing we can do if it fails */ + /* TODO: Don't use STDOUT, use a pipe set up just for this purpose */ + nbytes = write(STDOUT_FILENO, error_message, error_message_length); + nbytes = write(STDOUT_FILENO, hex_errno, n); + } + } (void) nbytes; diff --git a/src/common/util.h b/src/common/util.h index 6b7c6fb..a2ab0cc 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -473,8 +473,8 @@ void tor_process_handle_destroy(process_handle_t *process_handle, int format_hex_number_for_helper_exit_status(unsigned int x, char *buf, int max_len); -void format_helper_exit_status(unsigned char child_state, - int saved_errno, char *hex_errno); +int format_helper_exit_status(unsigned char child_state, + int saved_errno, char *hex_errno); /* Space for hex values of child state, a slash, saved_errno (with leading minus) and newline (no null) */ diff --git a/src/test/test_util.c b/src/test/test_util.c index d71d280..fab9595 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -2138,28 +2138,34 @@ test_util_exit_status(void *ptr) { /* Leave an extra byte for a \0 so we can do string comparison */ char hex_errno[HEX_ERRNO_SIZE + 1]; + int n; (void)ptr; clear_hex_errno(hex_errno); - format_helper_exit_status(0, 0, hex_errno); + n = format_helper_exit_status(0, 0, hex_errno); test_streq("0/0\n", hex_errno); + test_eq(n, strlen(hex_errno)); clear_hex_errno(hex_errno); - format_helper_exit_status(0, 0x7FFFFFFF, hex_errno); + n = format_helper_exit_status(0, 0x7FFFFFFF, hex_errno); test_streq("0/7FFFFFFF\n", hex_errno); + test_eq(n, strlen(hex_errno)); clear_hex_errno(hex_errno); - format_helper_exit_status(0xFF, -0x80000000, hex_errno); + n = format_helper_exit_status(0xFF, -0x80000000, hex_errno); test_streq("FF/-80000000\n", hex_errno); + test_eq(n, strlen(hex_errno)); clear_hex_errno(hex_errno); - format_helper_exit_status(0x7F, 0, hex_errno); + n = format_helper_exit_status(0x7F, 0, hex_errno); test_streq("7F/0\n", hex_errno); + test_eq(n, strlen(hex_errno)); clear_hex_errno(hex_errno); - format_helper_exit_status(0x08, -0x242, hex_errno); + n = format_helper_exit_status(0x08, -0x242, hex_errno); test_streq("8/-242\n", hex_errno); + test_eq(n, strlen(hex_errno)); done: ;

1 0

[tor/maint-0.2.3] Merge remote-tracking branch 'public/bug6225' into maint-0.2.3
by nickm＠torproject.org 25 Jun '12

25 Jun '12

commit 2703e072a1056edd3b68a16e045175e5bd268301 Merge: a6688c5 ffd7189 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Jun 25 11:51:19 2012 -0400 Merge remote-tracking branch 'public/bug6225' into maint-0.2.3 src/common/util.c | 34 ++++++++++++++++++++++++---------- src/common/util.h | 4 ++-- src/test/test_util.c | 16 +++++++++++----- 3 files changed, 37 insertions(+), 17 deletions(-)

1 0

[stem/master] Subprocess lacks a kill() method on python 2.5
by atagar＠torproject.org 25 Jun '12

25 Jun '12

commit 9d54db62a7b918c3bc6a852275adaf70dfdfef44 Author: Damian Johnson <atagar(a)torproject.org> Date: Fri Jun 22 09:14:20 2012 -0700 Subprocess lacks a kill() method on python 2.5 The Popen.kill() ad Popen.terminate() methods were first added in python 2.6... http://docs.python.org/library/subprocess.html#subprocess.Popen.kill We can work around this with os.kill() but that only works on *nix... http://stackoverflow.com/questions/552423/use-python-2-6-subprocess-module-… --- stem/process.py | 18 ++++++++++++++++-- test/integ/process.py | 18 +++++++++++++++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/stem/process.py b/stem/process.py index 36ef7b9..e9d453e 100644 --- a/stem/process.py +++ b/stem/process.py @@ -23,6 +23,7 @@ import signal import tempfile import subprocess +import stem.prereq import stem.util.system NO_TORRC = "<no torrc>" @@ -80,7 +81,15 @@ def launch_tor(tor_cmd = "tor", args = None, torrc_path = None, completion_perce try: os.remove(temp_file) except: pass - tor_process.kill() + # We can't kill the subprocess on python 2.5 running Windows without the + # win32process module... + # http://stackoverflow.com/questions/552423/use-python-2-6-subprocess-module-… + + if stem.prereq.is_python_26(): + tor_process.kill() + elif stem.util.system.is_windows(): + os.kill(tor_process.pid, signal.SIGTERM) + raise OSError("reached a %i second timeout without success" % timeout) signal.signal(signal.SIGALRM, timeout_handler) @@ -95,7 +104,12 @@ def launch_tor(tor_cmd = "tor", args = None, torrc_path = None, completion_perce # this will provide empty results if the process is terminated if not init_line: - tor_process.kill() # ... but best make sure + # ... but best make sure + if stem.prereq.is_python_26(): + tor_process.kill() + elif stem.util.system.is_windows(): + os.kill(tor_process.pid, signal.SIGTERM) + raise OSError("Process terminated: %s" % last_problem) # provide the caller with the initialization message if they want it diff --git a/test/integ/process.py b/test/integ/process.py index e3dd277..2037d93 100644 --- a/test/integ/process.py +++ b/test/integ/process.py @@ -2,12 +2,16 @@ Tests the stem.process functions with various use cases. """ +import os import time +import signal import unittest +import stem.prereq import stem.socket import stem.process import test.runner +import stem.util.system class TestProcess(unittest.TestCase): def test_launch_tor_with_config(self): @@ -15,6 +19,10 @@ class TestProcess(unittest.TestCase): Exercises launch_tor_with_config. """ + if not stem.prereq.is_python_26() and stem.util.system.is_windows(): + test.runner.skip("(unable to kill subprocesses)") + return + if test.runner.only_run_once(self, "test_launch_tor_with_config"): return # Launch tor without a torrc, but with a control port. Confirms that this @@ -37,13 +45,21 @@ class TestProcess(unittest.TestCase): self.assertEquals("ControlPort=2778", str(getconf_response)) finally: if control_socket: control_socket.close() - tor_process.kill() + + if stem.prereq.is_python_26(): + tor_process.kill() + elif stem.util.system.is_windows(): + os.kill(tor_process.pid, signal.SIGTERM) def test_launch_tor_with_timeout(self): """ Runs launch_tor where it times out before completing. """ + if not stem.prereq.is_python_26() and stem.util.system.is_windows(): + test.runner.skip("(unable to kill subprocesses)") + return + if test.runner.only_run_once(self, "test_launch_tor_with_timeout"): return start_time = time.time()

1 0

[stem/master] Avoiding the 'with' keyword for tarfile
by atagar＠torproject.org 25 Jun '12

25 Jun '12

commit e51fea598dced5e3c4a05c4cd0ad509beed530fe Author: Damian Johnson <atagar(a)torproject.org> Date: Thu Jun 21 09:45:14 2012 -0700 Avoiding the 'with' keyword for tarfile The tarfile's __exit__() method was added sometime after python 2.5, causing it to produce a stacktrace if used via the 'with' keyword. Pity, yet another hack to accomidate a six-year-old version of python... --- stem/descriptor/reader.py | 31 ++++++++++++++++++++----------- 1 files changed, 20 insertions(+), 11 deletions(-) diff --git a/stem/descriptor/reader.py b/stem/descriptor/reader.py index 7a7e565..716b58c 100644 --- a/stem/descriptor/reader.py +++ b/stem/descriptor/reader.py @@ -443,22 +443,31 @@ class DescriptorReader: self._notify_skip_listeners(target, ReadFailed(exc)) def _handle_archive(self, target): + # TODO: This would be nicer via the 'with' keyword, but tarfile's __exit__ + # method was added sometime after python 2.5. We should change this when + # we drop python 2.5 support. + + tar_file = None + try: - with tarfile.open(target) as tar_file: - for tar_entry in tar_file: - if tar_entry.isfile(): - entry = tar_file.extractfile(tar_entry) - - for desc in stem.descriptor.parse_file(target, entry): - if self._is_stopped.isSet(): return - self._unreturned_descriptors.put(desc) - self._iter_notice.set() - - entry.close() + tar_file = tarfile.open(target) + + for tar_entry in tar_file: + if tar_entry.isfile(): + entry = tar_file.extractfile(tar_entry) + + for desc in stem.descriptor.parse_file(target, entry): + if self._is_stopped.isSet(): return + self._unreturned_descriptors.put(desc) + self._iter_notice.set() + + entry.close() except TypeError, exc: self._notify_skip_listeners(target, ParsingFailure(exc)) except IOError, exc: self._notify_skip_listeners(target, ReadFailed(exc)) + finally: + if tar_file: tar_file.close() def _notify_skip_listeners(self, path, exception): for listener in self._skip_listeners:

1 0

[stem/master] Missed another tarfile 'with' declaration
by atagar＠torproject.org 25 Jun '12

25 Jun '12

commit ac34b8151c5b46369578922ba4f90fdce15128b3 Author: Damian Johnson <atagar(a)torproject.org> Date: Fri Jun 22 09:18:39 2012 -0700 Missed another tarfile 'with' declaration Missed that we were using the 'with' keyword for a tarfile in the integ tests. Using a try/catch instead so this'll work on python 2.5. --- test/integ/descriptor/reader.py | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/test/integ/descriptor/reader.py b/test/integ/descriptor/reader.py index 27d096f..25952de 100644 --- a/test/integ/descriptor/reader.py +++ b/test/integ/descriptor/reader.py @@ -50,13 +50,20 @@ def _get_raw_tar_descriptors(): test_path = os.path.join(DESCRIPTOR_TEST_DATA, "descriptor_archive.tar") raw_descriptors = [] - with tarfile.open(test_path) as tar_file: + # TODO: revert to using the 'with' keyword for this when dropping python 2.5 support + tar_file = None + + try: + tar_file = tarfile.open(test_path) + for tar_entry in tar_file: if tar_entry.isfile(): entry = tar_file.extractfile(tar_entry) entry.readline() # strip header raw_descriptors.append(entry.read()) entry.close() + finally: + if tar_file: tar_file.close() TAR_DESCRIPTORS = raw_descriptors

1 0

[stem/master] get_pid_by_port() unavailable on Mac OSX
by atagar＠torproject.org 25 Jun '12

25 Jun '12

commit 2fc1c420818bff19437a2b74713223a410d9898e Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Jun 23 11:47:24 2012 -0700 get_pid_by_port() unavailable on Mac OSX Tests for get_pid_by_port() are failing on Mac OSX 10.5.8 because sockstat is unavailable and lsof neither provides ports nor accepts the flags we need. Noting this in the pydocs/comments, and skipping its integ tests when on a mac. --- stem/util/system.py | 16 +++++++++++++++- test/integ/util/system.py | 6 ++++++ 2 files changed, 21 insertions(+), 1 deletions(-) diff --git a/stem/util/system.py b/stem/util/system.py index 73fd47e..b4c326f 100644 --- a/stem/util/system.py +++ b/stem/util/system.py @@ -8,6 +8,7 @@ best-effort, providing None if the lookup fails. :: is_windows - checks if we're running on windows + is_mac - checks if we're running on a mac is_bsd - checks if we're running on the bsd family of operating systems is_available - determines if a command is availabe on this system is_running - determines if a given process is running @@ -58,6 +59,15 @@ def is_windows(): return platform.system() == "Windows" +def is_mac(): + """ + Checks if we are running on Mac OSX. + + :returns: bool to indicate if we're on a Mac + """ + + return platform.system() == "Darwin" + def is_bsd(): """ Checks if we are within the BSD family of operating systems. This presently @@ -260,7 +270,8 @@ def get_pid_by_port(port): 2. sockstat -4l -P tcp -p <port> 3. lsof -wnP -iTCP -sTCP:LISTEN | grep ":<port>" - Most queries limit results to listening TCP connections. + Most queries limit results to listening TCP connections. This function likely + won't work on Mac OSX. :param int port: port where the process we're looking for is listening @@ -301,6 +312,7 @@ def get_pid_by_port(port): # attempts to resolve using sockstat, failing if: # - sockstat doesn't accept the -4 flag (BSD only) + # - sockstat isn't available (encountered with OSX 10.5.8) # - there are multiple instances using the same port on different addresses # # flags: @@ -334,6 +346,8 @@ def get_pid_by_port(port): # resolves using lsof which works on both Linux and BSD, only failing if: # - lsof is unavailable (not included by default on OpenBSD) + # - lsof doesn't provide the port ip/port, nor accept the -i and -s args + # (encountered with OSX 10.5.8) # - the process being run as a different user due to permissions # - there are multiple instances using the same port on different addresses # diff --git a/test/integ/util/system.py b/test/integ/util/system.py index 174357e..7999cbb 100644 --- a/test/integ/util/system.py +++ b/test/integ/util/system.py @@ -201,6 +201,9 @@ class TestSystem(unittest.TestCase): if not _has_port(): test.runner.skip(self, "(test instance has no port)") return + elif stem.util.system.is_mac(): + test.runner.skip(self, "(resolvers unavailable)") + return elif not runner.is_ptraceable(): test.runner.skip(self, "(DisableDebuggerAttachment is set)") return @@ -271,6 +274,9 @@ class TestSystem(unittest.TestCase): elif not stem.util.system.is_available("lsof"): test.runner.skip(self, "(lsof unavailable)") return + elif stem.util.system.is_mac(): + test.runner.skip(self, "(resolvers unavailable)") + return elif not runner.is_ptraceable(): test.runner.skip(self, "(DisableDebuggerAttachment is set)") return

1 0

[stem/master] Respecting custom tor commands for process integ tests
by atagar＠torproject.org 25 Jun '12

25 Jun '12

commit a5f6006b3e1d072e1cd9524aa57408d707ff3ba0 Author: Damian Johnson <atagar(a)torproject.org> Date: Fri Jun 22 09:27:03 2012 -0700 Respecting custom tor commands for process integ tests The process integ tests were attempting to run 'tor', which naturally doesn't work if it isn't in your path. Using the command that we're using for our main test instance instead since that is sure to work. --- test/integ/process.py | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/integ/process.py b/test/integ/process.py index 2037d93..0573a1c 100644 --- a/test/integ/process.py +++ b/test/integ/process.py @@ -28,7 +28,9 @@ class TestProcess(unittest.TestCase): # Launch tor without a torrc, but with a control port. Confirms that this # works by checking that we're still able to access the new instance. + runner = test.runner.get_runner() tor_process = stem.process.launch_tor_with_config( + tor_cmd = runner.get_tor_command(), config = {'SocksPort': '2777', 'ControlPort': '2778'}, completion_percent = 5 ) @@ -36,7 +38,6 @@ class TestProcess(unittest.TestCase): control_socket = None try: control_socket = stem.socket.ControlPort(control_port = 2778) - runner = test.runner.get_runner() stem.connection.authenticate(control_socket, chroot_path = runner.get_chroot()) # exercises the socket @@ -62,8 +63,9 @@ class TestProcess(unittest.TestCase): if test.runner.only_run_once(self, "test_launch_tor_with_timeout"): return + runner = test.runner.get_runner() start_time = time.time() - self.assertRaises(OSError, stem.process.launch_tor_with_config, {'SocksPort': '2777'}, "tor", 100, None, 2) + self.assertRaises(OSError, stem.process.launch_tor_with_config, {'SocksPort': '2777'}, runner.get_tor_command(), 100, None, 2) runtime = time.time() - start_time if not (runtime > 2 and runtime < 3):

1 0

[stem/master] Missing kill() fallback when ending integ tests
by atagar＠torproject.org 25 Jun '12

25 Jun '12

commit 4e02850db89006f256290e8bc7bea7169c6cd57c Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Jun 23 11:55:57 2012 -0700 Missing kill() fallback when ending integ tests As an earlier commit mentioned, Popen.kill() is unavailable on python 2.5, and its fallback (os.kill()) is unavailable on Windows. Accounting for this in the integ test runner. --- test/runner.py | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/test/runner.py b/test/runner.py index 1bf1860..3b696b3 100644 --- a/test/runner.py +++ b/test/runner.py @@ -41,6 +41,7 @@ import sys import time import stat import shutil +import signal import logging import tempfile import threading @@ -305,7 +306,13 @@ class Runner: # if the tor process has stopped on its own then the following raises # an OSError ([Errno 3] No such process) - try: self._tor_process.kill() + try: + if stem.prereq.is_python_26(): + self._tor_process.kill() + elif not stem.util.system.is_windows(): + os.kill(self._tor_process.pid, signal.SIGTERM) + else: + test.output.print_line("failed (unable to call kill() in python 2.5)", *ERROR_ATTR) except OSError: pass self._tor_process.communicate() # blocks until the process is done

1 0

[stem/master] Inverted check when falling back to kill a process
by atagar＠torproject.org 25 Jun '12

25 Jun '12

commit 04a6225ffdccc81a431044d05f4f411c7e5a88f5 Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Jun 23 14:18:32 2012 -0700 Inverted check when falling back to kill a process Oops, we're supposed to use os.kill() as a fallback if we *aren't* on windows. --- stem/process.py | 4 ++-- test/integ/process.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/stem/process.py b/stem/process.py index e9d453e..337699d 100644 --- a/stem/process.py +++ b/stem/process.py @@ -87,7 +87,7 @@ def launch_tor(tor_cmd = "tor", args = None, torrc_path = None, completion_perce if stem.prereq.is_python_26(): tor_process.kill() - elif stem.util.system.is_windows(): + elif not stem.util.system.is_windows(): os.kill(tor_process.pid, signal.SIGTERM) raise OSError("reached a %i second timeout without success" % timeout) @@ -107,7 +107,7 @@ def launch_tor(tor_cmd = "tor", args = None, torrc_path = None, completion_perce # ... but best make sure if stem.prereq.is_python_26(): tor_process.kill() - elif stem.util.system.is_windows(): + elif not stem.util.system.is_windows(): os.kill(tor_process.pid, signal.SIGTERM) raise OSError("Process terminated: %s" % last_problem) diff --git a/test/integ/process.py b/test/integ/process.py index 0573a1c..423b2c3 100644 --- a/test/integ/process.py +++ b/test/integ/process.py @@ -49,7 +49,7 @@ class TestProcess(unittest.TestCase): if stem.prereq.is_python_26(): tor_process.kill() - elif stem.util.system.is_windows(): + elif not stem.util.system.is_windows(): os.kill(tor_process.pid, signal.SIGTERM) def test_launch_tor_with_timeout(self):

1 0

[stem/master] Respecting reader stop() calls in mid-directory
by atagar＠torproject.org 25 Jun '12

25 Jun '12

commit e625e84b003420908d410ab10f51fdd78a1ccf17 Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Jun 23 12:16:53 2012 -0700 Respecting reader stop() calls in mid-directory If we, say, attempted to read a directory with a million files then the reader's stop() method won't be respected until we've finished with that directory. That's stupid. Checking the _is_stopped Event after processing each file. Python lacks targeted breaks, so I needed to refactor the code a bit to accomidate this. --- stem/descriptor/reader.py | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-) diff --git a/stem/descriptor/reader.py b/stem/descriptor/reader.py index 716b58c..8d4a462 100644 --- a/stem/descriptor/reader.py +++ b/stem/descriptor/reader.py @@ -354,13 +354,7 @@ class DescriptorReader: else: walker = os.walk(target) - # adds all of the files that it contains - for root, _, files in walker: - for filename in files: - self._handle_file(os.path.join(root, filename), new_processed_files) - - # this can take a while if, say, we're including the root directory - if self._is_stopped.isSet(): break + self._handle_walker(walker, new_processed_files) else: self._handle_file(target, new_processed_files) @@ -383,6 +377,14 @@ class DescriptorReader: self._iter_notice.wait() self._iter_notice.clear() + def _handle_walker(self, walker, new_processed_files): + for root, _, files in walker: + for filename in files: + self._handle_file(os.path.join(root, filename), new_processed_files) + + # this can take a while if, say, we're including the root directory + if self._is_stopped.isSet(): return + def _handle_file(self, target, new_processed_files): # This is a file. Register its last modified timestamp and check if # it's a file that we should skip.

1 0