Re: [boost] Asio formal review

From: Christopher Kohlhoff (chris_at_[hidden])
Date: 2005-12-14 19:37:10

Hi Matt,

--- Matthew Vogt <mattvogt_at_[hidden]> wrote:
> Ok, I see now.  In retrospect, this is adequately described in the
> tutorial, but the tutorial itself doesn't take advantage of the
> improvement.  Perhaps a more complicated example demonstrating
> this advantage would be useful?

Yep, fair enough.

<snip>
> Yes, I see your objection, although in terms of user code, it
> seems a little artificial. I would suggest 'serialise' as
> another name for 'wrap' - but not unless people other than me
> find it obtuse.

This name is part of the wider Dispatcher concept and so can be
implemented by things other than locking_dispatcher -- e.g. it
is also on the demuxer class. The name 'wrap' didn't feel
perfect when I first chose it either, but it has grown on me.

<snip>
> > I'm not convinced that this belongs as part of asio's interface,
> > since there are a multitude of ways to handle errors. For
> > example, there's the issue of what happens in async operations,
> > such as asio::async_read, that are composed of other async
> > operations. You wouldn't want the application-specific error
> > handling to be invoked until the composed operation had finished
> > what it was doing.
> 
> Sorry, can you give a code example of what you're describing here?

One possible example: consider a high level async function that
receives a complex data structure using multiple underlying
asynchronous operations:

  async_read_my_data(Stream& s, my_data& d, Handler h)
  {
    // Start read of first bit of message.
    asio::async_read(s, ...buffers...,
        boost::bind(my_data_handler_1, _1, s, d, h)); 
  }

  void my_data_handler_1(error& e, Stream& s,
                         my_data& d, Handler h)
  {
    if (e)
    {
      // Need to clean up stuff added to my_data here.

      // Call application handler
      h(e);
    }
    else
    {
      // Start read of second bit of message.
      asio::async_read(s, ...buffers...,
          boost::bind(my_data_handler_2, _1, s, d, h)); 
    }
  }

  ...

  void my_data_handler_N(error& e, Stream& s,
                         my_data& d, Handler h)
  {
    if (e)
    {
      // Need to clean up stuff added to my_data here.
    }

    // Call application handler
    h(e);
  }

Let's say that this function guarantees that, on failure, any
intermediate data is removed from the my_data structure.
On the flip side, the caller is required to guarantee that the
my_data structure is valid until the handler is called.
Therefore this composed async operation must be able to ensure
that the application handler is not called until it has finished
with the my_data structure.

<snip>
> > Essentially the object is owned by the chain of operations.
> > When the chain of async operations and their handlers is
> > terminated due to an error (or any other condition) the
> > object is cleaned up automatically.
> 
> Sorry, I don't follow this remark.  The tutorial and example
> code seems to show explicit socket close in error
> conditions...

For example:

class connection
  : public enable_shared_from_this<connection>
{
private:
  stream_socket socket_;
public:
  ...
  void start()
  {
    // start reading some data.
    socket_.async_read_some(buffers,
      boost::bind(&connection::handler, shared_from_this(), _1));
  }

  void handler(error& e)
  {
    if (!e)
    {
      // process data, then read some more.
      socket_.async_read_some(buffers,
        boost::bind(&connection::handler, shared_from_this(), _1));
    }
  }
};

The connection object is automatically destroyed, and the socket
closed, when there are no more operations associated with it.

<snip>
> Yes, I see your point.  However, I'm looking at this from the
> point of view that I would prefer errors to be handled out of
> the normal code path, and for the handling to be as global as
> possible.  With error-handling set up this way, you don't want
> EOF to be an error, since it is part of the normal sequence of
> events rather than an exceptional event.  

EOF is only in the normal sequence of events for some protocols,
and even then often only at certain points in the protocol. E.g.
with HTTP you can have responses terminated by EOF, but an early
EOF is an error if a content length is specified. Therefore I
prefer it to be explicitly handled in those cases, rather than
the other way around.

<snip>
> The other non-exceptional errors are try-again and
> would-block, right? I think these can be ignored entirely,
> since the bytes_transferred will give the correct information
> to enable another read/write operation to initiated.

At some point I need to document all the possible errors for
each operation, since I don't believe these examples are widely
applicable, especially not with asynchronous operations. In
general no errors should be ignored.

<snip>
> > What sort of things did you have in mind?
> 
> Basically, a 'Rationale' section and a 'Future Development'
> section similar to that in the (to pick the first that come to
> mind) Serialization library and the Iostreams library.  The
> content could all be cropped from review discussions, since
> the questions raised now will be questions for later users as
> well.

Actually I meant what sort of ideas did you have for future
development, so I can add them to the list? :)

<snip>

Cheers,
Chris