Re: [boost] Asio formal review

From: Matthew Vogt (mattvogt_at_[hidden])
Date: 2005-12-13 22:37:28

Hi Chris, thanks for your responses.

> > I don't really follow the intent of the locking dispatcher.
> > It appear to me that it simply achieves the same effect as
> > adding a scoped lock to each dispatched method (each using the
> > same mutex).
> 
> It's not quite the same thing. A scoped lock in each dispatched
> handler means that the method may be blocked while waiting to
> acquire the mutex. A blocked handler means that other handlers
> may also be prevented from executing even though they are ready
> to go.
> 
> The locking_dispatcher ensures that the handler won't even be
> dispatched until the lock is acquired. This means that the
> execution of other handlers can continue unimpeded.

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?

> I'd also like to note here that I intend asio to allow and
> encourage (although not enforce) the development of applications
> without any explicit locking of mutexes.

That's an excellent goal.

> > The locking_dispatcher's 'wrap' method seems to be its primary
> > purpose.  Since the terminology is a little unnatural, perhaps
> > it could be operator() ?
> 
> The wrap() function returns a new function object that wraps the
> provided one. It doesn't actually execute code at that point, so
> I'm not sure operator() would convey that meaning.

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.

> > I would like to see a user-supplied handler option for
> > handling genuine errors, not per-call but at a higher level
> > (maybe per-demuxer?)  Typically, the response to an error is
> > to close the socket,  once open; it would be handy to supply a
> > callback which received enough context to close down a socket,
> > and in all other code to ignore error-handling completely.
> 
> 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?

> I think the go is to use function object composition at the
> point where you start the asynchronous operation, e.g.:
> 
>   async_recv(s, bufs, add_my_error_handling(my_handler));
> 
> or perhaps:
> 
>   async_recv(s, bufs,
>     combine_handlers(error_handler, ok_handler));

Yes, but you don't want to obscure every asynch operation by repeating
the error code handling directions at the call site, if your error
handling will be very similar.  Looking at the examples, I expect that
existing code using asio must contain large portions of replicated
error-handling code, even if the repetition is merely in passing the
same handler over and over.  Perhaps someone who has a significant body
of code using asio could comment?

> Also if the default behaviour you want is to close the socket
> (although often there's an associated application object to be
> cleaned up too) you can simply bind a shared_ptr to the object
> as a parameter to your handlers:
> 
>   async_recv(s, bufs,
>     boost::bind(handler, connection_ptr, ...));

Yes.  Although, if you had a defined error hook receiving a reference to
the socket in error, it would also be easy to map any other dependant
variables or actions to the socket, outside the asio library's
functions.

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

> > I do not think EOF should constitute an error, and I would
> > expect that try-again and would-block errors could be safely
> > ignored by the user.  Perhaps EOF should be a testable state
> > of the socket, rather than a potential error resulting from
> > 'receive'.
> 
> In older versions of asio EOF was indicated by having a read
> return 0, same as the BSD sockets interface. However this
> increased complexity in composed synchronous and asynchronous
> operations, such as the now defunct async_read_n(), which had to
> return both the total number of bytes transferred *and* the last
> bytes transferred to allow checking for EOF.
> 
> Now with EOF as an error the problem can be addressed far more
> clearly and elegantly. For example, consider:
> 
>   asio::read(socket, buffers, asio::transfer_all());
> 
> Its "contract" is to fill the buffers, using multiple underlying
> read_some calls as necessary. If it is unable to fulfill the
> contract it must return within an error indicating why. If the
> stream closed early then that error is EOF.

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.  

Reading an unknown amount of data is a tricky interface question; I
think that I would prefer to test that the socket is still not-at-EOF
before continuing to read, as opposed to checking for EOF return code
after each read.  This all presumes that the error-handling is
configured so that checking the return code for each operation is not
required, to deal with exceptional events.

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.

> > Can the proactor be made to work with multiple event sources,
> > with the current interface?  For example, wrapping
> > simultaneous network IO and aio-based file IO on POSIX?
> 
> Yes, although in the current implementation one of them must be
> relegated to a background thread.

Good.

> > I believe there should be an automatically managed buffer-list
> > class, to provide simple heap-based, garbage-collected buffer
> > management, excluding the possibility of buffer overruns and
> > memory leaks.  Perhaps the one in the Giallo library could be
> > used without significant modification, and supported by the
> > asio buffer primitives. Also, this abstraction should be used
> > in tutorials; although it is useful and practical to support
> > automatically allocated buffer storage, it shouldn't be
> > needlessly encouraged.
> 
> I would rather see such a buffer-list utility developed as a
> separate boost library. As you say, it can integrate with easily
> with asio provided it supports the asio buffer primitives, or if
> it implements the Mutable_Buffers and Const_Buffers concepts.

Ok.  I don't think it is a significant undertaking, but it might require
changes to the existing buffer concepts.  I should look into it :)

> However, I don't agree that automatically allocated buffer
> storage shouldn't be encouraged. No allocation means no leaks,
> predictable memory usage, less fragmentation etc. I.e. it's one
> of the strengths of C++. Furthermore, it may be more appropriate
> in many applications to use a buffer that is a data member of
> some connection class, where the connection class as a whole is
> garbage collected, not the buffer.

Sorry, by 'not encouraging', I mean that they shouldn't be seen as the
first-choice way to use the library.  People developing high
performance, multi-connection servers will not need to be shown that
there are more efficient methods to provide access to data buffers.  The
people who need the best example are those who are using asio as their
first exposure to networking, because it's in that boost thing. These
people should see safe ways to use the library before they encounter
fast ways to use the library.  

The current audience for your tutorials is boost developers, but if
accepted, the tutorials will be read by many more people with less
experience, and I think the tutorials should be targetted at them.

> > I also would like to request some commentary (in the library
> > documentation) on future developments that are out of scope
> > for the current proposal. Given the late juncture at which
> > we're looking at bringing network programming into boost, I
> > think it's important to consider desirable extensions, and how
> > the current proposal will support them.  Much of this will be
> > discussed in the review, of course.
> 
> 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.

> > One thing I would like to see (even in Asio itself) is a
> > simplest-possible layer over the bare sockets layer, which
> > took care of resource management and the intricacies of
> > sockets programming.  Ideally, this would leave an interface
> > comparable to asynch IO programming in python and the like,
> > suitable for the smallest networking tasks.
> 
> I'm not sure what you mean here. Did you mean to say that such a
> layer would leave out asynch I/O? Otherwise if it includes
> asynch I/O then that's the role that asio is already trying to
> fill - i.e. be a thin yet portable asynchronous I/O abstraction.

Well, I shouldn't have asked this, really.  It's hard to look at Asio on
it own merits, without factoring in that it will be the first networking
library in boost, and likely the only one for some time.  New users, and
people needing portable, supported networking functionality will have
Asio as an obvious option, whether or not their code calls for a 'thin
yet portable asynchronous I/O abstraction'.

Whether or not concessions should be made for this, I don't know :)

Matt
-- 
  Matthew Vogt
  mattvogt_at_[hidden]
-- 
http://www.fastmail.fm - IMAP accessible web-mail