BIND 10 #2712: Cmdctl shutdown command does not shut down b10-cmdctl

[BIND 10 Development]

#2712: Cmdctl shutdown command does not shut down b10-cmdctl
-------------------------------------+-------------------------------------
            Reporter:  jelte         |                        Owner:
                Type:  defect        |  jinmei
            Priority:  medium        |                       Status:
           Component:  cmd-ctl       |  accepted
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130423
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  6             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 trac2712 is ready for review.

 This was a simple deadlock (-like situation).

 The 'Cmdctl shutdown' command from bindctl is handled in a separate
 thread specifically handling the corresponding HTTP session, and if
 the module is Cmdctl itself, it calls the `ModuleCCSession`'s
 command_handler() callback directly.  For the shutdown command it
 triggers "send_stopping", which eventually causes cc.Session.sendmsg()
 for the CC session of the `ModuleCCSession`.  This will require
 acquiring a lock for the CC session.

 On the other hand, another thread is normally sleeping on that CC
 session for read events, holding that lock.

 So, unless the sleeping thread is woken up by receiving a read event
 (receiving a command from other module, etc), it effectively hangs.

 Note that this shouldn't happen if the shutdown command was sent from
 the Init module, because it's handled by the right thread.

 Of course, the fundamental issue is, as has been the case, the naive
 use of threads: creating a threaded application with an (seemingly) ad
 hoc design, sharing various kinds of data without much consideration
 of thread related issues.  We need a fundamental overhaul here, and
 while I hope we'll actually have time for it some day, that's
 certainly too big for this ticket.  My suggested fix is to send all
 commands to msgq and have it send back to Cmdctl if the command is for
 Cmdctl itself.  That's redundant in some sense but should work, and
 coding-wise the change is quite small.

 I've implemented this fix with some additional tests (including a new
 lettuce test to confirm the revised behavior at the system level).  I
 also added an assertion check in command_handler in case we introduce
 a similar type of bug in future.

 Proposed changelog entry:
 {{{
 606.?   [bug]           jinmei
         b10-cmdctl: corrected a hangup problem on receiving the shutdown
         command from bindctl.  Note, however, that cmdctl is defined as
         a "needed" module by default, so shutting down cmdctl would cause
         shutdown of the entire BIND 10 system anyway, and is therefore
         still not very useful in practice.
         (Trac #2712, git TBD)
 }}}

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