BIND 10 #2400: notify auth::DataSrcClientsMgr when the builder thread dies

[BIND 10 Development]

#2400: notify auth::DataSrcClientsMgr when the builder thread dies
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:
  jinmei                             |                Status:  new
                       Type:         |             Milestone:  Next-Sprint-
  defect                             |  Proposed
                   Priority:         |            Resolution:
  medium                             |             Sensitive:  0
                  Component:         |           Sub-Project:  DNS
  b10-auth                           |  Estimated Difficulty:  0
                   Keywords:         |           Total Hours:  0
            Defect Severity:  N/A    |
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by vorner):

 Hello

 Replying to [comment:2 jinmei]:
 > Yeah, I know this is suboptimal in terms of the notification timing.
 > But in case you didn't notice it, note the "(every time) allow the
 > application to access the client lists".  So, the main thread will
 > notice it at the next query handling after the builder thread dies.
 > It's still possible that there's been no query between Monday and
 > Friday, so in theory the same worst case scenario could happen.  But
 > in this case the data source is not really used, so it's not really
 > incorrect in terms of externally visible behavior.

 Ah, sorry, my bad, I wasn't paying attention when reading this. I
 misinterpreted it as whenever the list of commands (the queue) is
 accessed,
 which would be whenever the zone (or other zone) would be reloaded again.
 Your
 way makes sense.

 > This is not really safe in its strictest sense.  But the race
 > condition in this case is that the main thread may not immediately
 > notice the change of the boolean.  Since we already made this
 > compromise as discussed above (as long as we use this approach), it
 > shouldn't be a big deal whether the main thread handles one more query
 > than the ideal case before it notices the failure.

 I don't know. I could imagine a system causing some strange failure (eg.
 SIGBUS) when one thread reads some memory location while the other one
 writes.
 Also, I don't know how long period it might happen to be without the
 memory
 barrier which is implicit in the mutex (eg. „flush“ on the caches). Maybe
 I'm
 just too paranoid there, but I consider this area as „Here be dragons and
 other
 beasts of undefined behaviour“.

 > BTW, when hinting omitting the lock, I was thinking it wouldn't be
 > good if we need to acquire one more extra lock in every query handling
 > only for a basically "should not happen" event.  But on second thought
 > we can probably just use the same lock that currently protects the
 > access to the client list map.

 +1 to the same lock.

 > > Hmm, I didn't notice yesterday during the review, but is
 `assert(false)` the
 > > best way? Asserts can be turned off, this probably should have been
 `abort()`.
 > > Should we fast-fix this somehow?
 >
 > As you know this is done.

 Thanks for that.

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