BIND 10 #352: Improve Xfrout request handling strategy

[BIND 10 Development]

#352: Improve Xfrout request handling strategy
------------------------------+---------------------------------------------
      Reporter:  zzchen_pku   |        Owner:  zhanglikun
          Type:  enhancement  |       Status:  reviewing 
      Priority:  major        |    Milestone:            
     Component:  xfrout       |   Resolution:            
      Keywords:               |    Sensitive:  0         
Estimatedhours:  0.0          |        Hours:  0         
      Billable:  1            |   Totalhours:  1.5       
      Internal:  0            |  
------------------------------+---------------------------------------------
Changes (by jinmei):

  * owner:  jinmei => zhanglikun


Comment:

 Replying to [comment:6 zhanglikun]:
 > The lastest change has been committed, see r3149, please have a review.
 thanks.

 First, there are some redundant white spaces after EOLs.  I've removed
 them in r3167.

 Second, now that we've seen real race conditions (Trac #335), I've re-
 reviewed the entire code more carefully, and have had additional concerns.
 Revised review comments with the new issues follow:
  - I don't think documentation is sufficient yet.  it still doesn't talk
 about why and when we need this mixin; it doesn't explain the seeming
 assumption of using threads (in the description of the ServeMixIn class).
 I suggest you refer to socketserver.py and provide that level of details.
  - I still don't see why we need to give a specific default value (i.e.
 0.5) to poll_interval.  since it's not used, I'd specify None.
  - this operation seems to be redundant:
 {{{
         self.__is_shut_down.clear()
 }}}
   because at this point !__is_shut_down should always be cleared.
  - do we need self.!__serving?  in which case do we need it even if we
 already have synchronization mechanisms?  note also that accessing
 self.!__serving isn't protected and can cause a race condition (consider,
 e.g. the case shutdown() is called immediately after invoking the thread,
 then serve_forever() is called from the thread.  !__serving will never be
 False.).
  - I'm afraid this raise is dangerous:
 {{{
             except select.error as err:
                 if err.args[0] != EINTR:
                     raise
 }}}
   consider the case where shutdown is called and the parent thread is
 waiting on !__is_shut_down.  what if the above raise is triggered under
 this condition?  doesn't it cause a deadlock?

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