[tor-commits] [stem/master] Unexpected exceptions could cause launch_tor() to leave a lingering process
atagar at torproject.org
atagar at torproject.org
Tue Dec 29 18:02:42 UTC 2015
commit 75f311db973412f3754eadacdd08104173f768a1
Author: Damian Johnson <atagar at torproject.org>
Date: Tue Dec 29 10:06:16 2015 -0800
Unexpected exceptions could cause launch_tor() to leave a lingering process
Good point from germn that KeyboardInterrupts and other unxpected exception
types could cause launch_tor() to leave an orphaned tor process behind...
https://trac.torproject.org/projects/tor/ticket/17946
Ensuring that when we raise an exception it gets terminated.
---
docs/change_log.rst | 1 +
stem/control.py | 2 +-
stem/process.py | 49 +++++++++++++++++++++++++++++++------------------
test/integ/process.py | 22 ++++++++++++++++++++++
4 files changed, 55 insertions(+), 19 deletions(-)
diff --git a/docs/change_log.rst b/docs/change_log.rst
index 87ea053..fb0a71c 100644
--- a/docs/change_log.rst
+++ b/docs/change_log.rst
@@ -50,6 +50,7 @@ The following are only available within Stem's `git repository
* Added `support for NETWORK_LIVENESS events <api/response.html#stem.response.events.NetworkLivenessEvent>`_ (:spec:`44aac63`)
* Added :func:`~stem.control.Controller.is_user_traffic_allowed` to the :class:`~stem.control.Controller`
* Added the replica attribute to the :class:`~stem.response.events.HSDescEvent` (:spec:`4989e73`)
+ * :func:`~stem.process.launch_tor` could leave a lingering process during an unexpected exception (:trac:`17946`)
* IPv6 addresses could trigger errors in :func:`~stem.control.Controller.get_listeners`, :class:`~stem.response.events.ORConnEvent`, and quite a few other things (:trac:`16174`)
* Don't obscure stacktraces, most notably :class:`~stem.control.Controller` getter methods with default values
* Classes with custom equality checks didn't provide a corresponding inequality method
diff --git a/stem/control.py b/stem/control.py
index 3b1edfa..e4509a9 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -1713,7 +1713,7 @@ class Controller(BaseController):
raise stem.DescriptorUnavailable('Descriptor information is unavailable, tor might still be downloading it')
return stem.descriptor.server_descriptor.RelayDescriptor(desc_content)
- except Exception as exc:
+ except:
if not self._is_server_descriptors_available():
raise ValueError(SERVER_DESCRIPTORS_UNSUPPORTED)
diff --git a/stem/process.py b/stem/process.py
index b8b1f92..f2d63f0 100644
--- a/stem/process.py
+++ b/stem/process.py
@@ -22,6 +22,7 @@ import os
import re
import signal
import subprocess
+import sys
import tempfile
import stem.prereq
@@ -105,27 +106,26 @@ def launch_tor(tor_cmd = 'tor', args = None, torrc_path = None, completion_perce
if take_ownership:
runtime_args += ['__OwningControllerProcess', str(os.getpid())]
- tor_process = subprocess.Popen(runtime_args, stdout = subprocess.PIPE, stdin = subprocess.PIPE, stderr = subprocess.PIPE)
+ tor_process = None
- if stdin:
- tor_process.stdin.write(stem.util.str_tools._to_bytes(stdin))
- tor_process.stdin.close()
+ try:
+ tor_process = subprocess.Popen(runtime_args, stdout = subprocess.PIPE, stdin = subprocess.PIPE, stderr = subprocess.PIPE)
- if timeout:
- def timeout_handler(signum, frame):
- # terminates the uninitialized tor process and raise on timeout
+ if stdin:
+ tor_process.stdin.write(stem.util.str_tools._to_bytes(stdin))
+ tor_process.stdin.close()
- tor_process.kill()
- raise OSError('reached a %i second timeout without success' % timeout)
+ if timeout:
+ def timeout_handler(signum, frame):
+ raise OSError('reached a %i second timeout without success' % timeout)
- signal.signal(signal.SIGALRM, timeout_handler)
- signal.alarm(timeout)
+ signal.signal(signal.SIGALRM, timeout_handler)
+ signal.alarm(timeout)
- bootstrap_line = re.compile('Bootstrapped ([0-9]+)%: ')
- problem_line = re.compile('\[(warn|err)\] (.*)$')
- last_problem = 'Timed out'
+ bootstrap_line = re.compile('Bootstrapped ([0-9]+)%: ')
+ problem_line = re.compile('\[(warn|err)\] (.*)$')
+ last_problem = 'Timed out'
- try:
while True:
# Tor's stdout will be read as ASCII bytes. This is fine for python 2, but
# in python 3 that means it'll mismatch with other operations (for instance
@@ -139,7 +139,6 @@ 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
raise OSError('Process terminated: %s' % last_problem)
# provide the caller with the initialization message if they want it
@@ -162,12 +161,26 @@ def launch_tor(tor_cmd = 'tor', args = None, torrc_path = None, completion_perce
msg = msg.split(': ')[-1].strip()
last_problem = msg
+ except:
+ if tor_process:
+ tor_process.kill() # don't leave a lingering process
+ tor_process.wait()
+
+ exc = sys.exc_info()[1]
+
+ if type(exc) == OSError:
+ raise # something we're raising ourselves
+ else:
+ raise OSError('Unexpected exception while starting tor (%s): %s' % (type(exc).__name__, exc))
finally:
if timeout:
signal.alarm(0) # stop alarm
- tor_process.stdout.close()
- tor_process.stderr.close()
+ if tor_process and tor_process.stdout:
+ tor_process.stdout.close()
+
+ if tor_process and tor_process.stderr:
+ tor_process.stderr.close()
if temp_file:
try:
diff --git a/test/integ/process.py b/test/integ/process.py
index eec1e1e..3b44ef5 100644
--- a/test/integ/process.py
+++ b/test/integ/process.py
@@ -196,6 +196,28 @@ class TestProcess(unittest.TestCase):
self.assertTrue('UseBridges' in output)
self.assertTrue('SocksPort' in output)
+ @patch('re.compile', Mock(side_effect = KeyboardInterrupt('nope')))
+ def test_no_orphaned_process(self):
+ """
+ Check that when an exception arises in the middle of spawning tor that we
+ don't leave a lingering process.
+ """
+
+ # We don't need to actually run tor for this test. Rather, any process will
+ # do the trick. Picking sleep so this'll clean itself up if our test fails.
+
+ mock_tor_process = subprocess.Popen(['sleep', '60'])
+
+ with patch('subprocess.Popen', Mock(return_value = mock_tor_process)):
+ try:
+ stem.process.launch_tor()
+ self.fail("tor shoudn't have started")
+ except OSError as exc:
+ if os.path.exists('/proc/%s' % mock_tor_process.pid):
+ self.fail("launch_tor() left a lingering tor process")
+
+ self.assertEqual('Unexpected exception while starting tor (KeyboardInterrupt): nope', str(exc))
+
def test_torrc_arguments(self):
"""
Pass configuration options on the commandline.
More information about the tor-commits
mailing list