BIND 10 #1600: Use UDPSyncServer for b10-auth

[BIND 10 Development]

#1600: Use UDPSyncServer for b10-auth
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  kevin_tes
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120320
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  b10-auth                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  5
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:  auth   |
  performance                        |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:22 muks]:

 > > Regarding the code, it's not exception safe: If message->toWire()
 > > throws in the middle of it, renderer_ will keep remembering the
 > > previously set buffer.  In the version of the base for trac1600 it
 > > doesn't matter much because the process will immediately die anyway.
 > > But the latest code catch the condition and continue to the next
 > > message.  So once this branch is merged this bug becomes more
 > > critical.
 >
 > kevin_tes: Was MessageRendererHandle introduced to address this issue?

 I suspect so, but if so, it doesn't seem to address my points (btw,
 messagerendererhandle.h has many style issues).

 What I envisioned was actually something simpler like this:

 {{{#!c++
 // This helper class holds a MessageRenderer for a limited scope, ensuring
 // any used resource is cleared on exit from the scope (whether its normal
 // return or escape due to an exception)
 class MessageRendererHolder {
 public:
     MessageRendererHolder(AbstractMessageRenderer& renderer) :
         renderer_(renderer)
     {}
     ~MessageRendererHolder() {
         renderer_.clear();
     }
 private:
     AbstractMessageRenderer& renderer_;
 };
 }}}

 {{{#!c++
     ...
     MessageRendererHolder renderer_holder(renderer_);
     renderer_.setBuffer(buffer.get());
     const bool udp_buffer =
         (io_message.getSocket().getProtocol() == IPPROTO_UDP);
     renderer_.setLengthLimit(udp_buffer ? remote_bufsize : 65535);
     if (tsig_context.get() != NULL) {
         message->toWire(renderer_.getRenderer(), *tsig_context);
     } else {
         message->toWire(renderer_.getRenderer());
     }
     renderer_.setBuffer(NULL);
     LOG_DEBUG(auth_logger, DBG_AUTH_MESSAGES, AUTH_SEND_NORMAL_RESPONSE)
               .arg(renderer_.getLength()).arg(message->toText());
 }}}

 and, the kind of tests we should add is to have the server respond to
 two queries.  Using a faked broken data source, the first one would
 result in an exception in Message:toWire().  We'll then confirm the
 second one succeeds and the response doesn't contain any leftover from
 the first query.

 If it's still not clear, I'm happy to provide a suggested patch.

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