[tor-bugs] #2812 [Torctl]: TorCtl Zombie Connections

[Tor Bug Tracker & Wiki]

#2812: TorCtl Zombie Connections
--------------------+-------------------------------------------------------
 Reporter:  atagar  |          Owner:  mikeperry
     Type:  defect  |         Status:  assigned 
 Priority:  normal  |      Milestone:           
Component:  Torctl  |        Version:           
 Keywords:          |         Parent:           
   Points:          |   Actualpoints:           
--------------------+-------------------------------------------------------
Changes (by atagar):

  * status:  needs_review => assigned
  * owner:  => mikeperry


Comment:

 Mike and I discussed this on irc. This should be simple to fix though
 there's still some mystery around why adding the shutdown effects the
 _thread. I might take another pass at this if Mike doesn't beat me to it.

 17:32 < atagar> mikeperry: are you confusing the connect with the
 pathsupport connect?
 17:32 < atagar> in torctl it doesn't raise an exception:
 https://gitweb.torproject.org/pytorctl.git/blob/HEAD:/TorCtl.py
 17:33 < atagar> er, meant line 129
 17:33 < mikeperry> it doesn't but authenticate does
 17:34 < mikeperry> ah but that is caught
 17:34 < atagar> right
 17:34 < mikeperry> ah, I was just confused by your example then
 17:34 < atagar> (in retrospect we should change that)
 17:34 < mikeperry> conn is none for me there, but you didn't indicate that
 17:35 < atagar> I didn't show the return result in the example (nore is it
 presented if none). That was copy-and-paste from the interpretor.
 17:35 < mikeperry> I think just closing the socket is not enough, we
 should also be closing the thread
 17:36 < atagar> you mean for the deadlock issue discussed at the end?
 17:37 < mikeperry> yeah
 17:43 < atagar> Sounds good. Would you still like another patch for the
 fixes-this-but-causes-deadlock against the current git master?
 17:45 < atagar> In looking at the arm close function this sounds perfect
 (http://pastebin.com/TVCU6Ntk). The only reason I'm joining on _thread is
 to prevent the syshook error from an orphaned thread during shutdown.
 17:46 < atagar> (though I'm still not sure why shutting down the socket
 causes _thread to never terminate as it did previously)
 17:54 < mikeperry> did you conn.close() there without the socket shutdown?
 17:57 < atagar> yes - is it expected that the caller is doing that?
 17:58 < mikeperry> no. I think it should be part of the fix if we're going
 to return none from connect
 17:59 < mikeperry> that should handle thread shutdown.. though I wonder if
 it is because of the socket.error issue that it doesn't
 17:59 < mikeperry> there should be no need to directly close the socket if
 conn.close() is properly called
 17:59 < atagar> There's two issues here - the connect, when it fails,
 should be closing the socket. But even when it succeeds calling close on
 the TorCtl object should be shutting down the socket - no?
 18:01 < atagar> (and, ideally, terminating _thread)
 18:01 < mikeperry> close() should be terminating _thread
 18:01 < atagar> s/terminating/joining on
 18:10 < atagar> mikeperry: Anything you want from me? I can take another
 stab at the deadlock issue in a bit if you want (I wasn't sure if there
 was a reason for _thread being left alone, hence leaving this to you).
 18:21 < mikeperry> atagar: _thread should terminate on its own, printing
 "Closed control connection. Exiting thread."
 18:21 < mikeperry> it is set to be a daemon thread, so I *think* this
 means it is a detatched pthread underneath and does not need joining
 18:22 < mikeperry> but nickm wrote the initial threading model here
 18:22 < atagar> hm, I'm suspecting that even daemon threads need to be
 terminated before the python process is shut down to avoid syshook errors
 18:23 < mikeperry> that is possible, but the python docs certainly seem to
 make out like they are meant to be allowed to be running at shutdown
 18:25 < atagar> Yup, sounds like. I'm not quite sure what was up with that
 syshook error then. :/

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