Re: [boost] Asio formal review

From: Matt Vogt (mattvogt_at_[hidden])
Date: 2005-12-19 18:04:05

Christopher Kohlhoff <chris <at> kohlhoff.com> writes:

> > 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.

Ok, I withdraw my objections :)

> > > 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?
> 

<snip example, showing error handling varying slightly during a linked series
of dependent IO operations>

> 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.

Ok, here's what I would like to see.

First, I want to supply a handler for errors, to the demuxer.  I want this
handler to be called back when required, with error code that occurred, and 
the socket to which error pertains.

Something like
void my_error_handler(asio::error e, shared_ptr<socket> s)
{
  // clean up resources associated with this socket
}

Then I want to ignore error handling completely in mainline code.

If I have a chain of interconnected operations, I can have a map<socket_id,
my_operation_data> to which I add the context required to revert an operation
which fails after partial completion.  I can use this map to free any 
resources or to revert any state changes in my_error_handler, or I can remove
the context when the entire operation completes.

Of course, not all errors pertain to sockets, but the idea can be generalised.

> > > 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.

Well, that's subtle.  I'm not sure if it's a good idea or not.

> 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.

In this case, the situation can still be handled explicitly by the caller, but
they would make their decision based on socket.eof(), rather than inspection of
a resulting error-code.

> > 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.

Ok, that's preferable.

I think both of the currently available error-handling options are inferior. 
C-style error return codes have long been shown to be inadequate, and the
exception throwing option is rendered almost useless because the thrown object
is an error code.  This means you essentially need a catch for every operation
in order to know the context, and it can't be used for aynch operations.  If it
were a pair of error-code and socket, and could be used asynchronously, it 
would be useful, but propagating the exception outside demuxer::run seems
messy, since multiple threads could be calling 'run'.

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

Well, socketstreams are what people obviously want.  I've never used one, so
I don't know how they play out in the real world, unfortunately.

One idea that seems useful is the compile-time stream composition Hugo Duncan
implemented in his giallo library.  It allows composing sequences of data
handlers-and-possibly-transformers, similar to the Boost Iostreams Library. 
You can see an example of it here: http://tinyurl.com/cmfby

[Split for gmane: <http://cvs.sourceforge.net/viewcvs.py/giallo/giallo/
libs/net/example/http_server.cpp?rev=1.10&view=auto> ]

Of course, these things need to be tested with real code to show their actual
value.

Matt