On Thu, Sep 20, 2007 at 09:21:29PM -0000, SVN commits to the Asterisk project
wrote:
> Author: mmichelson
> Date: Thu Sep 20 16:21:28 2007
> New Revision: 83350
>
> URL: http://svn.digium.com/view/asterisk?view=rev&rev=83350
> Log:
> Merging changes from queue_refcount_trunk into trunk. Refcounted queues now
> in place.
just randomly looking at the diff, there seem to be a couple
of incorrect patterns of usage of refcounts .This change is rather
extensive so i only point out two examples, but i would encourage
you to look at the ordering of ao2_unref/ao2_unlock and ao2_link/ao2_ref
(both wrong: almost surely you need unlock/unref and ref/link)
in your code.
The first one is shown below (but i have seen at least another instance):
once you release your reference with queue_unref(), you are not
guaranteed that the object exists anymore so you cannot use the
pointer anymore.
> @@ -628,9 +653,9 @@
> }
> ao2_ref(cur, -1);
> }
> - ast_mutex_unlock(&q->lock);
> - }
> - AST_LIST_UNLOCK(&queues);
> + queue_unref(q);
> + ao2_unlock(q);
> + }
>
> return NULL;
> }
A second problem below: ao2_link() implicitly passes the reference
to the container, so you are not guaranteed access to the object
after that. queue_ref() must be done before the ao2_link()
> @@ -1207,11 +1228,12 @@
> if (!q) {
> if (!(q = alloc_queue(queuename)))
> return NULL;
> - ast_mutex_lock(&q->lock);
> + ao2_lock(q);
> clear_queue(q);
> q->realtime = 1;
> init_queue(q); /* Ensure defaults for all parameters
> not set explicitly. */
> - AST_LIST_INSERT_HEAD(&queues, q, list);
> + ao2_link(queues, q);
> + queue_ref(q);
> }
>
> memset(tmpbuf, 0, sizeof(tmpbuf));
there are probably more instances like these two.
cheers
luigi
_______________________________________________
Sign up now for AstriCon 2007! September 25-28th. http://www.astricon.net/
--Bandwidth and Colocation Provided by http://www.api-digital.com--
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
