[Asterisk-code-review] app_queue: Remove lock on queues to avoid deadlock. (asterisk[13])

[Leif Einar Aune]

Leif Einar Aune has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/15302 )


Change subject: app_queue: Remove lock on queues to avoid deadlock.
......................................................................

app_queue: Remove lock on queues to avoid deadlock.

When analysing coredumps with gdb from situations
where asterisk stops handling queue calls it is observed that the threads are
blocked waiting for the mutex for the queues. The normal pattern is to
iterate over the queues, find a suitable queue, lock the queue, handle call
then unlock the queue. The mutex for the queues is implicitly locked by the
ao2_iterator functionality while iterating.

_queues_show() was however locking the queues collections first, then
iterating using the flag AO2_ITERATOR_DONTLOCK and locking the specific
queue in each iteration.

This resulted in the deadlock where the _queues_show() was waiting for a
specific queue mutex, while holding the queues mutex, and the other threads
were waiting for the queues mutex.

This patch uses the 'lock queues for each iteration' idiom in the function
_queues_show() instead of locking the queues collection before iterating.
It has been run succesfully in production with busy queues, and the observed
locking of queue call handling were no longer present.

ASTERISK-22976
Reported-by: Aaron An

Change-Id: I9b3583a20176f7c26442b7e406bb35fd7f8833b6
---
M apps/app_queue.c
1 file changed, 1 insertion(+), 3 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/02/15302/1

diff --git a/apps/app_queue.c b/apps/app_queue.c
index 4db468f..cbb39db 100644
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -9251,8 +9251,7 @@
 		}
 	}
 
-	ao2_lock(queues);
-	queue_iter = ao2_iterator_init(queues, AO2_ITERATOR_DONTLOCK);
+	queue_iter = ao2_iterator_init(queues, 0);
 	while ((q = ao2_t_iterator_next(&queue_iter, "Iterate through queues"))) {
 		float sl;
 		struct call_queue *realtime_queue = NULL;
@@ -9356,7 +9355,6 @@
 		queue_t_unref(q, "Done with iterator"); /* Unref the iterator's reference */
 	}
 	ao2_iterator_destroy(&queue_iter);
-	ao2_unlock(queues);
 	if (!found) {
 		if (argc == 3) {
 			ast_str_set(&out, 0, "No such queue: %s.", argv[2]);

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15302
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I9b3583a20176f7c26442b7e406bb35fd7f8833b6
Gerrit-Change-Number: 15302
Gerrit-PatchSet: 1
Gerrit-Owner: Leif Einar Aune <leif.einar at telemagic.no>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20201231/520c7d6b/attachment.html>