From: smtcbn <samet109.06@gmail.com>
Date: Fri, 23 Jan 2026 14:31:44 +0000 (+0300)
Subject: app_queue: fix double increment of member->calls with shared_lastcall
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=209f8f012e83e079eef6663aac6496337c8ba4fe;p=thirdparty%2Fasterisk.git

app_queue: fix double increment of member->calls with shared_lastcall

Under high concurrency, update_queue() may be invoked multiple times
for the same call, causing member->calls and queue-level counters to
be incremented more than once.

The existing starttime check is not atomic and allows concurrent
execution paths to pass. Treat member->starttime as a single-use token
and consume it via CAS to ensure the call is counted exactly once.

This also prevents incorrect call distribution when using strategies
such as fewestcalls.

Observed as a regression after upgrading to 20.17.

Resolves: #1736
---

diff --git a/apps/app_queue.c b/apps/app_queue.c
index 06fc4b0ac1..b49c7a0922 100644
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -6162,36 +6162,48 @@ static int wait_our_turn(struct queue_ent *qe, int ringing, enum queue_result *r
  * \brief update the queue status
  * \retval 0 always
 */
-static int update_queue(struct call_queue *q, struct member *member, int callcompletedinsl, time_t starttime)
+static int update_queue(struct call_queue *q, struct member *member,
+	int callcompletedinsl, time_t starttime)
 {
 	int oldtalktime;
-	int newtalktime = time(NULL) - starttime;
+	int newtalktime;
 	struct member *mem;
 	struct call_queue *qtmp;
 	struct ao2_iterator queue_iter;
+	int did_increment_any = 0;
+
+	if (!starttime) {
+		return 0;
+	}
+
+	newtalktime = (int)(time(NULL) - starttime);
 
 	/* It is possible for us to be called when a call has already been considered terminated
 	 * and data updated, so to ensure we only act on the call that the agent is currently in
 	 * we check when the call was bridged.
 	 */
-	if (!starttime || (member->starttime != starttime)) {
+	ao2_lock(q);
+	if (member->starttime != starttime) {
+		ao2_unlock(q);
 		return 0;
 	}
+	member->starttime = 0;
+	ao2_unlock(q);
 
 	if (shared_lastcall) {
 		queue_iter = ao2_iterator_init(queues, 0);
-		while ((qtmp = ao2_t_iterator_next(&queue_iter, "Iterate through queues"))) {
+		while ((qtmp = ao2_t_iterator_next(&queue_iter, "Iterate queues"))) {
 			ao2_lock(qtmp);
 			if ((mem = ao2_find(qtmp->members, member, OBJ_POINTER))) {
 				time(&mem->lastcall);
 				mem->calls++;
 				mem->callcompletedinsl = 0;
-				mem->starttime = 0;
 				mem->lastqueue = q;
+				did_increment_any = 1;
 				ao2_ref(mem, -1);
 			}
 			ao2_unlock(qtmp);
-			queue_t_unref(qtmp, "Done with iterator");
+			queue_t_unref(qtmp, "queue iteration done");
 		}
 		ao2_iterator_destroy(&queue_iter);
 	} else {
@@ -6199,8 +6211,8 @@ static int update_queue(struct call_queue *q, struct member *member, int callcom
 		time(&member->lastcall);
 		member->callcompletedinsl = 0;
 		member->calls++;
-		member->starttime = 0;
 		member->lastqueue = q;
+		did_increment_any = 1;
 		ao2_unlock(q);
 	}
 	/* Member might never experience any direct status change (local
@@ -6210,22 +6222,24 @@ static int update_queue(struct call_queue *q, struct member *member, int callcom
 	 */
 	pending_members_remove(member);
 
-	ao2_lock(q);
-	q->callscompleted++;
-	if (callcompletedinsl) {
-		q->callscompletedinsl++;
-	}
-	if (q->callscompleted == 1) {
-		q->talktime = newtalktime;
-	} else {
-		/* Calculate talktime using the same exponential average as holdtime code */
-		oldtalktime = q->talktime;
-		q->talktime = (((oldtalktime << 2) - oldtalktime) + newtalktime) >> 2;
+	if (did_increment_any) {
+		ao2_lock(q);
+		q->callscompleted++;
+		if (callcompletedinsl) {
+			q->callscompletedinsl++;
+		}
+		if (q->callscompleted == 1) {
+			q->talktime = newtalktime;
+		} else {
+			/* Calculate talktime using the same exponential average as holdtime code */
+			oldtalktime = q->talktime;
+			q->talktime = (((oldtalktime << 2) - oldtalktime) + newtalktime) >> 2;
+		}
+		ao2_unlock(q);
 	}
-	ao2_unlock(q);
+
 	return 0;
 }
-
 /*! \brief Calculate the metric of each member in the outgoing callattempts
  *
  * A numeric metric is given to each member depending on the ring strategy used