[asterisk-commits] kpfleming: branch 1.4 r222152 - in /branches/1.4: apps/ channels/ include/ast...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Mon Oct 5 20:16:39 CDT 2009
Author: kpfleming
Date: Mon Oct 5 20:16:36 2009
New Revision: 222152
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=222152
Log:
Fix ao2_iterator API to hold references to containers being iterated.
See Mantis issue for details of what prompted this change.
Additional notes:
This patch changes the ao2_iterator API in two ways: F_AO2I_DONTLOCK
has become an enum instead of a macro, with a name that fits our
naming policy; also, it is now necessary to call
ao2_iterator_destroy() on any iterator that has been
created. Currently this only releases the reference to the container
being iterated, but in the future this could also release other
resources used by the iterator, if the iterator implementation changes
to use additional resources.
(closes issue #15987)
Reported by: kpfleming
Review: https://reviewboard.asterisk.org/r/383/
Modified:
branches/1.4/apps/app_queue.c
branches/1.4/channels/chan_iax2.c
branches/1.4/include/asterisk/astobj2.h
branches/1.4/main/astobj2.c
branches/1.4/res/res_musiconhold.c
Modified: branches/1.4/apps/app_queue.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/apps/app_queue.c?view=diff&rev=222152&r1=222151&r2=222152
==============================================================================
--- branches/1.4/apps/app_queue.c (original)
+++ branches/1.4/apps/app_queue.c Mon Oct 5 20:16:36 2009
@@ -578,7 +578,7 @@
return QUEUE_NORMAL;
}
}
-
+ ao2_iterator_destroy(&mem_iter);
ast_mutex_unlock(&q->lock);
return result;
}
@@ -634,6 +634,7 @@
}
ao2_ref(cur, -1);
}
+ ao2_iterator_destroy(&mem_iter);
ast_mutex_unlock(&q->lock);
}
AST_LIST_UNLOCK(&queues);
@@ -924,6 +925,7 @@
}
ao2_ref(mem, -1);
}
+ ao2_iterator_destroy(&mem_iter);
ast_mutex_unlock(&q->lock);
if (ret)
break;
@@ -1182,6 +1184,7 @@
}
ao2_ref(cur, -1);
}
+ ao2_iterator_destroy(&mem_iter);
}
static void destroy_queue(struct call_queue *q)
@@ -1291,6 +1294,7 @@
m->dead = 1;
ao2_ref(m, -1);
}
+ ao2_iterator_destroy(&mem_iter);
while ((interface = ast_category_browse(member_config, interface))) {
rt_handle_member_record(q, interface,
@@ -1312,6 +1316,7 @@
}
ao2_ref(m, -1);
}
+ ao2_iterator_destroy(&mem_iter);
ast_mutex_unlock(&q->lock);
@@ -1362,6 +1367,7 @@
m->dead = 1;
ao2_ref(m, -1);
}
+ ao2_iterator_destroy(&mem_iter);
while ((interface = ast_category_browse(member_config, interface))) {
rt_handle_member_record(q, interface,
@@ -1383,6 +1389,7 @@
}
ao2_ref(m, -1);
}
+ ao2_iterator_destroy(&mem_iter);
ast_mutex_unlock(&q->lock);
ast_config_destroy(member_config);
}
@@ -1782,6 +1789,7 @@
break;
}
}
+ ao2_iterator_destroy(&mem_iter);
return avl;
}
@@ -2873,6 +2881,7 @@
struct ast_dialed_interface *di;
AST_LIST_HEAD(, ast_dialed_interface) *dialed_interfaces;
if (!tmp) {
+ ao2_iterator_destroy(&memi);
ao2_ref(cur, -1);
ast_mutex_unlock(&qe->parent->lock);
if (use_weight)
@@ -2881,6 +2890,7 @@
}
if (!datastore) {
if (!(datastore = ast_channel_datastore_alloc(&dialed_interface_info, NULL))) {
+ ao2_iterator_destroy(&memi);
ao2_ref(cur, -1);
ast_mutex_unlock(&qe->parent->lock);
if (use_weight)
@@ -2890,6 +2900,7 @@
}
datastore->inheritance = DATASTORE_INHERIT_FOREVER;
if (!(dialed_interfaces = ast_calloc(1, sizeof(*dialed_interfaces)))) {
+ ao2_iterator_destroy(&memi);
ao2_ref(cur, -1);
ast_mutex_unlock(&qe->parent->lock);
if (use_weight)
@@ -2927,6 +2938,7 @@
* it won't call one that has already been called. */
if (strncasecmp(cur->interface, "Local/", 6)) {
if (!(di = ast_calloc(1, sizeof(*di) + strlen(cur->interface)))) {
+ ao2_iterator_destroy(&memi);
ao2_ref(cur, -1);
ast_mutex_unlock(&qe->parent->lock);
if (use_weight)
@@ -2962,6 +2974,7 @@
free(tmp);
}
}
+ ao2_iterator_destroy(&memi);
if (qe->expire && (!qe->parent->timeout || (qe->expire - now) <= qe->parent->timeout))
to = (qe->expire - now) * 1000;
else
@@ -3330,10 +3343,13 @@
mem_iter = ao2_iterator_init(q->members, 0);
while ((mem = ao2_iterator_next(&mem_iter))) {
- if (!strcasecmp(interface, mem->interface))
+ if (!strcasecmp(interface, mem->interface)) {
+ ao2_iterator_destroy(&mem_iter);
return mem;
+ }
ao2_ref(mem, -1);
}
+ ao2_iterator_destroy(&mem_iter);
return NULL;
}
@@ -3375,6 +3391,7 @@
}
value_len += res;
}
+ ao2_iterator_destroy(&mem_iter);
if (value_len && !cur_member) {
if (ast_db_put(pm_family, pm_queue->name, value))
@@ -4228,6 +4245,7 @@
}
ao2_ref(m, -1);
}
+ ao2_iterator_destroy(&mem_iter);
ast_mutex_unlock(&q->lock);
} else
ast_log(LOG_WARNING, "queue %s was not found\n", data);
@@ -4326,6 +4344,7 @@
}
ao2_ref(m, -1);
}
+ ao2_iterator_destroy(&mem_iter);
ast_mutex_unlock(&q->lock);
} else
ast_log(LOG_WARNING, "queue %s was not found\n", data);
@@ -4458,6 +4477,7 @@
}
ao2_ref(cur, -1);
}
+ ao2_iterator_destroy(&mem_iter);
for (var = ast_variable_browse(cfg, cat); var; var = var->next) {
if (!strcasecmp(var->name, "member")) {
struct member tmpmem;
@@ -4536,6 +4556,7 @@
remove_from_interfaces(cur->state_interface);
ao2_ref(cur, -1);
}
+ ao2_iterator_destroy(&mem_iter);
if (q->strategy == QUEUE_STRATEGY_ROUNDROBIN)
rr_dep_warning();
@@ -4564,6 +4585,7 @@
cur->status = ast_device_state(cur->state_interface);
ao2_ref(cur, -1);
}
+ ao2_iterator_destroy(&mem_iter);
ast_mutex_unlock(&q->lock);
}
}
@@ -4686,6 +4708,7 @@
ast_cli(fd, " %s%s%s", mem->membername, max_buf, term);
ao2_ref(mem, -1);
}
+ ao2_iterator_destroy(&mem_iter);
} else if (s)
astman_append(s, " No Members%s", term);
else
@@ -4828,6 +4851,7 @@
}
ao2_ref(mem, -1);
}
+ ao2_iterator_destroy(&mem_iter);
/* List Queue Entries */
pos = 1;
for (qe = q->head; qe; qe = qe->next) {
@@ -4845,6 +4869,7 @@
S_OR(qe->chan->cid.cid_name, "unknown"),
(long) (now - qe->start), idText);
}
+ ao2_iterator_destroy(&mem_iter);
}
ast_mutex_unlock(&q->lock);
}
@@ -5120,6 +5145,7 @@
while ((m = ao2_iterator_next(&mem_iter))) {
if (++which > state) {
char *tmp;
+ ao2_iterator_destroy(&mem_iter);
ast_mutex_unlock(&q->lock);
tmp = ast_strdup(m->interface);
ao2_ref(m, -1);
@@ -5127,6 +5153,7 @@
}
ao2_ref(m, -1);
}
+ ao2_iterator_destroy(&mem_iter);
ast_mutex_unlock(&q->lock);
}
}
Modified: branches/1.4/channels/chan_iax2.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/channels/chan_iax2.c?view=diff&rev=222152&r1=222151&r2=222152
==============================================================================
--- branches/1.4/channels/chan_iax2.c (original)
+++ branches/1.4/channels/chan_iax2.c Mon Oct 5 20:16:36 2009
@@ -1325,6 +1325,7 @@
}
peer_unref(peer);
}
+ ao2_iterator_destroy(&i);
if (!peer) {
peer = realtime_peer(NULL, &sin);
@@ -2132,6 +2133,7 @@
}
ao2_ref(peercnt, -1);
}
+ ao2_iterator_destroy(&i);
if (argc == 4) {
ast_cli(fd, "\nNon-CallToken Validation Limit: %d\nNon-CallToken Validated: %d\n", global_maxcallno_nonval, total_nonval_callno_used);
} else if (argc == 5 && !found) {
@@ -3276,6 +3278,7 @@
}
peer_unref(peer);
}
+ ao2_iterator_destroy(&i);
return res;
}
@@ -4870,6 +4873,7 @@
}
peer_unref(peer);
}
+ ao2_iterator_destroy(&i);
return res;
}
@@ -5700,6 +5704,7 @@
user->contexts ? user->contexts->context : context,
user->ha ? "Yes" : "No", pstr);
}
+ ao2_iterator_destroy(&i);
if (havepattern)
regfree(®exbuf);
@@ -5817,6 +5822,7 @@
peer->encmethods ? "(E)" : " ", status, term);
total_peers++;
}
+ ao2_iterator_destroy(&i);
if (s)
astman_append(s,"%d iax2 peers [%d online, %d offline, %d unmonitored]%s", total_peers, online_peers, offline_peers, unmonitored_peers, term);
@@ -6348,6 +6354,7 @@
ast_codec_pref_convert(&iaxs[callno]->rprefs, ies->codec_prefs, 32, 0);
ast_codec_pref_convert(&iaxs[callno]->prefs, ies->codec_prefs, 32, 0);
}
+ ao2_iterator_destroy(&i);
if (!gotcapability)
iaxs[callno]->peercapability = iaxs[callno]->peerformat;
@@ -6415,6 +6422,7 @@
}
user_unref(user);
}
+ ao2_iterator_destroy(&i);
user = best;
if (!user && !ast_strlen_zero(iaxs[callno]->username)) {
user = realtime_user(iaxs[callno]->username, sin);
@@ -6944,6 +6952,7 @@
}
peer_unref(peer);
}
+ ao2_iterator_destroy(&i);
if (!peer) {
/* We checked our list and didn't find one. It's unlikely, but possible,
that we're trying to authenticate *to* a realtime peer */
@@ -11202,6 +11211,7 @@
}
user_unref(user);
}
+ ao2_iterator_destroy(&i);
}
/* Prune peers who still are supposed to be deleted */
@@ -11217,6 +11227,7 @@
}
peer_unref(peer);
}
+ ao2_iterator_destroy(&i);
}
static void set_timing(void)
@@ -11637,6 +11648,7 @@
iax2_poke_peer(peer, 0);
peer_unref(peer);
}
+ ao2_iterator_destroy(&i);
}
static int reload_config(void)
{
Modified: branches/1.4/include/asterisk/astobj2.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/include/asterisk/astobj2.h?view=diff&rev=222152&r1=222151&r2=222152
==============================================================================
--- branches/1.4/include/asterisk/astobj2.h (original)
+++ branches/1.4/include/asterisk/astobj2.h Mon Oct 5 20:16:36 2009
@@ -262,6 +262,8 @@
... do something on o ...
ao2_ref(o, -1);
}
+
+ ao2_iterator_destroy(&i);
The difference with the callback is that the control
on how to iterate is left to us.
@@ -505,6 +507,10 @@
* ao2_iterator_next() has its refcount incremented,
* and the reference must be explicitly released when done with it.
*
+ * In addition, ao2_iterator_init() will hold a reference to the container
+ * being iterated, which will be freed when ao2_iterator_destroy() is called
+ * to free up the resources used by the iterator (if any).
+ *
* Example:
*
* \code
@@ -519,6 +525,8 @@
* ... do something on o ...
* ao2_ref(o, -1);
* }
+ *
+ * ao2_iterator_destroy(&i);
*
* \endcode
*
@@ -538,13 +546,13 @@
* - a bucket number;
* - the object_id, which is also the container version number
* when the object was inserted. This identifies the object
- * univoquely, however reaching the desired object requires
+ * uniquely, however reaching the desired object requires
* scanning a list.
* - a pointer, and a container version when we saved the pointer.
* If the container has not changed its version number, then we
* can safely follow the pointer to reach the object in constant time.
* Details are in the implementation of ao2_iterator_next()
- * A freshly-initialized iterator has bucket=0, version = 0.
+ * A freshly-initialized iterator has bucket=0, version=0.
*/
struct ao2_iterator {
@@ -552,7 +560,6 @@
struct ao2_container *c;
/*! operation flags */
int flags;
-#define F_AO2I_DONTLOCK 1 /*!< don't lock when iterating */
/*! current bucket */
int bucket;
/*! container version */
@@ -563,8 +570,47 @@
unsigned int version;
};
+/*! Flags that can be passed to ao2_iterator_init() to modify the behavior
+ * of the iterator.
+ */
+enum ao2_iterator_flags {
+ /*! Prevents ao2_iterator_next() from locking the container
+ * while retrieving the next object from it.
+ */
+ AO2_ITERATOR_DONTLOCK = (1 << 0),
+};
+
+/*!
+ * \brief Create an iterator for a container
+ *
+ * \param c the container
+ * \param flags one or more flags from ao2_iterator_flags
+ *
+ * \retval the constructed iterator
+ *
+ * \note This function does \b not take a pointer to an iterator;
+ * rather, it returns an iterator structure that should be
+ * assigned to (overwriting) an existing iterator structure
+ * allocated on the stack or on the heap.
+ *
+ * This function will take a reference on the container being iterated.
+ *
+ */
struct ao2_iterator ao2_iterator_init(struct ao2_container *c, int flags);
+/*!
+ * \brief Destroy a container iterator
+ *
+ * \param i the iterator to destroy
+ *
+ * \retval none
+ *
+ * This function will release the container reference held by the iterator
+ * and any other resources it may be holding.
+ *
+ */
+void ao2_iterator_destroy(struct ao2_iterator *i);
+
void *ao2_iterator_next(struct ao2_iterator *a);
#endif /* _ASTERISK_ASTOBJ2_H */
Modified: branches/1.4/main/astobj2.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/main/astobj2.c?view=diff&rev=222152&r1=222151&r2=222152
==============================================================================
--- branches/1.4/main/astobj2.c (original)
+++ branches/1.4/main/astobj2.c Mon Oct 5 20:16:36 2009
@@ -580,8 +580,19 @@
.c = c,
.flags = flags
};
+
+ ao2_ref(c, +1);
return a;
+}
+
+/*!
+ * destroy an iterator
+ */
+void ao2_iterator_destroy(struct ao2_iterator *i)
+{
+ ao2_ref(i->c, -1);
+ i->c = NULL;
}
/*
@@ -596,7 +607,7 @@
if (INTERNAL_OBJ(a->c) == NULL)
return NULL;
- if (!(a->flags & F_AO2I_DONTLOCK))
+ if (!(a->flags & AO2_ITERATOR_DONTLOCK))
ao2_lock(a->c);
/* optimization. If the container is unchanged and
@@ -637,7 +648,7 @@
ao2_ref(ret, 1);
}
- if (!(a->flags & F_AO2I_DONTLOCK))
+ if (!(a->flags & AO2_ITERATOR_DONTLOCK))
ao2_unlock(a->c);
return ret;
Modified: branches/1.4/res/res_musiconhold.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/res/res_musiconhold.c?view=diff&rev=222152&r1=222151&r2=222152
==============================================================================
--- branches/1.4/res/res_musiconhold.c (original)
+++ branches/1.4/res/res_musiconhold.c Mon Oct 5 20:16:36 2009
@@ -1349,6 +1349,8 @@
}
}
+ ao2_iterator_destroy(&i);
+
return 0;
}
@@ -1370,6 +1372,8 @@
ast_cli(fd, "\tFormat: %s\n", ast_getformatname(class->format));
}
}
+
+ ao2_iterator_destroy(&i);
return 0;
}
More information about the asterisk-commits
mailing list