Re: cbdata semantics, object lifecycles, and cancellation

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

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

Only the owner of the original pointer is allowed to use cbdataFree().
That it also works on reference pointers is just an implementation
detail/accident.

> Take two threads, A and B. Consider A as "squid" and B as "work
> queue". A pushes some work into a work queue which is serviced by B,
> with the understanding that B will complete the work and then schedule
> a completion callback in A. A passes in a cbdata pointer for the work
> to be done. In both single-threade and multi-threaded code, a call to
> cbdataValid() holds true from the point its called to the point where
> another cbdata operation occurs on the code. For single threaded code
> this will generally be until the current threads program flow hits
> another cbdata operation - so you can assume that calling a callback
> immediately after checking cbdataValid() will hold correctly. But with
> two threads, A may schedule a callback which runs on B; A may then
> somewhere call cbdataFree() as part of some error/termination
> condition. A call to cbdataValid() in B only checks the pointer
> validity at call-time and, with the current code, will probably be
> invalid w/out locking or atomic instructions.

In multithreaded mode cbdata needs to be used slightly differently.. but
not much.

cbdataValid() is alays meaningful, and if false you'll know the
reference is invalid and can be used as an optimization shortcircuit
where there is no explicit invalidation of operations. A true result
however isn't useful.

Actuall callbacks needs to be scheduled down to the thread owning the
called object, or per-object thread locks needs to be added for
transferring the object ownership between threads (note: there will be a
automatic no veto from me on any such locking design as it quickly
degrades performance and results in ugly deadlocks). My preffered
mechanism is to post a message with the callback to the object (or
actually it's owning working thread). Very similar to an AsyncCall.

There MUST NOT be more than one thread at any given point in time
claiming ownership of an object.

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

cbdata pointers isn't meant to be usable as pointers, only passed back.
The fact that cbdata pointers have the same type as the original object
is another implementation detail/accident.

cbdata references returned by cbdataReference should really be viewed as
weak "void" references to the object, whos purpose is to locate the
object on a callback if it still exists.

It's abused for some other uses in the code base relying on the
implementation details/accidents, but any such use is a bad sign and
needs to be looked very careful at. Quite likely should be replaced with
true references instead...

Regards
Henrik