[asterisk-dev] Astobj2 and locks

[Tilghman Lesher]

On Wednesday 12 September 2007 18:29:19 Watkins, Bradley wrote:
> I'm working on some pretty big changes to app_queue for internal use (if
> I think it'll end up remotely useful to the community, I'll be certain
> to submit a patch), and it looks like the features of astobj2 are
> exactly what I need.
>
> However, I'm trying to understand its use, and I've either found some
> bugs in existing usage or I'm missing something (I don't rule out the
> latter in any way).
>
> When iterating through a container using ao2_iterator_next, it seems as
> though you should need to wrap any changes to the returned object with
> calls to ao2_lock() and subsequently ao2_unlock() after the changes are
> complete.
>
> But here is a snippet from app_queue.c in branches/1.4:
>
> mem_iter = ao2_iterator_init(q->members, 0);
> while ((m = ao2_iterator_next(&mem_iter))) {
> 	q->membercount++;
> 	if (m->realtime)
> 	m->dead = 1;
> 	ao2_ref(m, -1);
> }
>
>
> I would expect (based on my cursory glance at the implementation in
> astobj2.c) that it should look something like:
>
> mem_iter = ao2_iterator_init(q->members, 0);
> while ((m = ao2_iterator_next(&mem_iter))) {
> 	q->membercount++;
> 	if (m->realtime) {
> 		if(ao2_lock(m)) {
> 			m->dead = 1;
> 			ao2_unlock(m);
> 		}
> 	}
> 	ao2_ref(m, -1);
> }

I'm not sure how that would make any difference.  What you're doing is
assigning the value 1 to a particular spot within the member structure.
Whether you do that while it's locked or not is irrelevant.  If you were
_incrementing_ m->dead, then you might have a point (specifically,
because that operation is generally a load, add, and store, where there
is a possible race condition).  I'm discounting that we have a function that
does an atomic increment for illustrative purposes.

Remember that since you have a reference to the member structure, it
is impossible for the member structure to disappear, so setting a value to
1 is still safe.  Only when you decrement the reference count is it possible
for the member to be deallocated (and we only do that after we're done
twiddling the bits).

-- 
Tilghman