Commit 209f8f012e for asterisk.org
commit 209f8f012e83e079eef6663aac6496337c8ba4fe
Author: smtcbn <samet109.06@gmail.com>
Date: Fri Jan 23 17:31:44 2026 +0300
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