Re: cbdata semantics, object lifecycles, and cancellation

On Fri, 2008-08-22 at 23:01 +0800, Adrian Chadd wrote:

> I've been staring at the cbdata semantics and use in the Squid-2
> codebase (which translates pretty well to how its used in the Squid-3
> codebase) and I'd like to get peoples' opinions on some stuff.
>
> Generally, Squid's event driven methodology involves registering
> callbacks and using cbdata pointers, then relying on pointer
> validation/invalidation to determine whether to run a callback or not.

AsyncCalls have an explicit call cancellation mechanism. Legacy code
(wrapped in AsyncCalls) should still check cbdata validity, of course,
but that is not the recommended AsyncCall use.

Most (and eventually all?) async calls are done between AsyncJobs. That
common interface makes it easier to check whether the recipient is still
"there" to answer the call. Those internals are relatively easy to
change if we want to migrate to a thread-safe world.

> This means that an owner or anyone else can simply call cbdataFree()
> (which, thankfully, shouldn't happen all that often in the codebase!)
> and it invalidates the callbacks which are registered for this.
>
> This makes using it in threaded code difficult.

<explanation why cbdata is not thread-safe snipped>

> Because of this, there's no guarantee that a cbdata pointer passed
> into another thread will ever really be usable, given the current
> coding methodology.
>
> Comments?

Using cbdata for async calls protection is not going to be relevant in
Squid3 once AsyncCall API is used natively everywhere. Christos already
converted a lot of code and I think we can finish that work, possibly
after the code is split into modules (which will eliminate or make it
easier to eliminate some of the uses).

For other (valid) uses, cbdata API should be converted to a C++ friendly
(and eventually thread-safe) class, possibly using 3rd party thread-safe
code such as various Boost pointers.

BTW, Squid3 refcounting is probably also not thread-safe, but it is
easier to fix when needed because the refcounting interface is already a
class.

$0.02,

Alex.
Received on Mon Aug 25 2008 - 21:01:50 MDT