BIND 10 #2861: synchronization between auth main thread and datasrc builder

[BIND 10 Development]

#2861: synchronization between auth main thread and datasrc builder
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  vorner
            Priority:  medium        |                       Status:
           Component:  b10-auth      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130723
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  4             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  shared memory data source
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by pselkirk):

 * owner:  pselkirk => vorner


Comment:

 First a nit: When you changed `Command` from pair to struct, you removed
 the documentation about the members:
 {{{
 -/// The first element of the pair is the command ID, and the second
 element
 -/// is its argument.  If the command doesn't take an argument it should
 be
 -/// a null pointer.
 }}}

 It's a minor point, but I like having documentation, especially when
 arguments are optional.

 In `run()`, I'm a bit more concerned about the untested code that
 terminates on wake error. When I added a `close(write_end)` to the test
 code to trigger this, I got
 {{{
 terminate called without an active exception
 make[5]: *** [check-local] Aborted (core dumped)
 }}}

 It seems that it would be better to throw an exception, which would be
 immediately caught and logged by the catch block. The program would still
 terminate, but with something like
 {{{
 terminate called after throwing an instance of 'isc::Unexpected'
   what():  failed to wake up main thread
 }}}

 Finally, I understand the purpose of `FDGuard`, but I don't understand why
 you don't just put the `close()` calls in the `DataSrcClientsMgrBase`
 destructor, or in `cleanup()`. Why do you need an extra object for this?

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