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

On Tue, 2008-04-08 at 11:39 +0200, Henrik Nordstrom wrote:

> - There is two events that may happen, 'A' and 'B'.
> - In the processing of one interest of the other is deregistered with
> the silent assumption that after that it won't get invoked, but the
> object it's related to still exists.
> - Both events fire at the same time and gets queued in the async call
> queue.
> - 'A' executes. Deregisters interest in 'B' and assumes it won't get
> called, but it's already in the async call queue.
> - 'B' executes unexpectedly..
>
> There is three ways to deal with this
>
> a) make sure the event interest is registered with a cbdata that gets
> invalidated when the interest is deregistered.
>
> b) shield every such action so that if fails gracefully if it gets
> unexpectedly called due to race conditions with other actions on the
> same object.
>
> c) Make the async call queue always drain between each event processed
> and hope that's sufficient to avoid races (it's not 100%)

I believe there is a better option that was added to handle specifically
to handle these kind of problems:

d) Cancel the call when it is no longer expected.

This is already supported by AsyncCall::cancel() but the old code needs
to make use of it. Canceling a call requires a pointer to the call,
which in some cases slightly increases the RAM footprint of an object.
It has negligible performance overhead. Call pointers can be (and
already are, but usually indirectly) used by the job to maintain its
state.

Option (d) is similar to (a), but does not require rewriting AsyncCalls
and is more explicit/direct, which may help developers with using it
correctly.

It is an open question whether low-level interface like comm may cancel
calls implicitly (i.e., when something happens to FD state and not as a
result of an explicit comm_cancel_* call).

HTH,

Alex.
Received on Tue Apr 08 2008 - 07:47:37 MDT