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

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed May 28 14:56:18 CDT 2008


Author: mmichelson
Date: Wed May 28 14:56:18 2008
New Revision: 118783

URL: http://svn.digium.com/view/asterisk?view=rev&rev=118783
Log:
Update to the janitor project for making sure to be thread-safe when
retrieving the value of a channel variable. This covers app_queue.

This commit also incorporates a logical change. Previously, if MixMonitor
is to be used to record the call, all the arguments were parsed first. Then
the MixMonitor app would be located. Now the order of these operations has
been swapped. Now the app is located first so that we only go through the
work of parsing the arguments if the app was found.

(closes issue #12742)
Reported by: snuffy
Patches:
      bug_12742.diff uploaded by snuffy (license 35)


Modified:
    trunk/apps/app_queue.c

Modified: trunk/apps/app_queue.c
URL: http://svn.digium.com/view/asterisk/trunk/apps/app_queue.c?view=diff&rev=118783&r1=118782&r2=118783
==============================================================================
--- trunk/apps/app_queue.c (original)
+++ trunk/apps/app_queue.c Wed May 28 14:56:18 2008
@@ -3438,11 +3438,15 @@
 		set_queue_variables(qe);
 		ao2_unlock(qe->parent);
 		
+		ast_channel_lock(qe->chan);
+		if ((monitorfilename = pbx_builtin_getvar_helper(qe->chan, "MONITOR_FILENAME"))) {
+				monitorfilename = ast_strdupa(monitorfilename);
+		}
+		ast_channel_unlock(qe->chan);
 		/* Begin Monitoring */
 		if (qe->parent->monfmt && *qe->parent->monfmt) {
 			if (!qe->parent->montype) {
 				ast_debug(1, "Starting Monitor as requested.\n");
-				monitorfilename = pbx_builtin_getvar_helper(qe->chan, "MONITOR_FILENAME");
 				if (pbx_builtin_getvar_helper(qe->chan, "MONITOR_EXEC") || pbx_builtin_getvar_helper(qe->chan, "MONITOR_EXEC_ARGS"))
 					which = qe->chan;
 				else
@@ -3457,75 +3461,79 @@
 					ast_monitor_start(which, qe->parent->monfmt, tmpid, 1, X_REC_IN | X_REC_OUT);
 				}
 			} else {
-				ast_debug(1, "Starting MixMonitor as requested.\n");
-				monitorfilename = pbx_builtin_getvar_helper(qe->chan, "MONITOR_FILENAME");
-				if (!monitorfilename) {
-					if (qe->chan->cdr)
-						ast_copy_string(tmpid, qe->chan->cdr->uniqueid, sizeof(tmpid));
-					else
-						snprintf(tmpid, sizeof(tmpid), "chan-%lx", ast_random());
-				} else {
-					const char *m = monitorfilename;
-					for (p = tmpid2; p < tmpid2 + sizeof(tmpid2) - 1; p++, m++) {
-						switch (*m) {
-						case '^':
-							if (*(m + 1) == '{')
-								*p = '$';
-							break;
-						case ',':
-							*p++ = '\\';
-							/* Fall through */
-						default:
-							*p = *m;
-						}
-						if (*m == '\0')
-							break;
-					}
-					if (p == tmpid2 + sizeof(tmpid2))
-						tmpid2[sizeof(tmpid2) - 1] = '\0';
-
-					pbx_substitute_variables_helper(qe->chan, tmpid2, tmpid, sizeof(tmpid) - 1);
-				}
-
-				monitor_exec = pbx_builtin_getvar_helper(qe->chan, "MONITOR_EXEC");
-				monitor_options = pbx_builtin_getvar_helper(qe->chan, "MONITOR_OPTIONS");
-
-				if (monitor_exec) {
-					const char *m = monitor_exec;
-					for (p = meid2; p < meid2 + sizeof(meid2) - 1; p++, m++) {
-						switch (*m) {
-						case '^':
-							if (*(m + 1) == '{')
-								*p = '$';
-							break;
-						case ',':
-							*p++ = '\\';
-							/* Fall through */
-						default:
-							*p = *m;
-						}
-						if (*m == '\0')
-							break;
-					}
-					if (p == meid2 + sizeof(meid2))
-						meid2[sizeof(meid2) - 1] = '\0';
-
-					pbx_substitute_variables_helper(qe->chan, meid2, meid, sizeof(meid) - 1);
-				}
-	
-				snprintf(tmpid2, sizeof(tmpid2), "%s.%s", tmpid, qe->parent->monfmt);
-
 				mixmonapp = pbx_findapp("MixMonitor");
-
-				if (!monitor_options)
-					monitor_options = "";
 				
 				if (mixmonapp) {
+					ast_debug(1, "Starting MixMonitor as requested.\n");
+					if (!monitorfilename) {
+						if (qe->chan->cdr)
+							ast_copy_string(tmpid, qe->chan->cdr->uniqueid, sizeof(tmpid));
+						else
+							snprintf(tmpid, sizeof(tmpid), "chan-%lx", ast_random());
+					} else {
+						const char *m = monitorfilename;
+						for (p = tmpid2; p < tmpid2 + sizeof(tmpid2) - 1; p++, m++) {
+							switch (*m) {
+							case '^':
+								if (*(m + 1) == '{')
+									*p = '$';
+								break;
+							case ',':
+								*p++ = '\\';
+								/* Fall through */
+							default:
+								*p = *m;
+							}
+							if (*m == '\0')
+								break;
+						}
+						if (p == tmpid2 + sizeof(tmpid2))
+							tmpid2[sizeof(tmpid2) - 1] = '\0';
+
+						pbx_substitute_variables_helper(qe->chan, tmpid2, tmpid, sizeof(tmpid) - 1);
+					}
+
+					ast_channel_lock(qe->chan);
+					if ((monitor_exec = pbx_builtin_getvar_helper(qe->chan, "MONITOR_EXEC"))) {
+							monitor_exec = ast_strdupa(monitor_exec);
+					}
+					if ((monitor_options = pbx_builtin_getvar_helper(qe->chan, "MONITOR_OPTIONS"))) {
+							monitor_options = ast_strdupa(monitor_options);
+					} else {
+						monitor_options = "";
+					}
+					ast_channel_unlock(qe->chan);
+
+					if (monitor_exec) {
+						const char *m = monitor_exec;
+						for (p = meid2; p < meid2 + sizeof(meid2) - 1; p++, m++) {
+							switch (*m) {
+							case '^':
+								if (*(m + 1) == '{')
+									*p = '$';
+								break;
+							case ',':
+								*p++ = '\\';
+								/* Fall through */
+							default:
+								*p = *m;
+							}
+							if (*m == '\0')
+								break;
+						}
+						if (p == meid2 + sizeof(meid2))
+							meid2[sizeof(meid2) - 1] = '\0';
+
+						pbx_substitute_variables_helper(qe->chan, meid2, meid, sizeof(meid) - 1);
+					}
+	
+					snprintf(tmpid2, sizeof(tmpid2), "%s.%s", tmpid, qe->parent->monfmt);
+
 					if (!ast_strlen_zero(monitor_exec))
 						snprintf(mixmonargs, sizeof(mixmonargs), "%s,b%s,%s", tmpid2, monitor_options, monitor_exec);
 					else
 						snprintf(mixmonargs, sizeof(mixmonargs), "%s,b%s", tmpid2, monitor_options);
-
+					
 					ast_debug(1, "Arguments being passed to MixMonitor: %s\n", mixmonargs);
 					/* We purposely lock the CDR so that pbx_exec does not update the application data */
 					if (qe->chan->cdr)
@@ -3534,9 +3542,9 @@
 					if (qe->chan->cdr)
 						ast_clear_flag(qe->chan->cdr, AST_CDR_FLAG_LOCKED);
 
-				} else
+				} else {
 					ast_log(LOG_WARNING, "Asked to run MixMonitor on this call, but cannot find the MixMonitor app!\n");
-
+				}
 			}
 		}
 		/* Drop out of the queue at this point, to prepare for next caller */
@@ -4449,6 +4457,7 @@
 		qe.expire = 0;
 
 	/* Get the priority from the variable ${QUEUE_PRIO} */
+	ast_channel_lock(chan);
 	user_priority = pbx_builtin_getvar_helper(chan, "QUEUE_PRIO");
 	if (user_priority) {
 		if (sscanf(user_priority, "%d", &prio) == 1) {
@@ -4488,6 +4497,7 @@
 	} else {
 		min_penalty = 0;
 	}
+	ast_channel_unlock(chan);
 
 	if (args.options && (strchr(args.options, 'r')))
 		ringing = 1;




More information about the asterisk-commits mailing list