[tor-bugs] #17946 [Stem]: launch_tor should kill process on exception while bootstrapping (for example, on KeyboardInterrupt)

[Tor Bug Tracker & Wiki]

#17946: launch_tor should kill process on exception while bootstrapping (for
example, on KeyboardInterrupt)
-----------------------------------+------------------------
 Reporter:  germn                  |          Owner:  atagar
     Type:  defect                 |         Status:  new
 Priority:  Medium                 |      Milestone:
Component:  Stem                   |        Version:
 Severity:  Normal                 |     Resolution:
 Keywords:  launch_tor, bootstrap  |  Actual Points:
Parent ID:                         |         Points:
  Sponsor:                         |
-----------------------------------+------------------------

Comment (by germn):

 Hello, thanks for answer!

 I didn't understent this part:


 {{{
   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))
 }}}

 If KeyboardInterrupt (or asyncio.CancelledError, or other) was raised, we
 should reraise exactly this error after resources cleanup, we shouldn't
 raise our type of error instead. Top-level code might want to know if
 exactly KeyboardInterrupt happened.

 Here's little syntetic example that demonstrates what I mean:


 {{{
 import time
 import sys


 def launch_tor_mock():
     try:
         time.sleep(5)  # emulate bootstrapping
     except:
         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))


 if __name__ == '__main__':
     try:
         launch_tor_mock()
     except KeyboardInterrupt:
         print('Dear user, you\'ve just pressed CTRL+C, don\'t do it!')
     except:
         print('Dear user, some tor process error you couldn\'t prevent
 happend.')
 }}}

 If you'll run it and press CTRL+C, you'll see second msg that's wrong.

 I think we should just reraise error, without thinking what type it is:


 {{{
   except:
     if tor_process:
       tor_process.kill()  # don't leave a lingering process
       tor_process.wait()
     raise  # just reraise exception we got
 }}}

 I understand about docstring ":raises", but you can't forecast any type of
 error code can raise in special cases. KeyboardInterrupt is one of this
 type of "unexpected" exceptions that can be raised anywhere.

 ...

 One another little thing is about line 141:

 {{{
       # this will provide empty results if the process is terminated
       if not init_line:
         raise OSError('Process terminated: %s' % last_problem)
 }}}

 Here was process killing. I'm not sure, but may be better is to keep it?
 At least in case this commit is about another thing...

 {{{
       # this will provide empty results if the process is terminated
       if not init_line:
         if tor_process:  # ... but best make sure
           tor_process.kill()
           tor_process.wait()
         raise OSError('Process terminated: %s' % last_problem)
 }}}

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/17946#comment:2>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online