[asterisk-commits] russell: branch 1.4 r303546 - in /branches/1.4: apps/ include/asterisk/ main/...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Mon Jan 24 14:32:31 CST 2011


Author: russell
Date: Mon Jan 24 14:32:21 2011
New Revision: 303546

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=303546
Log:
Fix channel redirect out of MeetMe() and other issues with channel softhangup.

Mantis issue #18585 reports that a channel redirect out of MeetMe() stopped
working properly.  This issue includes a patch that resolves the issue by
removing a call to ast_check_hangup() from app_meetme.c.  I left that in my
patch, as it doesn't need to be there.  However, the rest of the patch fixes
this problem with or without the change to app_meetme.

The key difference between what happens before and after this patch is the
effect of the END_OF_Q control frame.  After END_OF_Q is hit in ast_read(),
ast_read() will return NULL.  With the ast_check_hangup() removed, app_meetme
sees this which causes it to exit as intended.  Checking ast_check_hangup()
caused app_meetme to exit earlier in the process, and the target of the
redirect saw the condition where ast_read() returned NULL.

Removing ast_check_hangup() works around the issue in app_meetme, but doesn't
solve the issue if another application did the same thing.  There are also
other edge cases where if an application finishes at the same time that a
redirect happens, the target of the redirect will think that the channel hung
up.  So, I made some changes in pbx.c to resolve it at a deeper level.  There
are already places that unset the SOFTHANGUP_ASYNCGOTO flag in an attempt to
abort the hangup process.  My patch extends this to remove the END_OF_Q frame
from the channel's read queue, making the "abort hangup" more complete.  This
same technique was used in every place where a softhangup flag was cleared.

(closes issue #18585)
Reported by: oej
Tested by: oej, wedhorn, russell

Review: https://reviewboard.asterisk.org/r/1082/

Modified:
    branches/1.4/apps/app_meetme.c
    branches/1.4/include/asterisk/channel.h
    branches/1.4/main/channel.c
    branches/1.4/main/pbx.c
    branches/1.4/res/res_features.c

Modified: branches/1.4/apps/app_meetme.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/apps/app_meetme.c?view=diff&rev=303546&r1=303545&r2=303546
==============================================================================
--- branches/1.4/apps/app_meetme.c (original)
+++ branches/1.4/apps/app_meetme.c Mon Jan 24 14:32:21 2011
@@ -2116,10 +2116,6 @@
 				ret = 0;
 				break;
 			}
-
-			/* Perform an extra hangup check just in case */
-			if (ast_check_hangup(chan))
-				break;
 
 			c = ast_waitfor_nandfds(&chan, 1, &fd, nfds, NULL, &outfd, &ms);
 

Modified: branches/1.4/include/asterisk/channel.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/include/asterisk/channel.h?view=diff&rev=303546&r1=303545&r2=303546
==============================================================================
--- branches/1.4/include/asterisk/channel.h (original)
+++ branches/1.4/include/asterisk/channel.h Mon Jan 24 14:32:21 2011
@@ -645,6 +645,15 @@
 	 * instead of actually hanging up.
 	 */
 	AST_SOFTHANGUP_UNBRIDGE =  (1 << 6),
+
+
+	/*!
+	 * \brief All softhangup flags.
+	 *
+	 * This can be used as an argument to ast_channel_softhangup_clear
+	 * to clear all softhangup flags from a channel.
+	 */
+	AST_SOFTHANGUP_ALL =       (0xFFFFFFFF)
 };
 
 
@@ -833,6 +842,20 @@
  * \param chan channel to be soft-hung-up
  * \param reason an AST_SOFTHANGUP_* reason code */
 int ast_softhangup_nolock(struct ast_channel *chan, int cause);
+
+/*!
+ * \brief Clear a set of softhangup flags from a channel
+ *
+ * Never clear a softhangup flag from a channel directly.  Instead,
+ * use this function.  This ensures that all aspects of the softhangup
+ * process are aborted.
+ *
+ * \param chan the channel to clear the flag on
+ * \param flag the flag or flags to clear
+ *
+ * \return Nothing.
+ */
+void ast_channel_clear_softhangup(struct ast_channel *chan, int flag);
 
 /*! \brief Check to see if a channel is needing hang up 
  * \param chan channel on which to check for hang up

Modified: branches/1.4/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/main/channel.c?view=diff&rev=303546&r1=303545&r2=303546
==============================================================================
--- branches/1.4/main/channel.c (original)
+++ branches/1.4/main/channel.c Mon Jan 24 14:32:21 2011
@@ -1569,6 +1569,31 @@
 	AST_LIST_TRAVERSE_SAFE_END
 
 	return datastore;
+}
+
+void ast_channel_clear_softhangup(struct ast_channel *chan, int flag)
+{
+	ast_channel_lock(chan);
+
+	chan->_softhangup &= ~flag;
+
+	if (!chan->_softhangup) {
+		struct ast_frame *fr;
+
+		/* If we have completely cleared the softhangup flag,
+		 * then we need to fully abort the hangup process.  This requires
+		 * pulling the END_OF_Q frame out of the channel frame queue if it
+		 * still happens to be there. */
+
+		fr = AST_LIST_LAST(&chan->readq);
+		if (fr && fr->frametype == AST_FRAME_CONTROL &&
+				fr->subclass == AST_CONTROL_END_OF_Q) {
+			AST_LIST_REMOVE(&chan->readq, fr, frame_list);
+			ast_frfree(fr);
+		}
+	}
+
+	ast_channel_unlock(chan);
 }
 
 /*! \brief Softly hangup a channel, don't lock */
@@ -4598,10 +4623,10 @@
 				ast_jb_get_and_deliver(c0, c1);
 			if ((c0->_softhangup | c1->_softhangup) & AST_SOFTHANGUP_UNBRIDGE) {/* Bit operators are intentional. */
 				if (c0->_softhangup & AST_SOFTHANGUP_UNBRIDGE) {
-					c0->_softhangup &= ~AST_SOFTHANGUP_UNBRIDGE;
+					ast_channel_clear_softhangup(c0, AST_SOFTHANGUP_UNBRIDGE);
 				}
 				if (c1->_softhangup & AST_SOFTHANGUP_UNBRIDGE) {
-					c1->_softhangup &= ~AST_SOFTHANGUP_UNBRIDGE;
+					ast_channel_clear_softhangup(c1, AST_SOFTHANGUP_UNBRIDGE);
 				}
 				c0->_bridge = c1;
 				c1->_bridge = c0;
@@ -4844,10 +4869,10 @@
 
 		if ((c0->_softhangup | c1->_softhangup) & AST_SOFTHANGUP_UNBRIDGE) {/* Bit operators are intentional. */
 			if (c0->_softhangup & AST_SOFTHANGUP_UNBRIDGE) {
-				c0->_softhangup &= ~AST_SOFTHANGUP_UNBRIDGE;
+				ast_channel_clear_softhangup(c0, AST_SOFTHANGUP_UNBRIDGE);
 			}
 			if (c1->_softhangup & AST_SOFTHANGUP_UNBRIDGE) {
-				c1->_softhangup &= ~AST_SOFTHANGUP_UNBRIDGE;
+				ast_channel_clear_softhangup(c1, AST_SOFTHANGUP_UNBRIDGE);
 			}
 			c0->_bridge = c1;
 			c1->_bridge = c0;

Modified: branches/1.4/main/pbx.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/main/pbx.c?view=diff&rev=303546&r1=303545&r2=303546
==============================================================================
--- branches/1.4/main/pbx.c (original)
+++ branches/1.4/main/pbx.c Mon Jan 24 14:32:21 2011
@@ -2383,7 +2383,7 @@
 		   keep reading digits until we can't possibly get a right answer anymore.  */
 		digit = ast_waitfordigit(c, waittime * 1000);
 		if (c->_softhangup & AST_SOFTHANGUP_ASYNCGOTO) {
-			c->_softhangup &= ~AST_SOFTHANGUP_ASYNCGOTO;
+			ast_channel_clear_softhangup(c, AST_SOFTHANGUP_ASYNCGOTO);
 		} else {
 			if (!digit)	/* No entry */
 				break;
@@ -2474,7 +2474,7 @@
 				if (option_verbose > 1)
 					ast_verbose( VERBOSE_PREFIX_2 "Spawn extension (%s, %s, %d) exited non-zero on '%s'\n", c->context, c->exten, c->priority, c->name);
 				if (c->_softhangup & AST_SOFTHANGUP_ASYNCGOTO) {
-					c->_softhangup &= ~AST_SOFTHANGUP_ASYNCGOTO;
+					ast_channel_clear_softhangup(c, AST_SOFTHANGUP_ASYNCGOTO);
 				} else if (c->_softhangup & AST_SOFTHANGUP_TIMEOUT) {
 					/* atimeout, nothing bad */
 				} else {
@@ -2485,12 +2485,12 @@
 				}
 			}
 			if (c->_softhangup & AST_SOFTHANGUP_ASYNCGOTO) {
-				c->_softhangup &= ~AST_SOFTHANGUP_ASYNCGOTO;
+				ast_channel_clear_softhangup(c, AST_SOFTHANGUP_ASYNCGOTO);
 			} else if (c->_softhangup & AST_SOFTHANGUP_TIMEOUT && ast_exists_extension(c,c->context,"T",1,c->cid.cid_num)) {
 				set_ext_pri(c, "T", 0); /* 0 will become 1 with the c->priority++; at the end */
 				/* If the AbsoluteTimeout is not reset to 0, we'll get an infinite loop */
 				c->whentohangup = 0;
-				c->_softhangup &= ~AST_SOFTHANGUP_TIMEOUT;
+				ast_channel_clear_softhangup(c, AST_SOFTHANGUP_ASYNCGOTO);
 			} else if (c->_softhangup) {
 				if (option_debug)
 					ast_log(LOG_DEBUG, "Extension %s, priority %d returned normally even though call was hung up\n",
@@ -2522,7 +2522,7 @@
 			}
 		} else if (c->_softhangup & AST_SOFTHANGUP_TIMEOUT) {
 			/* If we get this far with AST_SOFTHANGUP_TIMEOUT, then we know that the "T" extension is next. */
-			c->_softhangup &= ~AST_SOFTHANGUP_TIMEOUT;
+			ast_channel_clear_softhangup(c, AST_SOFTHANGUP_TIMEOUT);
 		} else {	/* keypress received, get more digits for a full extension */
 			int waittime = 0;
 			if (digit)

Modified: branches/1.4/res/res_features.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/res/res_features.c?view=diff&rev=303546&r1=303545&r2=303546
==============================================================================
--- branches/1.4/res/res_features.c (original)
+++ branches/1.4/res/res_features.c Mon Jan 24 14:32:21 2011
@@ -419,6 +419,7 @@
 	xferchan->_state = AST_STATE_UP;
 	ast_clear_flag(xferchan, AST_FLAGS_ALL);	
 	xferchan->_softhangup = 0;
+	ast_channel_clear_softhangup(xferchan, AST_SOFTHANGUP_ALL);
 	if ((f = ast_read(xferchan))) {
 		ast_frfree(f);
 		f = NULL;




More information about the asterisk-commits mailing list