Re: [PATCH] Re: pconn.cc assert index >= 0, async call queue madness

Henrik Nordstrom wrote:
> So any proposals on how we would go about fixing comm_read_cancel?
>
> Hmm.. on reading that code commio_cancel_callback looks a bit odd in
> trunk. Why is callback set to NULL twice, and what is really the purpose
> of this function now that the actual callback is in an AsyncCall..?

AsyncCalls are refcounted so believed that with "ccb->callback = NULL"
the AsyncCall which the ccb->callback point released. But yes if it is
refcounted by an other object (for example in AsyncCalls queues,....)
it is not released and remains active. Looks that this is the bug.

Propably this callback set to NULL twice because the developer has in
his mind to put something different here :-)

This bug exits only for function based AsyncCalls (the compatible with
the old comm_read interface) and which canceled using the
comm_read_cancel(int fd, IOCB *callback, void *data) function.

Some lines after inside the
   comm_read_cancel(int fd, AsyncCall::Pointer &callback)
the "callback->cancel("comm_read_cancel")" is the first call...

Just one comment:
 Maybe because in the case of
 comm_read_cancel(int fd, AsyncCall::Pointer &callback)
call the asyncCall canceled twice it is better to move the asyncCall
cancelation to
 comm_read_cancel(int fd, IOCB *callback, void *data) function

>
> Is it perhaps as simple as making commio_cancel_callback call
> ccb->callback->cancel()? Testing.. seems to work. Patch attached. Any
> comments?
>
> Most of the AsyncCall code is still a bit alien to me, just starting to
> unwind it...

I think Alex has do excellent work with AsyncCalls. The difficult and
tricky part is the related with the Dialers, but you need to look on
them only once (just for curiosity) and then just use their interface
without bother about them...
Received on Tue Apr 08 2008 - 11:47:45 MDT