Re: Comm API notes

From: Adrian Chadd <adrian_at_squid-cache.org>
Date: Thu, 11 Sep 2008 10:27:05 +0800

2008/9/11 Alex Rousskov <rousskov_at_measurement-factory.com>:
> * I/O cancellation.
>
> To cancel an interest in a read operation, call comm_read_cancel()
> with an AsyncCall object. This call guarantees that the passed Call
> will be canceled (see the AsyncCall API for call cancellation
> definitions and details). Naturally, the code has to store the
> original read callback Call pointer to use this interface. This call
> does not guarantee that the read operation has not already happen.
> This call guarantees that the read operation will not happen.

As I said earlier, you can't guarantee that with asynchronous IO. The
call may be in progress and not completed. I'm assuming you'd count
"in progress" as "has already happened" but unlike the latter, you
can't cancel it at the OS level.

As long as the API keeps all the relevant OS related structures in
place to allow the IO to complete, and callers to the cancellation
function are prepared to handle the case where the IO is happening
versus has already happened, then i'm happy.

> You cannot reliably cancel an interest in read operation using the old
> comm_read_cancel call that uses a function pointer. The handler may
> get even called after old comm_read_cancel was called. This old API
> will be removed.

I really did think I had fixed removing the pending callbacks from the
callback queue when I implemented this. (Ie, I thought I implemented
enough for the POSIX read/write API but not enough for
overlapped/POSIX IO.) What were people seeing pre-AsyncCalls?

> It is OK to call comm_read_cancel (both old and new) at any time as
> long as the descriptor has not been closed and there is either no read
> interest registered or the passed parameters match the registered
> ones. If the descriptor has been closed, the behavior is undefined.
> Otherwise, if parameters do not match, you get an assertion.
>
> To cancel other operations, close the descriptor with comm_close.

I'm still not happy with comm_close() being used in that way; it seems
you aren't either and are stipulating new user code aborts jobs via
alternative paths.

I'm also not happy with the idea of close handlers to unwind state
associated with it; how "deep" do close handlers actually get? Would
we be better off in the long run by stipulating a more rigid shutdown
process (eg - shutting down a client-side fd would not involve
comm_close(fd); but ConnStateData::close() which would handle clearing
the clientHttpRequests and such, then itself + fd?)

> Raw socket descriptors may be replaced with unique IDs or small
> objects that help detect stale descriptor/socket usage bugs and
> encapsulate access to socket-specific information. New user code
> should treat descriptor integers as opaque objects.

I do agree with this. As Henrik said, this makes Windows porting a bit
easier. There are still other problems to tackle to properly abuse
overlapped IO in any sensible fashion, mostly surrounding IO
scheduling and callback scheduling..

adrian
Received on Thu Sep 11 2008 - 02:27:07 MDT

This archive was generated by hypermail 2.2.0 : Thu Sep 11 2008 - 12:00:12 MDT