BIND 10 #335: b10-xfrout not exiting

[BIND 10 Development]

#335: b10-xfrout not exiting
-------------------------+--------------------------------------------------
      Reporter:  jreed   |        Owner:  zhanglikun
          Type:  defect  |       Status:  reviewing 
      Priority:  major   |    Milestone:            
     Component:  xfrout  |   Resolution:            
      Keywords:          |    Sensitive:  0         
Estimatedhours:  0.0     |        Hours:  0         
      Billable:  1       |   Totalhours:  0         
      Internal:  0       |  
-------------------------+--------------------------------------------------
Changes (by vorner):

  * owner:  vorner => zhanglikun


Comment:

 Replying to [comment:18 zhanglikun]:
 > This is the reply for the change in  r3200.
 >
 > /trac335/src/bin/xfrout/xfrout.py.in :
 >
 > 1. I don't know why you want to remove the following lines code. there
 will be 3 or more
 > threads when xfrout is running, they are mainthread, notify-out thread
 and unix-socket_-
 > server thread, and some other threads which are doing zone transfer.

 Because they are unnecessary, as much as I'm avare. .join() just waits for
 the thread to finish, does not terminate it. So the code just waited for
 everything to terminate. But as the threads are not daemon threads, the
 python interpretter will wait for them anyway. Or I missed something?

 > 2. some function string doesn't follow the standard way.
 >
 >   see the old one is
 {{{
   """ This the comments line1
   this is the comments line2
   """
 }}}
 >
 > has been changed as
 >
 {{{
   """
   This the comments line1
   this is the comments line2
   """
 }}}

 Is it a standard one? I read that the terminating tripple-quotemark should
 be on separate line and it looked more symetric/aestetical to have the
 first one on separate line as well. I do not mind changing it back, if it
 is more usual.

 > the variable self._started_event is used by another function
 self._despatcher().
 >    self._started_event = threading.Event()

 I need a new one every time I start the thread (some tests start it
 multiple times). Furthermore, I'm not sure how expensive .set() is, it
 might include some synchronisation, locking, etc. So I remove the event
 when it is no longer needed and the loop ignores it.

 Another way would be to just set it each iteration, and before starting
 the thread again, .clear() it. Do you think this way is better?

 > 2. do we really need do the clean up in shutdown()?

 Yes, we need to clean the socket. We write b'shutdown' to it and never
 read it, just terminate. If we start it again (like from tests), we would
 get a busyloop, it would be readable all the time, but self._serving would
 still be true. So removing the socket and throwing it away with the data
 inside is needed (we could take the work and read it in non-blocking way,
 but that would be longer). Furthermore, freeing some resources (2 file
 descriptors, to start with) is a nice sideeffect.

 The thread is cleaned up only for consistency, it will not be needed any
 more anyway.
 > I find you have mentioned self._read_sock maybe be empty in some test,
 but I doesn't find
 > it in the test case.

 Some test runs some internal functions (_wait_for_notify_reply) directly,
 without calling dispatcher() (and starting a thread). Therefore the
 _read_sock is on default None value. It is just a sideeffect of running
 only part of the machinery, not that the test would set it on purpose.

 {{{
       if min_timeout is not None:
 }}}

 Because min_timeout might be 0 legally (and 0 is considered False) in
 which case we would replace it with tmp_timeout, but that might be higher
 than 0.

 Do you think I should modify anything else except the comment strings (I'm
 getting to it soon)? Put some of these explanations to comments? Or, do
 you think I'm wrong in any of them?

 Thank you

-- 
Ticket URL: <http://bind10.isc.org/ticket/335#comment:20>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development