[asterisk-commits] irroot: trunk r336095 - in /trunk: ./ apps/app_queue.c
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Thu Sep 15 10:59:30 CDT 2011
Author: irroot
Date: Thu Sep 15 10:59:24 2011
New Revision: 336095
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=336095
Log:
Merged revisions 336094 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/10
................
r336094 | irroot | 2011-09-15 17:54:46 +0200 (Thu, 15 Sep 2011) | 26 lines
Merged revisions 336093 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8
........
r336093 | irroot | 2011-09-15 17:46:21 +0200 (Thu, 15 Sep 2011) | 20 lines
Locking order in app_queue.c causes deadlocks.
a channel lock must never be held with the queues container lock held.
the deadlock occured on masquerade.
the queues container lock is a relic of the past the old queue module lock.
with ao2 there is no need to hold this lock when dealing with members this
patch removes unneeded locks.
(closes issue ASTERISK-18101)
(closes issue ASTERISK-18487)
Reported by: Paul Rolfe, Jason Legault
Tested by: irroot, Jason Legault, Paul Rolfe
Reviewed by: Matthew Nicholson
Review: https://reviewboard.asterisk.org/r/1402/
........
................
Modified:
trunk/ (props changed)
trunk/apps/app_queue.c
Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-10-merged' - no diff available.
Modified: trunk/apps/app_queue.c
URL: http://svnview.digium.com/svn/asterisk/trunk/apps/app_queue.c?view=diff&rev=336095&r1=336094&r2=336095
==============================================================================
--- trunk/apps/app_queue.c (original)
+++ trunk/apps/app_queue.c Thu Sep 15 10:59:24 2011
@@ -2418,13 +2418,11 @@
queue_t_unref(q, "Need to find realtime queue");
}
- ao2_lock(queues);
-
q = find_queue_by_name_rt(queuename, queue_vars, member_config);
ast_config_destroy(member_config);
ast_variables_destroy(queue_vars);
- /* update the use_weight value if the queue's has gained or lost a weight */
+ /* update the use_weight value if the queue's has gained or lost a weight */
if (q) {
if (!q->weight && prev_weight) {
ast_atomic_fetchadd_int(&use_weight, -1);
@@ -2434,8 +2432,6 @@
}
}
/* Other cases will end up with the proper value for use_weight */
- ao2_unlock(queues);
-
} else {
update_realtime_members(q);
}
@@ -2469,10 +2465,9 @@
return;
}
- ao2_lock(queues);
ao2_lock(q);
-
- /* Temporarily set realtime members dead so we can detect deleted ones.*/
+
+ /* Temporarily set realtime members dead so we can detect deleted ones.*/
mem_iter = ao2_iterator_init(q->members, 0);
while ((m = ao2_iterator_next(&mem_iter))) {
if (m->realtime)
@@ -2501,7 +2496,6 @@
}
ao2_iterator_destroy(&mem_iter);
ao2_unlock(q);
- ao2_unlock(queues);
ast_config_destroy(member_config);
}
@@ -2516,7 +2510,6 @@
if (!(q = load_realtime_queue(queuename)))
return res;
- ao2_lock(queues);
ao2_lock(q);
/* This is our one */
@@ -2525,7 +2518,6 @@
if ((status = get_member_status(q, qe->max_penalty, qe->min_penalty, q->joinempty))) {
*reason = QUEUE_JOINEMPTY;
ao2_unlock(q);
- ao2_unlock(queues);
queue_t_unref(q, "Done with realtime queue");
return res;
}
@@ -2589,7 +2581,6 @@
ast_debug(1, "Queue '%s' Join, Channel '%s', Position '%d'\n", q->name, qe->chan->name, qe->pos );
}
ao2_unlock(q);
- ao2_unlock(queues);
queue_t_unref(q, "Done with realtime queue");
return res;
@@ -2982,9 +2973,7 @@
struct member *mem;
int found = 0;
struct ao2_iterator queue_iter;
-
- /* q's lock and rq's lock already set by try_calling()
- * to solve deadlock */
+
queue_iter = ao2_iterator_init(queues, 0);
while ((q = ao2_t_iterator_next(&queue_iter, "Iterate through queues"))) {
if (q == rq) { /* don't check myself, could deadlock */
@@ -4448,7 +4437,6 @@
struct ao2_iterator memi;
struct ast_datastore *datastore, *transfer_ds;
struct queue_end_bridge *queue_end_bridge = NULL;
- const int need_weight = use_weight;
ast_channel_lock(qe->chan);
datastore = ast_channel_datastore_find(qe->chan, &dialed_interface_info, NULL);
@@ -4531,9 +4519,6 @@
qe->cancel_answered_elsewhere = 1;
}
- /* Hold the lock while we setup the outgoing calls */
- if (need_weight)
- ao2_lock(queues);
ao2_lock(qe->parent);
ast_debug(1, "%s is trying to call a queue member.\n",
qe->chan->name);
@@ -4552,8 +4537,6 @@
ao2_ref(cur, -1);
ao2_unlock(qe->parent);
ao2_iterator_destroy(&memi);
- if (need_weight)
- ao2_unlock(queues);
goto out;
}
if (!datastore) {
@@ -4561,8 +4544,6 @@
ao2_ref(cur, -1);
ao2_unlock(qe->parent);
ao2_iterator_destroy(&memi);
- if (need_weight)
- ao2_unlock(queues);
callattempt_free(tmp);
goto out;
}
@@ -4571,8 +4552,6 @@
ao2_ref(cur, -1);
ao2_unlock(&qe->parent);
ao2_iterator_destroy(&memi);
- if (need_weight)
- ao2_unlock(queues);
callattempt_free(tmp);
goto out;
}
@@ -4609,8 +4588,6 @@
ao2_ref(cur, -1);
ao2_unlock(qe->parent);
ao2_iterator_destroy(&memi);
- if (need_weight)
- ao2_unlock(queues);
callattempt_free(tmp);
goto out;
}
@@ -4673,8 +4650,6 @@
++qe->pending;
ao2_unlock(qe->parent);
ring_one(qe, outgoing, &numbusies);
- if (need_weight)
- ao2_unlock(queues);
lpeer = wait_for_answer(qe, outgoing, &to, &digit, numbusies, ast_test_flag(&(bridge_config.features_caller), AST_FEATURE_DISCONNECT), forwardsallowed, update_connectedline);
/* The ast_channel_datastore_remove() function could fail here if the
* datastore was moved to another channel during a masquerade. If this is
@@ -5279,7 +5254,6 @@
ast_copy_string(tmpmem.interface, interface, sizeof(tmpmem.interface));
if ((q = ao2_t_find(queues, &tmpq, OBJ_POINTER, "Temporary reference for interface removal"))) {
- ao2_lock(queues);
ao2_lock(q);
if ((mem = ao2_find(q->members, &tmpmem, OBJ_POINTER))) {
/* XXX future changes should beware of this assumption!! */
@@ -5290,7 +5264,6 @@
ao2_ref(mem, -1);
ao2_unlock(q);
queue_t_unref(q, "Interface wasn't dynamic, expiring temporary reference");
- ao2_unlock(queues);
return RES_NOT_DYNAMIC;
}
q->membercount--;
@@ -5310,7 +5283,6 @@
res = RES_EXISTS;
}
ao2_unlock(q);
- ao2_unlock(queues);
queue_t_unref(q, "Expiring temporary reference");
}
@@ -5334,8 +5306,6 @@
* short-circuits if the queue is already in memory. */
if (!(q = load_realtime_queue(queuename)))
return res;
-
- ao2_lock(queues);
ao2_lock(q);
if ((old_member = interface_exists(q, interface)) == NULL) {
@@ -5374,7 +5344,6 @@
res = RES_EXISTS;
}
ao2_unlock(q);
- ao2_unlock(queues);
queue_t_unref(q, "Expiring temporary reference");
return res;
@@ -5554,8 +5523,6 @@
struct call_queue *cur_queue;
char queue_data[PM_MAX_LEN];
- ao2_lock(queues);
-
/* Each key in 'pm_family' is the name of a queue */
db_tree = ast_db_gettree(pm_family, NULL);
for (entry = db_tree; entry; entry = entry->next) {
@@ -5626,7 +5593,6 @@
queue_t_unref(cur_queue, "Expire reload reference");
}
- ao2_unlock(queues);
if (db_tree) {
ast_log(LOG_NOTICE, "Queue members successfully reloaded from database.\n");
ast_db_freetree(db_tree);
@@ -7012,14 +6978,11 @@
return -1;
}
- /* We've made it here, so it looks like we're doing operations on all queues. */
- ao2_lock(queues);
-
/* Mark all queues as dead for the moment if we're reloading queues.
* For clarity, we could just be reloading members, in which case we don't want to mess
* with the other queue parameters at all*/
if (queue_reload) {
- ao2_callback(queues, OBJ_NODATA, mark_dead_and_unfound, (char *) queuename);
+ ao2_callback(queues, OBJ_NODATA | OBJ_NOLOCK, mark_dead_and_unfound, (char *) queuename);
}
/* Chug through config file */
@@ -7036,17 +6999,16 @@
ast_config_destroy(cfg);
/* Unref all the dead queues if we were reloading queues */
if (queue_reload) {
- ao2_callback(queues, OBJ_NODATA | OBJ_MULTIPLE | OBJ_UNLINK, kill_dead_queues, (char *) queuename);
- }
- ao2_unlock(queues);
+ ao2_callback(queues, OBJ_NODATA | OBJ_MULTIPLE | OBJ_UNLINK | OBJ_NOLOCK, kill_dead_queues, (char *) queuename);
+ }
return 0;
}
-
+
/*! \brief Facilitates resetting statistics for a queue
*
* This function actually does not reset any statistics, but
* rather finds a call_queue struct which corresponds to the
- * passed-in queue name and passes that structure to the
+ * passed-in queue name and passes that structure to the
* clear_queue function. If no queuename is passed in, then
* all queues will have their statistics reset.
*
More information about the asterisk-commits
mailing list