[Asterisk-code-review] app_queue: Read latest wrapuptime instead of (possibly stale) copy (asterisk[13])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/14562 )

Change subject: app_queue: Read latest wrapuptime instead of (possibly stale) copy
......................................................................

app_queue: Read latest wrapuptime instead of (possibly stale) copy

Before this changeset, it was possible that a queue member (agent) was
called even though they just got out of a call, and wrapuptime seconds
hadn't passed yet.

This could happen if a member ended a call _between_ a new call attempt
and asterisk trying that particular member for a new call.

In that case, Asterisk would check the hangup time of the
call-before-the-last-call instead of the hangup time of the-last-call.

ASTERISK-28952

Change-Id: Ie0cab8f0e8d639c01cba633d4968ba19873d80b3
---
M apps/app_queue.c
1 file changed, 15 insertions(+), 17 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit



diff --git a/apps/app_queue.c b/apps/app_queue.c
index 1580734..5d2721b 100644
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -1443,8 +1443,6 @@
 	struct ast_channel *chan;
 	char interface[256];			/*!< An Asterisk dial string (not a channel name) */
 	int metric;
-	time_t lastcall;
-	struct call_queue *lastqueue;
 	struct member *member;
 	/*! Saved connected party info from an AST_CONTROL_CONNECTED_LINE. */
 	struct ast_party_connected_line connected;
@@ -4236,36 +4234,38 @@
  */
 static int can_ring_entry(struct queue_ent *qe, struct callattempt *call)
 {
-	if (call->member->paused) {
+	struct member *memberp = call->member;
+
+	if (memberp->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)) {
+	if (!memberp->ringinuse && !member_status_available(memberp->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))) {
+	if ((memberp->lastqueue && memberp->lastqueue->wrapuptime && (time(NULL) - memberp->lastcall < memberp->lastqueue->wrapuptime))
+		|| (!memberp->lastqueue && qe->parent->wrapuptime && (time(NULL) - memberp->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),
+			(memberp->lastqueue ? memberp->lastqueue->name : qe->parent->name),
 			call->interface);
 		return 0;
 	}
 
-	if (use_weight && compare_weight(qe->parent, call->member)) {
+	if (use_weight && compare_weight(qe->parent, memberp)) {
 		ast_debug(1, "Priority queue delaying call to %s:%s\n",
 			qe->parent->name, call->interface);
 		return 0;
 	}
 
-	if (!call->member->ringinuse) {
+	if (!memberp->ringinuse) {
 		struct member *mem;
 
 		ao2_lock(pending_members);
 
-		mem = ao2_find(pending_members, call->member,
+		mem = ao2_find(pending_members, memberp,
 				  OBJ_SEARCH_OBJECT | OBJ_NOLOCK);
 		if (mem) {
 			/*
@@ -4283,8 +4283,8 @@
 		 * If not found add it to the container so another queue
 		 * won't attempt to call this member at the same time.
 		 */
-		ast_debug(3, "Add %s to pending_members\n", call->member->membername);
-		ao2_link(pending_members, call->member);
+		ast_debug(3, "Add %s to pending_members\n", memberp->membername);
+		ao2_link(pending_members, memberp);
 		ao2_unlock(pending_members);
 
 		/*
@@ -4292,10 +4292,10 @@
 		 * 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))) {
+		if (!member_status_available(get_queue_member_status(memberp))) {
 			ast_debug(1, "%s actually not available, can't receive call\n",
 				call->interface);
-			pending_members_remove(call->member);
+			pending_members_remove(memberp);
 			return 0;
 		}
 	}
@@ -6638,9 +6638,7 @@
 
 		tmp->block_connected_update = block_connected_line;
 		tmp->stillgoing = 1;
-		tmp->member = cur;/* Place the reference for cur into callattempt. */
-		tmp->lastcall = cur->lastcall;
-		tmp->lastqueue = cur->lastqueue;
+		tmp->member = cur; /* Place the reference for cur into callattempt. */
 		ast_copy_string(tmp->interface, cur->interface, sizeof(tmp->interface));
 		/* Calculate the metric for the appropriate strategy. */
 		if (!calc_metric(qe->parent, cur, x++, qe, tmp)) {

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14562
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: Ie0cab8f0e8d639c01cba633d4968ba19873d80b3
Gerrit-Change-Number: 14562
Gerrit-PatchSet: 2
Gerrit-Owner: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200617/192c291c/attachment-0001.html>