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 kevin_tes):

 Replying to [comment:23 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.

 Jinmei,thanks a lot for the efforts, yes it is clear.But as you comment
 23, this will be done later.

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