[asterisk-commits] mmichelson: trunk r168629 - in /trunk: ./ apps/app_queue.c

[SVN commits to the Asterisk project]

Author: mmichelson
Date: Wed Jan 14 18:14:17 2009
New Revision: 168629

URL: http://svn.digium.com/svn-view/asterisk?view=rev&rev=168629
Log:
Merged revisions 168628 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r168628 | mmichelson | 2009-01-14 18:11:01 -0600 (Wed, 14 Jan 2009) | 16 lines

Fix some crashes from bad datastore handling in app_queue.c

* The queue_transfer_fixup function was searching for and removing
  the datastore from the incorrect channel, so this was fixed.

* Most datastore operations regarding the queue_transfer datastore
  were being done without the channel locked, so proper channel locking
  was added, too.

(closes issue #14086)
Reported by: ZX81
Patches:
      14086v2.patch uploaded by putnopvut (license 60)
Tested by: ZX81, festr


........

Modified:
    trunk/   (props changed)
    trunk/apps/app_queue.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-1.4-merged' - no diff available.

Modified: trunk/apps/app_queue.c
URL: http://svn.digium.com/svn-view/asterisk/trunk/apps/app_queue.c?view=diff&rev=168629&r1=168628&r2=168629
==============================================================================
--- trunk/apps/app_queue.c (original)
+++ trunk/apps/app_queue.c Wed Jan 14 18:14:17 2009
@@ -3332,8 +3332,9 @@
 
 	update_queue(qe->parent, member, callcompletedinsl, (time(NULL) - callstart));
 	
-	if ((datastore = ast_channel_datastore_find(new_chan, &queue_transfer_info, NULL))) {
-		ast_channel_datastore_remove(new_chan, datastore);
+	/* No need to lock the channels because they are already locked in ast_do_masquerade */
+	if ((datastore = ast_channel_datastore_find(old_chan, &queue_transfer_info, NULL))) {
+		ast_channel_datastore_remove(old_chan, datastore);
 	} else {
 		ast_log(LOG_WARNING, "Can't find the queue_transfer datastore.\n");
 	}
@@ -3344,6 +3345,8 @@
  * When a caller is atxferred, then the queue_transfer_info datastore
  * is removed from the channel. If it's still there after the bridge is
  * broken, then the caller was not atxferred.
+ *
+ * \note Only call this with chan locked
  */
 static int attended_transfer_occurred(struct ast_channel *chan)
 {
@@ -3667,9 +3670,11 @@
 	 * to which the datastore was moved hangs up, it will attempt to free this
 	 * datastore again, causing a crash
 	 */
+	ast_channel_lock(qe->chan);
 	if (datastore && !ast_channel_datastore_remove(qe->chan, datastore)) {
 		ast_datastore_free(datastore);
 	}
+	ast_channel_unlock(qe->chan);
 	ao2_lock(qe->parent);
 	if (qe->parent->strategy == QUEUE_STRATEGY_RRMEMORY) {
 		store_next_rr(qe, outgoing);
@@ -4078,6 +4083,7 @@
 		/* If the queue member did an attended transfer, then the TRANSFER already was logged in the queue_log
 		 * when the masquerade occurred. These other "ending" queue_log messages are unnecessary
 		 */
+		ast_channel_lock(qe->chan);
 		if (!attended_transfer_occurred(qe->chan)) {
 			struct ast_datastore *tds;
 			if (strcasecmp(oldcontext, qe->chan->context) || strcasecmp(oldexten, qe->chan->exten)) {
@@ -4094,17 +4100,16 @@
 					(long) (callstart - qe->start), (long) (time(NULL) - callstart), qe->opos);
 				send_agent_complete(qe, queuename, peer, member, callstart, vars, sizeof(vars), AGENT);
 			}
-			ast_channel_lock(qe->chan);
-			if ((tds = ast_channel_datastore_find(qe->chan, &queue_transfer_info, NULL))) {
+			if ((tds = ast_channel_datastore_find(qe->chan, &queue_transfer_info, NULL))) {	
 				ast_channel_datastore_remove(qe->chan, tds);
 			}
-			ast_channel_unlock(qe->chan);
 			update_queue(qe->parent, member, callcompletedinsl, (time(NULL) - callstart));
 		}
 
 		if (transfer_ds) {
 			ast_datastore_free(transfer_ds);
 		}
+		ast_channel_unlock(qe->chan);
 		ast_hangup(peer);
 		res = bridge ? bridge : 1;
 		ao2_ref(member, -1);