[Asterisk-code-review] asterisk: Audit locking of channel when manipulating flags. (asterisk[13])

Jenkins2 asteriskteam at digium.com
Fri May 26 09:12:12 CDT 2017


Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5624 )

Change subject: asterisk: Audit locking of channel when manipulating flags.
......................................................................


asterisk: Audit locking of channel when manipulating flags.

When manipulating flags on a channel the channel has to be
locked to guarantee that nothing else is also manipulating
the flags. This change introduces locking where necessary to
guarantee this. It also adds helper functions that manipulate
channel flags and lock to reduce repeated code.

ASTERISK-26789

Change-Id: I489280662dba0f4c50981bfc5b5a7073fef2db10
---
M apps/app_chanspy.c
M apps/app_dial.c
M apps/app_disa.c
M apps/app_dumpchan.c
M apps/app_externalivr.c
M include/asterisk/channel.h
M main/autoservice.c
M main/bridge_after.c
M main/bridge_channel.c
M main/channel.c
M main/file.c
M main/manager.c
M main/pbx.c
M res/res_agi.c
M res/res_musiconhold.c
15 files changed, 96 insertions(+), 55 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Mark Michelson: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved; Verified
  Jenkins2: Approved for Submit



diff --git a/apps/app_chanspy.c b/apps/app_chanspy.c
index 608eb6b..2a472bf 100644
--- a/apps/app_chanspy.c
+++ b/apps/app_chanspy.c
@@ -676,9 +676,7 @@
 		}
 	}
 
-	ast_channel_lock(chan);
-	ast_set_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
-	ast_channel_unlock(chan);
+	ast_channel_set_flag(chan, AST_FLAG_END_DTMF_ONLY);
 
 	csth.volfactor = *volfactor;
 
@@ -808,9 +806,7 @@
 	else
 		ast_deactivate_generator(chan);
 
-	ast_channel_lock(chan);
-	ast_clear_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
-	ast_channel_unlock(chan);
+	ast_channel_clear_flag(chan, AST_FLAG_END_DTMF_ONLY);
 
 	if (ast_test_flag(flags, OPTION_WHISPER | OPTION_BARGE | OPTION_DTMF_SWITCH_MODES)) {
 		ast_audiohook_lock(&csth.whisper_audiohook);
@@ -904,7 +900,7 @@
 	if (ast_channel_state(chan) != AST_STATE_UP)
 		ast_answer(chan);
 
-	ast_set_flag(ast_channel_flags(chan), AST_FLAG_SPYING); /* so nobody can spy on us while we are spying */
+	ast_channel_set_flag(chan, AST_FLAG_SPYING);
 
 	waitms = 100;
 
@@ -917,7 +913,7 @@
 			if (!res)
 				res = ast_waitstream(chan, "");
 			else if (res < 0) {
-				ast_clear_flag(ast_channel_flags(chan), AST_FLAG_SPYING);
+				ast_channel_clear_flag(chan, AST_FLAG_SPYING);
 				break;
 			}
 			if (!ast_strlen_zero(exitcontext)) {
@@ -960,7 +956,7 @@
 		res = ast_waitfordigit(chan, waitms);
 		if (res < 0) {
 			iter = ast_channel_iterator_destroy(iter);
-			ast_clear_flag(ast_channel_flags(chan), AST_FLAG_SPYING);
+			ast_channel_clear_flag(chan, AST_FLAG_SPYING);
 			break;
 		}
 		if (!ast_strlen_zero(exitcontext)) {
@@ -1179,7 +1175,7 @@
 	}
 exit:
 
-	ast_clear_flag(ast_channel_flags(chan), AST_FLAG_SPYING);
+	ast_channel_clear_flag(chan, AST_FLAG_SPYING);
 
 	ast_channel_setoption(chan, AST_OPTION_TXGAIN, &zero_volume, sizeof(zero_volume), 0);
 
diff --git a/apps/app_dial.c b/apps/app_dial.c
index 2503405..9f77cd8 100644
--- a/apps/app_dial.c
+++ b/apps/app_dial.c
@@ -2870,7 +2870,7 @@
 				ast_log(LOG_ERROR, "error streaming file '%s' to callee\n", opt_args[OPT_ARG_ANNOUNCE]);
 			}
 
-			ast_set_flag(ast_channel_flags(peer), AST_FLAG_END_DTMF_ONLY);
+			ast_channel_set_flag(peer, AST_FLAG_END_DTMF_ONLY);
 			while (ast_channel_stream(peer)) {
 				int ms;
 
@@ -2934,13 +2934,13 @@
 				}
 				ast_sched_runq(ast_channel_sched(peer));
 			}
-			ast_clear_flag(ast_channel_flags(peer), AST_FLAG_END_DTMF_ONLY);
+			ast_channel_clear_flag(peer, AST_FLAG_END_DTMF_ONLY);
 		}
 
 		if (chan && peer && ast_test_flag64(&opts, OPT_GOTO) && !ast_strlen_zero(opt_args[OPT_ARG_GOTO])) {
 			/* chan and peer are going into the PBX; as such neither are considered
 			 * outgoing channels any longer */
-			ast_clear_flag(ast_channel_flags(chan), AST_FLAG_OUTGOING);
+			ast_channel_clear_flag(chan, AST_FLAG_OUTGOING);
 
 			ast_replace_subargument_delimiter(opt_args[OPT_ARG_GOTO]);
 			ast_parseable_goto(chan, opt_args[OPT_ARG_GOTO]);
diff --git a/apps/app_disa.c b/apps/app_disa.c
index 824e8fe..dbdad94 100644
--- a/apps/app_disa.c
+++ b/apps/app_disa.c
@@ -210,7 +210,7 @@
 
 	play_dialtone(chan, args.mailbox);
 
-	ast_set_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
+	ast_channel_set_flag(chan, AST_FLAG_END_DTMF_ONLY);
 
 	for (;;) {
 		  /* if outa time, give em reorder */
@@ -226,7 +226,7 @@
 		}
 
 		if (!(f = ast_read(chan))) {
-			ast_clear_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
+			ast_channel_clear_flag(chan, AST_FLAG_END_DTMF_ONLY);
 			return -1;
 		}
 
@@ -234,7 +234,7 @@
 			if (f->data.uint32)
 				ast_channel_hangupcause_set(chan, f->data.uint32);
 			ast_frfree(f);
-			ast_clear_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
+			ast_channel_clear_flag(chan, AST_FLAG_END_DTMF_ONLY);
 			return -1;
 		}
 
@@ -263,7 +263,7 @@
 						fp = fopen(args.passcode,"r");
 						if (!fp) {
 							ast_log(LOG_WARNING,"DISA password file %s not found on chan %s\n",args.passcode,ast_channel_name(chan));
-							ast_clear_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
+							ast_channel_clear_flag(chan, AST_FLAG_END_DTMF_ONLY);
 							return -1;
 						}
 						pwline[0] = 0;
@@ -359,7 +359,7 @@
 		}
 	}
 
-	ast_clear_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
+	ast_channel_clear_flag(chan, AST_FLAG_END_DTMF_ONLY);
 
 	if (k == 3) {
 		int recheck = 0;
diff --git a/apps/app_dumpchan.c b/apps/app_dumpchan.c
index 1a5a446..1416438 100644
--- a/apps/app_dumpchan.c
+++ b/apps/app_dumpchan.c
@@ -90,8 +90,6 @@
 
 	ast_channel_lock(c);
 	bridge = ast_channel_get_bridge(c);
-	ast_channel_unlock(c);
-
 	snprintf(buf,size,
 		"Name=               %s\n"
 		"Type=               %s\n"
@@ -168,7 +166,7 @@
 		ast_channel_appl(c) ? ast_channel_appl(c) : "(N/A)",
 		ast_channel_data(c) ? S_OR(ast_channel_data(c), "(Empty)") : "(None)",
 		(ast_test_flag(ast_channel_flags(c), AST_FLAG_BLOCKING) ? ast_channel_blockproc(c) : "(Not Blocking)"));
-
+	ast_channel_unlock(c);
 	ao2_cleanup(bridge);
 	return 0;
 }
diff --git a/apps/app_externalivr.c b/apps/app_externalivr.c
index 9cb8839..b661a2e 100644
--- a/apps/app_externalivr.c
+++ b/apps/app_externalivr.c
@@ -645,9 +645,9 @@
 		setvbuf(eivr_errors, NULL, _IONBF, 0);
 	}
 
- 	while (1) {
- 		if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_ZOMBIE)) {
- 			ast_chan_log(LOG_ERROR, chan, "Is a zombie\n");
+	while (1) {
+		if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_ZOMBIE)) {
+			ast_chan_log(LOG_ERROR, chan, "Is a zombie\n");
  			break;
  		}
  		if (!hangup_info_sent && !(ast_test_flag(&flags, run_dead)) && ast_check_hangup(chan)) {
diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index 9acd820..393c020 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -4330,6 +4330,31 @@
 struct ast_flags *ast_channel_flags(struct ast_channel *chan);
 
 /*!
+ * \since 13.17.0
+ * \brief Set a flag on a channel
+ *
+ * \param chan The channel to set the flag on
+ * \param flag The flag to set
+ *
+ * \note This will lock the channel internally. If the channel is already
+ * locked it is still safe to call.
+ */
+
+void ast_channel_set_flag(struct ast_channel *chan, unsigned int flag);
+
+/*!
+ * \since 13.17.0
+ * \param Clear a flag on a channel
+ *
+ * \param chan The channel to clear the flag from
+ * \param flag The flag to clear
+ *
+ * \note This will lock the channel internally. If the channel is already
+ * locked it is still safe to call.
+ */
+void ast_channel_clear_flag(struct ast_channel *chan, unsigned int flag);
+
+/*!
  * \since 12.4.0
  * \brief Return whether or not any manager variables have been set
  *
diff --git a/main/autoservice.c b/main/autoservice.c
index 305ab23..f353f0e 100644
--- a/main/autoservice.c
+++ b/main/autoservice.c
@@ -300,11 +300,11 @@
 		res = 0;
 	}
 
+	ast_channel_lock(chan);
 	if (!as->orig_end_dtmf_flag) {
 		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
 	}
 
-	ast_channel_lock(chan);
 	while ((f = AST_LIST_REMOVE_HEAD(&as->deferred_frames, frame_list))) {
 		if (!((1 << f->frametype) & as->ignore_frame_types)) {
 			ast_queue_frame_head(chan, f);
diff --git a/main/bridge_after.c b/main/bridge_after.c
index a41f8a5..1208b57 100644
--- a/main/bridge_after.c
+++ b/main/bridge_after.c
@@ -484,7 +484,7 @@
 		}
 	} else if (!ast_check_hangup(chan)) {
 		/* Clear the outgoing flag */
-		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_OUTGOING);
+		ast_channel_clear_flag(chan, AST_FLAG_OUTGOING);
 
 		if (after_bridge->specific) {
 			goto_failed = ast_explicit_goto(chan, after_bridge->context,
diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index b466b3c..eba5ae4 100644
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -2092,7 +2092,7 @@
 	    && (ast_channel_is_leaving_bridge(bridge_channel->chan)
 	        || bridge_channel->state == BRIDGE_CHANNEL_STATE_WAIT)) {
 		ast_debug(2, "Channel %s will survive this bridge; clearing outgoing (dialed) flag\n", ast_channel_name(bridge_channel->chan));
-		ast_clear_flag(ast_channel_flags(bridge_channel->chan), AST_FLAG_OUTGOING);
+		ast_channel_clear_flag(bridge_channel->chan, AST_FLAG_OUTGOING);
 	}
 
 	bridge->reconfigured = 1;
diff --git a/main/channel.c b/main/channel.c
index 4192c00..5aaeab9 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -1296,8 +1296,10 @@
 	int pre = 0;
 
 	if (chan) {
+		ast_channel_lock(chan);
 		pre = ast_test_flag(ast_channel_flags(chan), AST_FLAG_DEFER_DTMF);
 		ast_set_flag(ast_channel_flags(chan), AST_FLAG_DEFER_DTMF);
+		ast_channel_unlock(chan);
 	}
 	return pre;
 }
@@ -1305,8 +1307,9 @@
 /*! \brief Unset defer DTMF flag on channel */
 void ast_channel_undefer_dtmf(struct ast_channel *chan)
 {
-	if (chan)
-		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_DEFER_DTMF);
+	if (chan) {
+		ast_channel_clear_flag(chan, AST_FLAG_DEFER_DTMF);
+	}
 }
 
 struct ast_channel *ast_channel_callback(ao2_callback_data_fn *cb_fn, void *arg,
@@ -3518,7 +3521,7 @@
 		return -1;
 
 	/* Only look for the end of DTMF, don't bother with the beginning and don't emulate things */
-	ast_set_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+	ast_channel_set_flag(c, AST_FLAG_END_DTMF_ONLY);
 
 	/* Wait for a digit, no more than timeout_ms milliseconds total.
 	 * Or, wait indefinitely if timeout_ms is <0.
@@ -3537,12 +3540,12 @@
 			if (errno == 0 || errno == EINTR)
 				continue;
 			ast_log(LOG_WARNING, "Wait failed (%s)\n", strerror(errno));
-			ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+			ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 			return -1;
 		} else if (outfd > -1) {
 			/* The FD we were watching has something waiting */
 			ast_log(LOG_WARNING, "The FD we were waiting for has something waiting. Waitfordigit returning numeric 1\n");
-			ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+			ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 			return 1;
 		} else if (rchan) {
 			int res;
@@ -3556,13 +3559,13 @@
 			case AST_FRAME_DTMF_END:
 				res = f->subclass.integer;
 				ast_frfree(f);
-				ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+				ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 				return res;
 			case AST_FRAME_CONTROL:
 				switch (f->subclass.integer) {
 				case AST_CONTROL_HANGUP:
 					ast_frfree(f);
-					ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+					ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 					return -1;
 				case AST_CONTROL_STREAM_STOP:
 				case AST_CONTROL_STREAM_SUSPEND:
@@ -3573,7 +3576,7 @@
 					 * that perform stream control will handle this. */
 					res = f->subclass.integer;
 					ast_frfree(f);
-					ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+					ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 					return res;
 				case AST_CONTROL_PVT_CAUSE_CODE:
 				case AST_CONTROL_RINGING:
@@ -3608,7 +3611,7 @@
 		}
 	}
 
-	ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+	ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 
 	return 0; /* Time is up */
 }
@@ -5873,9 +5876,9 @@
 	} else if (caller) { /* no outgoing helper so use caller if available */
 		call_forward_inherit(new_chan, caller, orig);
 	}
-	ast_set_flag(ast_channel_flags(new_chan), AST_FLAG_ORIGINATED);
 
 	ast_channel_lock_both(orig, new_chan);
+	ast_channel_set_flag(new_chan, AST_FLAG_ORIGINATED);
 	pbx_builtin_setvar_helper(new_chan, "FORWARDERNAME", forwarder);
 	ast_party_connected_line_copy(ast_channel_connected(new_chan), ast_channel_connected(orig));
 	ast_party_redirecting_copy(ast_channel_redirecting(new_chan), ast_channel_redirecting(orig));
@@ -10996,3 +10999,18 @@
 {
 	return ast_channel_internal_errno();
 }
+
+void ast_channel_set_flag(struct ast_channel *chan, unsigned int flag)
+{
+	ast_channel_lock(chan);
+	ast_set_flag(ast_channel_flags(chan), flag);
+	ast_channel_unlock(chan);
+}
+
+void ast_channel_clear_flag(struct ast_channel *chan, unsigned int flag)
+{
+	ast_channel_lock(chan);
+	ast_clear_flag(ast_channel_flags(chan), flag);
+	ast_channel_unlock(chan);
+}
+
diff --git a/main/file.c b/main/file.c
index 11617fb..bb1db3e 100644
--- a/main/file.c
+++ b/main/file.c
@@ -1545,7 +1545,7 @@
 		reverse = "";
 
 	/* Switch the channel to end DTMF frame only. waitstream_core doesn't care about the start of DTMF. */
-	ast_set_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+	ast_channel_set_flag(c, AST_FLAG_END_DTMF_ONLY);
 
 	if (ast_test_flag(ast_channel_flags(c), AST_FLAG_MASQ_NOSTREAM))
 		orig_chan_name = ast_strdupa(ast_channel_name(c));
@@ -1577,7 +1577,7 @@
 			res = ast_waitfor(c, ms);
 			if (res < 0) {
 				ast_log(LOG_WARNING, "Select failed (%s)\n", strerror(errno));
-				ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+				ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 				return res;
 			}
 		} else {
@@ -1588,11 +1588,11 @@
 				if (errno == EINTR)
 					continue;
 				ast_log(LOG_WARNING, "Wait failed (%s)\n", strerror(errno));
-				ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+				ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 				return -1;
 			} else if (outfd > -1) { /* this requires cmdfd set */
 				/* The FD we were watching has something waiting */
-				ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+				ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 				return 1;
 			}
 			/* if rchan is set, it is 'c' */
@@ -1601,7 +1601,7 @@
 		if (res > 0) {
 			struct ast_frame *fr = ast_read(c);
 			if (!fr) {
-				ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+				ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 				return -1;
 			}
 			switch (fr->frametype) {
@@ -1612,7 +1612,7 @@
 						S_COR(ast_channel_caller(c)->id.number.valid, ast_channel_caller(c)->id.number.str, NULL))) {
 						res = fr->subclass.integer;
 						ast_frfree(fr);
-						ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+						ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 						return res;
 					}
 				} else {
@@ -1628,7 +1628,7 @@
 							"Break");
 
 						ast_frfree(fr);
-						ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+						ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 						return res;
 					}
 				}
@@ -1645,7 +1645,7 @@
 						"Break");
 					res = fr->subclass.integer;
 					ast_frfree(fr);
-					ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+					ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 					return res;
 				case AST_CONTROL_STREAM_REVERSE:
 					if (!skip_ms) {
@@ -1663,7 +1663,7 @@
 				case AST_CONTROL_BUSY:
 				case AST_CONTROL_CONGESTION:
 					ast_frfree(fr);
-					ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+					ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 					return -1;
 				case AST_CONTROL_RINGING:
 				case AST_CONTROL_ANSWER:
@@ -1700,7 +1700,7 @@
 		ast_sched_runq(ast_channel_sched(c));
 	}
 
-	ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY);
+	ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY);
 
 	return (err || ast_channel_softhangup_internal_flag(c)) ? -1 : 0;
 }
diff --git a/main/manager.c b/main/manager.c
index 6f57f05..6db52b8 100644
--- a/main/manager.c
+++ b/main/manager.c
@@ -4819,14 +4819,10 @@
 
 	/* Release the bridge wait. */
 	if (chan1_wait) {
-		ast_channel_lock(chan);
-		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT);
-		ast_channel_unlock(chan);
+		ast_channel_clear_flag(chan, AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT);
 	}
 	if (chan2_wait) {
-		ast_channel_lock(chan2);
-		ast_clear_flag(ast_channel_flags(chan2), AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT);
-		ast_channel_unlock(chan2);
+		ast_channel_clear_flag(chan, AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT);
 	}
 
 	chan2 = ast_channel_unref(chan2);
diff --git a/main/pbx.c b/main/pbx.c
index b4089ab..cfc5f7f 100644
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -4257,8 +4257,10 @@
 	ast_channel_pbx(c)->rtimeoutms = 10000;
 	ast_channel_pbx(c)->dtimeoutms = 5000;
 
+	ast_channel_lock(c);
 	autoloopflag = ast_test_flag(ast_channel_flags(c), AST_FLAG_IN_AUTOLOOP);	/* save value to restore at the end */
 	ast_set_flag(ast_channel_flags(c), AST_FLAG_IN_AUTOLOOP);
+	ast_channel_unlock(c);
 
 	if (ast_strlen_zero(ast_channel_exten(c))) {
 		/* If not successful fall back to 's' - but only if there is no given exten  */
@@ -4503,8 +4505,10 @@
 		ast_pbx_hangup_handler_run(c);
 	}
 
+	ast_channel_lock(c);
 	ast_set2_flag(ast_channel_flags(c), autoloopflag, AST_FLAG_IN_AUTOLOOP);
 	ast_clear_flag(ast_channel_flags(c), AST_FLAG_BRIDGE_HANGUP_RUN); /* from one round to the next, make sure this gets cleared */
+	ast_channel_unlock(c);
 	pbx_destroy(ast_channel_pbx(c));
 	ast_channel_pbx_set(c, NULL);
 
diff --git a/res/res_agi.c b/res/res_agi.c
index cd0d621..4b60dae 100644
--- a/res/res_agi.c
+++ b/res/res_agi.c
@@ -3121,12 +3121,14 @@
 	ast_verb(3, "AGI Script Executing Application: (%s) Options: (%s)\n", argv[1], argc >= 3 ? argv[2] : "");
 
 	if ((app_to_exec = pbx_findapp(argv[1]))) {
+		ast_channel_lock(chan);
 		if (!(workaround = ast_test_flag(ast_channel_flags(chan), AST_FLAG_DISABLE_WORKAROUNDS))) {
 			ast_set_flag(ast_channel_flags(chan), AST_FLAG_DISABLE_WORKAROUNDS);
 		}
+		ast_channel_unlock(chan);
 		res = pbx_exec(chan, app_to_exec, argc == 2 ? "" : argv[2]);
 		if (!workaround) {
-			ast_clear_flag(ast_channel_flags(chan), AST_FLAG_DISABLE_WORKAROUNDS);
+			ast_channel_clear_flag(chan, AST_FLAG_DISABLE_WORKAROUNDS);
 		}
 	} else {
 		ast_log(LOG_WARNING, "Could not find application (%s)\n", argv[1]);
diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c
index 9d61941..c52c964 100644
--- a/res/res_musiconhold.c
+++ b/res/res_musiconhold.c
@@ -1548,8 +1548,10 @@
 		}
 	}
 	if (!res) {
+		ast_channel_lock(chan);
 		ast_channel_latest_musicclass_set(chan, mohclass->name);
 		ast_set_flag(ast_channel_flags(chan), AST_FLAG_MOH);
+		ast_channel_unlock(chan);
 	}
 
 	mohclass = mohclass_unref(mohclass, "unreffing local reference to mohclass in local_ast_moh_start");
@@ -1559,10 +1561,10 @@
 
 static void local_ast_moh_stop(struct ast_channel *chan)
 {
-	ast_clear_flag(ast_channel_flags(chan), AST_FLAG_MOH);
 	ast_deactivate_generator(chan);
 
 	ast_channel_lock(chan);
+	ast_clear_flag(ast_channel_flags(chan), AST_FLAG_MOH);
 	if (ast_channel_music_state(chan)) {
 		if (ast_channel_stream(chan)) {
 			ast_closestream(ast_channel_stream(chan));

-- 
To view, visit https://gerrit.asterisk.org/5624
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I489280662dba0f4c50981bfc5b5a7073fef2db10
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>



More information about the asterisk-code-review mailing list