[asterisk-commits] rmudgett: branch 11 r378687 - in /branches/11: ./ apps/ configs/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Tue Jan 8 17:28:07 CST 2013


Author: rmudgett
Date: Tue Jan  8 17:28:03 2013
New Revision: 378687

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=378687
Log:
app_queue: Fix multiple calls to a queue member that is in only one queue.

When ringinuse=no queue members can receive more than one call if these
calls happen at nearly the same time.

* Fix so a queue member does not receive more than one call from a queue.

NOTE: This fix does not prevent multiple calls to a member if the member
is in more than one queue.

* Did some refactoring to eliminate some code redundancy.

(issue ASTERISK-16115)
Reported by: nik600
Patches:
      jira_asterisk_16115_single_q_v1.8.patch (license #5621) patch uploaded by rmudgett
      Modified

* Revert the -r341580 and -r341599 changes adding the queues.conf
check_state_unknown option as it was added in an attempt to fix this
problem.  The fix did not need to be optional.  The fix should not have
tried to explicitly set the device state.  Setting the device state by
something other than the device introduces a race condition.  I also could
not see how the change would be effective other than delaying the
app_queue code long enough for the device state to propagate to app_queue.
........

Merged revisions 378663 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 378683 from http://svn.asterisk.org/svn/asterisk/branches/10

Modified:
    branches/11/   (props changed)
    branches/11/CHANGES
    branches/11/UPGRADE.txt
    branches/11/apps/app_queue.c
    branches/11/configs/queues.conf.sample

Propchange: branches/11/
------------------------------------------------------------------------------
Binary property 'branch-10-merged' - no diff available.

Modified: branches/11/CHANGES
URL: http://svnview.digium.com/svn/asterisk/branches/11/CHANGES?view=diff&rev=378687&r1=378686&r2=378687
==============================================================================
--- branches/11/CHANGES (original)
+++ branches/11/CHANGES Tue Jan  8 17:28:03 2013
@@ -901,8 +901,6 @@
  * Added member option ignorebusy this when set and ringinuse is not
    will allow per member control of multiple calls as ringinuse does for
    the Queue.
- * Added global option check_state_unknown to enforce checking of device state
-   when the device state is unknown app_queue will see unknown as available.
 
 Applications
 ------------

Modified: branches/11/UPGRADE.txt
URL: http://svnview.digium.com/svn/asterisk/branches/11/UPGRADE.txt?view=diff&rev=378687&r1=378686&r2=378687
==============================================================================
--- branches/11/UPGRADE.txt (original)
+++ branches/11/UPGRADE.txt Tue Jan  8 17:28:03 2013
@@ -25,6 +25,9 @@
 * Asterisk has always had code to ignore dash '-' characters that are not
   part of a character set in the dialplan extensions.  The code now
   consistently ignores these characters when matching dialplan extensions.
+
+* Removed the queues.conf check_state_unknown option.  It is no longer
+  necessary.
 
 From 11.0 to 11.1:
 

Modified: branches/11/apps/app_queue.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/apps/app_queue.c?view=diff&rev=378687&r1=378686&r2=378687
==============================================================================
--- branches/11/apps/app_queue.c (original)
+++ branches/11/apps/app_queue.c Tue Jan  8 17:28:03 2013
@@ -1042,9 +1042,6 @@
 /*! \brief queues.conf [general] option */
 static int log_membername_as_agent = 0;
 
-/*! \brief queues.conf [general] option */
-static int check_state_unknown = 0;
-
 /*! \brief name of the ringinuse field in the realtime database */
 static char *realtime_ringinuse_field;
 
@@ -1160,6 +1157,7 @@
 	struct call_queue *lastqueue;	     /*!< Last queue we received a call */
 	unsigned int dead:1;                 /*!< Used to detect members deleted in realtime */
 	unsigned int delme:1;                /*!< Flag to delete entry on reload */
+	unsigned int call_pending:1;         /*!< TRUE if the Q is attempting to place a call to the member. */
 	char rt_uniqueid[80];                /*!< Unique id of realtime member entry */
 	unsigned int ringinuse:1;            /*!< Flag to ring queue members even if their status is 'inuse' */
 };
@@ -3488,6 +3486,112 @@
 	return vars;
 }
 
+/*!
+ * \internal
+ * \brief Check if the member status is available.
+ *
+ * \param status Member status to check if available.
+ *
+ * \retval non-zero if the member status is available.
+ */
+static int member_status_available(int status)
+{
+	return status == AST_DEVICE_NOT_INUSE || status == AST_DEVICE_UNKNOWN;
+}
+
+/*!
+ * \internal
+ * \brief Clear the member call pending flag.
+ *
+ * \param mem Queue member.
+ *
+ * \return Nothing
+ */
+static void member_call_pending_clear(struct member *mem)
+{
+	ao2_lock(mem);
+	mem->call_pending = 0;
+	ao2_unlock(mem);
+}
+
+/*!
+ * \internal
+ * \brief Set the member call pending flag.
+ *
+ * \param mem Queue member.
+ *
+ * \retval non-zero if call pending flag was already set.
+ */
+static int member_call_pending_set(struct member *mem)
+{
+	int old_pending;
+
+	ao2_lock(mem);
+	old_pending = mem->call_pending;
+	mem->call_pending = 1;
+	ao2_unlock(mem);
+
+	return old_pending;
+}
+
+/*!
+ * \internal
+ * \brief Determine if can ring a queue entry.
+ *
+ * \param qe Queue entry to check.
+ * \param call Member call attempt.
+ *
+ * \retval non-zero if an entry can be called.
+ */
+static int can_ring_entry(struct queue_ent *qe, struct callattempt *call)
+{
+	if (call->member->paused) {
+		ast_debug(1, "%s paused, can't receive call\n", call->interface);
+		return 0;
+	}
+
+	if (!call->member->ringinuse && !member_status_available(call->member->status)) {
+		ast_debug(1, "%s not available, can't receive call\n", call->interface);
+		return 0;
+	}
+
+	if ((call->lastqueue && call->lastqueue->wrapuptime && (time(NULL) - call->lastcall < call->lastqueue->wrapuptime))
+		|| (!call->lastqueue && qe->parent->wrapuptime && (time(NULL) - call->lastcall < qe->parent->wrapuptime))) {
+		ast_debug(1, "Wrapuptime not yet expired on queue %s for %s\n",
+			(call->lastqueue ? call->lastqueue->name : qe->parent->name),
+			call->interface);
+		return 0;
+	}
+
+	if (use_weight && compare_weight(qe->parent, call->member)) {
+		ast_debug(1, "Priority queue delaying call to %s:%s\n",
+			qe->parent->name, call->interface);
+		return 0;
+	}
+
+	if (!call->member->ringinuse) {
+		if (member_call_pending_set(call->member)) {
+			ast_debug(1, "%s has another call pending, can't receive call\n",
+				call->interface);
+			return 0;
+		}
+
+		/*
+		 * The queue member is available.  Get current status to be sure
+		 * because the device state and extension state callbacks may
+		 * not have updated the status yet.
+		 */
+		if (!member_status_available(get_queue_member_status(call->member))) {
+			ast_debug(1, "%s actually not available, can't receive call\n",
+				call->interface);
+			member_call_pending_clear(call->member);
+			return 0;
+		}
+	}
+
+	return 1;
+}
+
 /*! 
  * \brief Part 2 of ring_one
  *
@@ -3509,60 +3613,17 @@
 	char tech[256];
 	char *location;
 	const char *macrocontext, *macroexten;
-	enum ast_device_state newstate;
 
 	/* on entry here, we know that tmp->chan == NULL */
-	if (tmp->member->paused) {
-		ast_debug(1, "%s paused, can't receive call\n", tmp->interface);
+	if (!can_ring_entry(qe, tmp)) {
 		if (ast_channel_cdr(qe->chan)) {
 			ast_cdr_busy(ast_channel_cdr(qe->chan));
 		}
 		tmp->stillgoing = 0;
-		(*busies)++;
+		++*busies;
 		return 0;
 	}
-
-	if ((tmp->lastqueue && tmp->lastqueue->wrapuptime && (time(NULL) - tmp->lastcall < tmp->lastqueue->wrapuptime)) ||
-		(!tmp->lastqueue && qe->parent->wrapuptime && (time(NULL) - tmp->lastcall < qe->parent->wrapuptime))) {
-		ast_debug(1, "Wrapuptime not yet expired on queue %s for %s\n",
-				(tmp->lastqueue ? tmp->lastqueue->name : qe->parent->name), tmp->interface);
-		if (ast_channel_cdr(qe->chan)) {
-			ast_cdr_busy(ast_channel_cdr(qe->chan));
-		}
-		tmp->stillgoing = 0;
-		(*busies)++;
-		return 0;
-	}
-
-	if (!tmp->member->ringinuse) {
-		if (check_state_unknown && (tmp->member->status == AST_DEVICE_UNKNOWN)) {
-			newstate = ast_device_state(tmp->member->interface);
-			if (newstate != tmp->member->status) {
-				ast_log(LOG_WARNING, "Found a channel matching iterface %s while status was %s changed to %s\n",
-					tmp->member->interface, ast_devstate2str(tmp->member->status), ast_devstate2str(newstate));
-				ast_devstate_changed_literal(newstate, AST_DEVSTATE_CACHABLE, tmp->member->interface);
-			}
-		}
-		if ((tmp->member->status != AST_DEVICE_NOT_INUSE) && (tmp->member->status != AST_DEVICE_UNKNOWN)) {
-			ast_debug(1, "%s in use, can't receive call\n", tmp->interface);
-			if (ast_channel_cdr(qe->chan)) {
-				ast_cdr_busy(ast_channel_cdr(qe->chan));
-			}
-			tmp->stillgoing = 0;
-			(*busies)++;
-			return 0;
-		}
-	}
-
-	if (use_weight && compare_weight(qe->parent,tmp->member)) {
-		ast_debug(1, "Priority queue delaying call to %s:%s\n", qe->parent->name, tmp->interface);
-		if (ast_channel_cdr(qe->chan)) {
-			ast_cdr_busy(ast_channel_cdr(qe->chan));
-		}
-		tmp->stillgoing = 0;
-		(*busies)++;
-		return 0;
-	}
+	ast_assert(qe->parent->ringinuse || tmp->member->call_pending);
 
 	ast_copy_string(tech, tmp->interface, sizeof(tech));
 	if ((location = strchr(tech, '/'))) {
@@ -3574,18 +3635,18 @@
 	/* Request the peer */
 	tmp->chan = ast_request(tech, ast_channel_nativeformats(qe->chan), qe->chan, location, &status);
 	if (!tmp->chan) {			/* If we can't, just go on to the next call */
-		if (ast_channel_cdr(qe->chan)) {
-			ast_cdr_busy(ast_channel_cdr(qe->chan));
-		}
-		tmp->stillgoing = 0;
-
 		ao2_lock(qe->parent);
-		update_status(qe->parent, tmp->member, get_queue_member_status(tmp->member));
 		qe->parent->rrpos++;
 		qe->linpos++;
 		ao2_unlock(qe->parent);
 
-		(*busies)++;
+		member_call_pending_clear(tmp->member);
+
+		if (ast_channel_cdr(qe->chan)) {
+			ast_cdr_busy(ast_channel_cdr(qe->chan));
+		}
+		tmp->stillgoing = 0;
+		++*busies;
 		return 0;
 	}
 
@@ -3661,10 +3722,12 @@
 		/* Again, keep going even if there's an error */
 		ast_verb(3, "Couldn't call %s\n", tmp->interface);
 		do_hang(tmp);
-		(*busies)++;
-		update_status(qe->parent, tmp->member, get_queue_member_status(tmp->member));
+		member_call_pending_clear(tmp->member);
+		++*busies;
 		return 0;
-	} else if (qe->parent->eventwhencalled) {
+	}
+
+	if (qe->parent->eventwhencalled) {
 		char vars[2048];
 
 		ast_channel_lock_both(tmp->chan, qe->chan);
@@ -3720,7 +3783,7 @@
 		ast_verb(3, "Called %s\n", tmp->interface);
 	}
 
-	update_status(qe->parent, tmp->member, get_queue_member_status(tmp->member));
+	member_call_pending_clear(tmp->member);
 	return 1;
 }
 
@@ -7714,10 +7777,6 @@
 	if ((general_val = ast_variable_retrieve(cfg, "general", "log_membername_as_agent"))) {
 		log_membername_as_agent = ast_true(general_val);
 	}
-	check_state_unknown = 0;
-	if ((general_val = ast_variable_retrieve(cfg, "general", "check_state_unknown"))) {
-		check_state_unknown = ast_true(general_val);
-	}
 }
 
 /*! \brief reload information pertaining to a single member
@@ -8495,7 +8554,7 @@
 			while ((mem = ao2_iterator_next(&mem_iter))) {
 				if ((mem->status != AST_DEVICE_UNAVAILABLE) && (mem->status != AST_DEVICE_INVALID)) {
 					++qmemcount;
-					if (((mem->status == AST_DEVICE_NOT_INUSE) || (mem->status == AST_DEVICE_UNKNOWN)) && !(mem->paused)) {
+					if (member_status_available(mem->status) && !mem->paused) {
 						++qmemavail;
 					}
 				}

Modified: branches/11/configs/queues.conf.sample
URL: http://svnview.digium.com/svn/asterisk/branches/11/configs/queues.conf.sample?view=diff&rev=378687&r1=378686&r2=378687
==============================================================================
--- branches/11/configs/queues.conf.sample (original)
+++ branches/11/configs/queues.conf.sample Tue Jan  8 17:28:03 2013
@@ -70,13 +70,6 @@
 ; is set.  The default value (no) maintains backward compatibility.
 ;
 ;log_membername_as_agent = no
-;
-; app_queue allows calls to members in a "Unknown" state to be treated as available
-; setting check_state_unknown = yes will cause app_queue to query the channel driver
-; to better determine the state this only applies to queues with ringinuse set
-; appropriately.
-;
-;check_state_unknown = no
 ;
 ;[markq]
 ;




More information about the asterisk-commits mailing list