[Asterisk-code-review] app queue: Fix locking behavior in stasis message handlers (asterisk[13])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/5226 )

Change subject: app_queue: Fix locking behavior in stasis message handlers
......................................................................


app_queue: Fix locking behavior in stasis message handlers

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
---
M apps/app_queue.c
1 file changed, 16 insertions(+), 12 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/apps/app_queue.c b/apps/app_queue.c
index 65f1bf5..ddb62d2 100644
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -5884,6 +5884,7 @@
 {
 	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;
@@ -5947,7 +5948,7 @@
 	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);
@@ -5983,15 +5984,16 @@
 	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)) {
@@ -6040,16 +6042,17 @@
 	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;
 	}
 
 	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);
@@ -6234,11 +6237,12 @@
 	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;
@@ -6268,7 +6272,7 @@
 	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);

-- 
To view, visit https://gerrit.asterisk.org/5226
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I246b7dbff8447acc957a1299f6ad0ebd0fd39088
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>