app_queue: queue members can receive multiple calls

It was possible for a queue member that is a member of at least 2 or more
queues to receive mulitiple calls at the same time. This happened because
of a race between when a member was being rung and when the device state
notified the other queue(s) member object of the state change.

This patch makes it so when a queue member is being rung it gets added to
a global pool of queue members. If that same member is tried again, e.g.
from another queue, and it is found to already exist in the pending member
container then it will not ring that member.

ASTERISK-16115 #close

Change-Id: Ice45a1c95b9f6f15d8a9fa709c5e5c84ffd29780

diff --git a/apps/app_queue.c b/apps/app_queue.c
index fa432cb76f5e41ad1168b73d3cd415daf3c851c3..2beb3dbf0b66a23c1148d1f4c1a3fc9479361ab1 100644 (file)
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -1212,7 +1212,6 @@ struct member {
        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' */
 };
@@ -1675,6 +1674,38 @@ static int get_member_status(struct call_queue *q, int max_penalty, int min_pena
        return -1;
 }
 
+/*
+ * A "pool" of member objects that calls are currently pending on. If an
+ * agent is a member of multiple queues it's possible for that agent to be
+ * called by each of the queues at the same time. This happens because device
+ * state is slow to notify the queue app of one of it's member's being rung.
+ * This "pool" allows us to track which members are currently being rung while
+ * we wait on the device state change.
+ */
+static struct ao2_container *pending_members;
+#define MAX_CALL_ATTEMPT_BUCKETS 353
+
+static int pending_members_hash(const void *obj, const int flags)
+{
+       const struct member *object = obj;
+       const char *key = (flags & OBJ_KEY) ? obj : object->interface;
+       return ast_str_case_hash(key);
+}
+
+static int pending_members_cmp(void *obj, void *arg, int flags)
+{
+       const struct member *object_left = obj;
+       const struct member *object_right = arg;
+       const char *right_key = (flags & OBJ_KEY) ? arg : object_right->interface;
+
+       return strcasecmp(object_left->interface, right_key) ? 0 : CMP_MATCH;
+}
+
+static void pending_members_remove(struct member *mem)
+{
+       ao2_find(pending_members, mem, OBJ_POINTER | OBJ_NODATA | OBJ_UNLINK);
+}
+
 struct statechange {
        AST_LIST_ENTRY(statechange) entry;
        int state;
@@ -1690,6 +1721,9 @@ static int update_status(struct call_queue *q, struct member *m, const int statu
 {
        m->status = status;
 
+       /* Whatever the status is clear the member from the pending members pool */
+       pending_members_remove(m);
+
        if (q->maskmemberstatus) {
                return 0;
        }
@@ -2548,6 +2582,7 @@ static void member_add_to_queue(struct call_queue *queue, struct member *mem)
  */
 static void member_remove_from_queue(struct call_queue *queue, struct member *mem)
 {
+       pending_members_remove(mem);
        ao2_lock(queue->members);
        queue_member_follower_removal(queue, mem);
        ao2_unlink(queue->members, mem);
@@ -3585,41 +3620,6 @@ 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.
@@ -3662,12 +3662,30 @@ static int can_ring_entry(struct queue_ent *qe, struct callattempt *call)
        }
 
        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);
+               struct member *mem;
+
+               ao2_lock(pending_members);
+
+               mem = ao2_find(pending_members, call->member, OBJ_POINTER | OBJ_NOLOCK);
+               if (mem) {
+                       /*
+                        * If found that means this member is currently being attempted
+                        * from another calling thread, so stop trying from this thread
+                        */
+                       ast_debug(1, "%s has another call trying, can't receive call\n",
+                                 call->interface);
+                       ao2_ref(mem, -1);
+                       ao2_unlock(pending_members);
                        return 0;
                }
 
+               /*
+                * If not found add it to the container so another queue
+                * won't attempt to call this member at the same time.
+                */
+               ao2_link(pending_members, call->member);
+               ao2_unlock(pending_members);
+
                /*
                 * The queue member is available.  Get current status to be sure
                 * because the device state and extension state callbacks may
@@ -3676,7 +3694,7 @@ static int can_ring_entry(struct queue_ent *qe, struct callattempt *call)
                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);
+                       pending_members_remove(call->member);
                        return 0;
                }
        }
@@ -3715,7 +3733,6 @@ static int ring_entry(struct queue_ent *qe, struct callattempt *tmp, int *busies
                ++*busies;
                return 0;
        }
-       ast_assert(tmp->member->ringinuse || tmp->member->call_pending);
 
        ast_copy_string(tech, tmp->interface, sizeof(tech));
        if ((location = strchr(tech, '/'))) {
@@ -3732,7 +3749,7 @@ static int ring_entry(struct queue_ent *qe, struct callattempt *tmp, int *busies
                qe->linpos++;
                ao2_unlock(qe->parent);
 
-               member_call_pending_clear(tmp->member);
+               pending_members_remove(tmp->member);
 
                if (ast_channel_cdr(qe->chan)) {
                        ast_cdr_busy(ast_channel_cdr(qe->chan));
@@ -3814,7 +3831,7 @@ static int ring_entry(struct queue_ent *qe, struct callattempt *tmp, int *busies
                /* Again, keep going even if there's an error */
                ast_verb(3, "Couldn't call %s\n", tmp->interface);
                do_hang(tmp);
-               member_call_pending_clear(tmp->member);
+               pending_members_remove(tmp->member);
                ++*busies;
                return 0;
        }
@@ -3875,7 +3892,6 @@ static int ring_entry(struct queue_ent *qe, struct callattempt *tmp, int *busies
                ast_verb(3, "Called %s\n", tmp->interface);
        }
 
-       member_call_pending_clear(tmp->member);
        return 1;
 }
 
@@ -10084,6 +10100,7 @@ static int unload_module(void)
        }
        ao2_iterator_destroy(&q_iter);
        devicestate_tps = ast_taskprocessor_unreference(devicestate_tps);
+       ao2_cleanup(pending_members);
        ao2_ref(queues, -1);
        ast_unload_realtime("queue_members");
        return res;
@@ -10096,6 +10113,16 @@ static int load_module(void)
        struct ast_config *member_config;
 
        queues = ao2_container_alloc(MAX_QUEUE_BUCKETS, queue_hash_cb, queue_cmp_cb);
+       if (!queues) {
+               return AST_MODULE_LOAD_DECLINE;
+       }
+
+       pending_members = ao2_container_alloc(
+               MAX_CALL_ATTEMPT_BUCKETS, pending_members_hash, pending_members_cmp);
+       if (!pending_members) {
+               unload_module();
+               return AST_MODULE_LOAD_DECLINE;
+       }
 
        use_weight = 0;