app_queue: Fix locking behavior in stasis message handlers
authorSean Bright <sean.bright@gmail.com>
Thu, 16 Mar 2017 13:42:54 +0000 (09:42 -0400)
committerSean Bright <sean.bright@gmail.com>
Fri, 17 Mar 2017 14:22:53 +0000 (08:22 -0600)
The queue_stasis_data structure contains various mutable fields that require
appropriate locking. Specifically, the 'dying,' 'member_uniqueid,' and
'caller_uniqueid' fields need to be locked when read from or written to.

Change-Id: I246b7dbff8447acc957a1299f6ad0ebd0fd39088

apps/app_queue.c

index c0de001..3886b7c 100644 (file)
@@ -5948,6 +5948,7 @@ static void handle_bridge_enter(void *userdata, struct stasis_subscription *sub,
 {
        struct queue_stasis_data *queue_data = userdata;
        struct ast_bridge_blob *enter_blob = stasis_message_data(msg);
+       SCOPED_AO2LOCK(lock, queue_data);
 
        if (queue_data->dying) {
                return;
@@ -6011,7 +6012,7 @@ static void handle_bridge_left(void *userdata, struct stasis_subscription *sub,
        ast_debug(3, "Detected redirect of queue caller channel %s\n",
                caller_snapshot->name);
 
-       ast_queue_log(queue_data->queue->name, queue_data->caller_uniqueid, queue_data->member->membername,
+       ast_queue_log(queue_data->queue->name, caller_snapshot->uniqueid, queue_data->member->membername,
                "COMPLETECALLER", "%ld|%ld|%d",
                (long) (queue_data->starttime - queue_data->holdstart),
                (long) (time(NULL) - queue_data->starttime), queue_data->caller_pos);
@@ -6047,16 +6048,17 @@ static void handle_blind_transfer(void *userdata, struct stasis_subscription *su
        RAII_VAR(struct ast_channel_snapshot *, caller_snapshot, NULL, ao2_cleanup);
        RAII_VAR(struct ast_channel_snapshot *, member_snapshot, NULL, ao2_cleanup);
 
-       if (queue_data->dying) {
-               return;
-       }
-
        if (transfer_msg->result != AST_BRIDGE_TRANSFER_SUCCESS) {
                return;
        }
 
        ao2_lock(queue_data);
 
+       if (queue_data->dying) {
+               ao2_unlock(queue_data);
+               return;
+       }
+
        if (ast_strlen_zero(queue_data->bridge_uniqueid) ||
                        strcmp(queue_data->bridge_uniqueid, transfer_msg->bridge->uniqueid)) {
                ao2_unlock(queue_data);
@@ -6104,10 +6106,6 @@ static void handle_attended_transfer(void *userdata, struct stasis_subscription
        RAII_VAR(struct ast_channel_snapshot *, caller_snapshot, NULL, ao2_cleanup);
        RAII_VAR(struct ast_channel_snapshot *, member_snapshot, NULL, ao2_cleanup);
 
-       if (queue_data->dying) {
-               return;
-       }
-
        if (atxfer_msg->result != AST_BRIDGE_TRANSFER_SUCCESS ||
                        atxfer_msg->dest_type == AST_ATTENDED_TRANSFER_DEST_THREEWAY) {
                return;
@@ -6115,6 +6113,11 @@ static void handle_attended_transfer(void *userdata, struct stasis_subscription
 
        ao2_lock(queue_data);
 
+       if (queue_data->dying) {
+               ao2_unlock(queue_data);
+               return;
+       }
+
        if (ast_strlen_zero(queue_data->bridge_uniqueid)) {
                ao2_unlock(queue_data);
                return;
@@ -6298,12 +6301,13 @@ static void handle_hangup(void *userdata, struct stasis_subscription *sub,
        RAII_VAR(struct ast_channel *, chan, NULL, ao2_cleanup);
        enum agent_complete_reason reason;
 
+       ao2_lock(queue_data);
+
        if (queue_data->dying) {
+               ao2_unlock(queue_data);
                return;
        }
 
-       ao2_lock(queue_data);
-
        if (!strcmp(channel_blob->snapshot->uniqueid, queue_data->caller_uniqueid)) {
                reason = CALLER;
        } else if (!strcmp(channel_blob->snapshot->uniqueid, queue_data->member_uniqueid)) {
@@ -6332,7 +6336,7 @@ static void handle_hangup(void *userdata, struct stasis_subscription *sub,
        ast_debug(3, "Detected hangup of queue %s channel %s\n", reason == CALLER ? "caller" : "member",
                        channel_blob->snapshot->name);
 
-       ast_queue_log(queue_data->queue->name, queue_data->caller_uniqueid, queue_data->member->membername,
+       ast_queue_log(queue_data->queue->name, caller_snapshot->uniqueid, queue_data->member->membername,
                        reason == CALLER ? "COMPLETECALLER" : "COMPLETEAGENT", "%ld|%ld|%d",
                (long) (queue_data->starttime - queue_data->holdstart),
                (long) (time(NULL) - queue_data->starttime), queue_data->caller_pos);