[tor-bugs] #7666 [Stem]: Support TAKEOWNERSHIP command

[Tor Bug Tracker & Wiki]

#7666: Support TAKEOWNERSHIP command
-------------------------+--------------------------------------------------
 Reporter:  lunar        |          Owner:  atagar      
     Type:  enhancement  |         Status:  needs_review
 Priority:  normal       |      Milestone:              
Component:  Stem         |        Version:              
 Keywords:               |         Parent:              
   Points:               |   Actualpoints:              
-------------------------+--------------------------------------------------

Comment(by atagar):

 > A year or two ago, I would have tried to look for ways to detect that a
 Tor process is ‘local’ to a controller. Now, I don't even know what
 ‘local’ should mean

 I liked your example about tunneled connections. That's something that it
 would be nice for is_localhost() to be able to detect. As for split PID
 namespaces on the same machine, this is the first time I've ever heard of
 those. How does that even work? Does 'ps' show multiple processes with the
 pid? Does kill or other pid based commands affect all processes that share
 that pid?

 Shared PID namespaces sound like a terrible idea that would break... well,
 a lot of things. Guess that's something I should read up on at some point.
 O brave new world that has such things in it...

 Personally I define 'local' as 'I can shell out with commands like ps and
 kill by its pid to affect it'.

 > Ick. This should be wrapped in a launch_captive_tor function

 I disagree. The user may want to set OwningControllerProcess for a process
 that won't have a controller. Having these specialized functions for
 launching then connecting to tor lend a lot of flexibility to stem's users
 (flexibility that blended functions like TorCtl's connect() lacked, and
 often making those functions useless).

 > That's absolutely minor, but dropping the '--' would make it easier to
 spot options that are command-line only and those that are configuration
 options.

 Oops. That's what I get for having a horribly out of date copy of tor's
 man page installed on my system. Changed...

 https://gitweb.torproject.org/stem.git/commitdiff/4cacd2f13d1cd003db0c15addcf265fa4262ee37

 ========================================

 {{{
 config = self.extra_config + [
   ('DataDirectory', self.datadir),
   ('ControlSocket', self.control_socket_path()),
   ('CookieAuthentication', '1'),
   ('CookieAuthFile', self.cookie_auth_file_path()),
   ('SocksPort', 'auto'),
   ('__OwningControllerProcess', str(os.getpid())),
 ]

 args = []
 for key, value in config:
 args += [key, value]

 self.popen = stem.process.launch_tor(
   args=args,
   torrc_path=stem.process.NO_TORRC,
   init_msg_handler=self.handle_init_msg)
 }}}

 Out of curiosity, why are you using launch_tor() rather than
 launch_tor_with_config() for this? It should both make the code nicer, and
 avoid having a huge process name.

 {{{
 self.popen = stem.process.launch_tor_with_config(
   config = self.extra_config + {
     'DataDirectory': self.datadir,
     'ControlSocket': self.control_socket_path(),
     'CookieAuthentication': '1',
     'CookieAuthFile': self.cookie_auth_file_path(),
     'SocksPort': 'auto',
   }.
   init_msg_handler=self.handle_init_msg,
   take_ownership = True,
 )
 }}}

 ========================================

 {{{
 stem.connection.authenticate_cookie(self.controller,
 self.cookie_auth_file_path())
 }}}

 You could replace this with...

 {{{
 self.controller.authenticate()
 }}}

 Stem can figure out that the controller's using cookie authentication and
 where the cookie is located without being told. You could then also drop
 the stem.connection import.

 ========================================

 {{{
 self.pid = int(self.controller.get_info('process/pid'))
 }}}

 You don't need to track this. You have 'self.popen', so this would be
 equiviant to...

 {{{
 self.popen.pid
 }}}

 ========================================

 {{{
 attempts = 10
 while attempts > 0:
   time.sleep(1)
   self.popen.poll()
   attempts -= 1
   try:
     os.kill(self.pid, 0)
   except OSError, err:
     if err.errno == errno.ESRCH:
       break
   if attempts == 5:
     os.kill(self.pid, signal.SIGTERM)
 if attempts == 0:
   os.kill(self.pid, signal.SIGKILL)
   self.popen.poll()
 }}}

 Doing this with a poll based approach will make your __exit__() a bit
 shower than it needs to be. Are you using python 2.6 or later? If so then
 this will do the trick...

 {{{
 self.popen.kill()
 process.communicate() # block until it's gone
 }}}

 ... and if it's python 2.5 then...

 {{{
 os.kill(self.popen.pid, signal.SIGTERM)
 process.communicate() # block until it's gone
 }}}

 That's what I do for stem's integ test runner and I've never seen it have
 a problem. If you're really paranoid then you could throw in an alarm to
 time out the kill if it blocks (shouldn't be needed, but to find an
 example look for 'signal' in stem/process.py).

 ========================================

 {{{
 c = pycurl.Curl()
 c.setopt(pycurl.URL, url)
 c.setopt(pycurl.PROXY, socks_host)
 ...
 }}}

 Neat, I wasn't familiar with pycurl. Do you know if it has been added to
 the builtin library? It could be handy for future tests.

 I definitely need to invest some time in making stem friendlier for these
 sort of client use cases, preferably with an example (that's what #7505 is
 for).

 Let me know if you have any ideas that would make stem friendlier to use!

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