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
signature.asc
Description: This is a digitally signed message part
