Two fixes:

1. Make the list of ast_dial_channels a lockable list. This is because in some cases,
   the ast_dial may exist in multiple threads due to asynchronous execution of its application, and
   I found some cases where race conditions could exist.

2. Check in ast_dial_join to be sure that the channel still exists before attempting to lock it, since
   it could have gotten hung up but the is_running_app flag on the ast_dial_channel may not have been
   cleared yet.

(closes issue #12038)
Reported by: jvandal
Patches:
      12038v2.patch uploaded by putnopvut (license 60)
Tested by: jvandal



git-svn-id: http://svn.digium.com/svn/asterisk/branches/1.4@104841 f38db490-d61c-443f-a65b-d21fe96a405b

1 files changed, 31 insertions, 5 deletions

diff --git a/main/dial.c b/main/dial.c
index 2ca0fd0d2..807ba12c3 100644
--- a/main/dial.c
+++ b/main/dial.c
@@ -50,7 +50,7 @@ struct ast_dial {
 	enum ast_dial_result state;                       /*! Status of dial */
 	void *options[AST_DIAL_OPTION_MAX];                /*! Global options */
 	ast_dial_state_callback state_callback;          /*! Status callback */
-	AST_LIST_HEAD_NOLOCK(, ast_dial_channel) channels; /*! Channels being dialed */
+	AST_LIST_HEAD(, ast_dial_channel) channels; /*! Channels being dialed */
 	pthread_t thread;                                  /*! Thread (if running in async) */
 	ast_mutex_t lock;                                  /*! Lock to protect the thread information above */
 };
@@ -189,7 +189,7 @@ struct ast_dial *ast_dial_create(void)
 		return NULL;
 
 	/* Initialize list of channels */
-	AST_LIST_HEAD_INIT_NOLOCK(&dial->channels);
+	AST_LIST_HEAD_INIT(&dial->channels);
 
 	/* Initialize thread to NULL */
 	dial->thread = AST_PTHREADT_NULL;
@@ -236,6 +236,7 @@ static int begin_dial(struct ast_dial *dial, struct ast_channel *chan)
 	int success = 0, res = 0;
 
 	/* Iterate through channel list, requesting and calling each one */
+	AST_LIST_LOCK(&dial->channels);
 	AST_LIST_TRAVERSE(&dial->channels, channel, list) {
 		char numsubst[AST_MAX_EXTENSION];
 
@@ -285,6 +286,7 @@ static int begin_dial(struct ast_dial *dial, struct ast_channel *chan)
 				ast_verbose(VERBOSE_PREFIX_3 "Called %s\n", numsubst);
 		}
 	}
+	AST_LIST_UNLOCK(&dial->channels);
 
 	/* If number of failures matches the number of channels, then this truly failed */
 	return success;
@@ -295,10 +297,12 @@ static struct ast_dial_channel *find_relative_dial_channel(struct ast_dial *dial
 {
 	struct ast_dial_channel *channel = NULL;
 
+	AST_LIST_LOCK(&dial->channels);
 	AST_LIST_TRAVERSE(&dial->channels, channel, list) {
 		if (channel->owner == owner)
 			break;
 	}
+	AST_LIST_UNLOCK(&dial->channels);
 
 	return channel;
 }
@@ -319,8 +323,10 @@ static void handle_frame(struct ast_dial *dial, struct ast_dial_channel *channel
 		case AST_CONTROL_ANSWER:
 			if (option_verbose > 2)
 				ast_verbose( VERBOSE_PREFIX_3 "%s answered %s\n", channel->owner->name, chan->name);
+			AST_LIST_LOCK(&dial->channels);
 			AST_LIST_REMOVE(&dial->channels, channel, list);
 			AST_LIST_INSERT_HEAD(&dial->channels, channel, list);
+			AST_LIST_UNLOCK(&dial->channels);
 			set_state(dial, AST_DIAL_RESULT_ANSWERED);
 			break;
 		case AST_CONTROL_BUSY:
@@ -394,8 +400,10 @@ static void handle_frame_ownerless(struct ast_dial *dial, struct ast_dial_channe
 	case AST_CONTROL_ANSWER:
 		if (option_verbose > 2)
 			ast_verbose( VERBOSE_PREFIX_3 "%s answered\n", channel->owner->name);
+		AST_LIST_LOCK(&dial->channels);
 		AST_LIST_REMOVE(&dial->channels, channel, list);
 		AST_LIST_INSERT_HEAD(&dial->channels, channel, list);
+		AST_LIST_UNLOCK(&dial->channels);
 		set_state(dial, AST_DIAL_RESULT_ANSWERED);
 		break;
 	case AST_CONTROL_BUSY:
@@ -460,12 +468,14 @@ static enum ast_dial_result monitor_dial(struct ast_dial *dial, struct ast_chann
 			cs[pos++] = chan;
 
 		/* Add channels we are attempting to dial */
+		AST_LIST_LOCK(&dial->channels);
 		AST_LIST_TRAVERSE(&dial->channels, channel, list) {
 			if (channel->owner) {
 				cs[pos++] = channel->owner;
 				count++;
 			}
 		}
+		AST_LIST_UNLOCK(&dial->channels);
 
 		/* If we have no outbound channels in progress, switch state to unanswered and stop */
 		if (!count) {
@@ -517,12 +527,14 @@ static enum ast_dial_result monitor_dial(struct ast_dial *dial, struct ast_chann
 	/* Do post-processing from loop */
 	if (dial->state == AST_DIAL_RESULT_ANSWERED) {
 		/* Hangup everything except that which answered */
+		AST_LIST_LOCK(&dial->channels);
 		AST_LIST_TRAVERSE(&dial->channels, channel, list) {
 			if (!channel->owner || channel->owner == who)
 				continue;
 			ast_hangup(channel->owner);
 			channel->owner = NULL;
 		}
+		AST_LIST_UNLOCK(&dial->channels);
 		/* If ANSWER_EXEC is enabled as an option, execute application on answered channel */
 		if ((channel = find_relative_dial_channel(dial, who)) && (answer_exec = FIND_RELATIVE_OPTION(dial, channel, AST_DIAL_OPTION_ANSWER_EXEC))) {
 			channel->is_running_app = 1;
@@ -531,12 +543,14 @@ static enum ast_dial_result monitor_dial(struct ast_dial *dial, struct ast_chann
 		}
 	} else if (dial->state == AST_DIAL_RESULT_HANGUP) {
 		/* Hangup everything */
+		AST_LIST_LOCK(&dial->channels);
 		AST_LIST_TRAVERSE(&dial->channels, channel, list) {
 			if (!channel->owner)
 				continue;
 			ast_hangup(channel->owner);
 			channel->owner = NULL;
 		}
+		AST_LIST_UNLOCK(&dial->channels);
 	}
 
 	return dial->state;
@@ -636,15 +650,19 @@ enum ast_dial_result ast_dial_join(struct ast_dial *dial)
 	dial->thread = AST_PTHREADT_STOP;
 
 	/* If the answered channel is running an application we have to soft hangup it, can't just poke the thread */
+	AST_LIST_LOCK(&dial->channels);
 	if (AST_LIST_FIRST(&dial->channels)->is_running_app) {
 		struct ast_channel *chan = AST_LIST_FIRST(&dial->channels)->owner;
-		ast_channel_lock(chan);
-		ast_softhangup(chan, AST_SOFTHANGUP_EXPLICIT);
-		ast_channel_unlock(chan);
+		if (chan) {
+			ast_channel_lock(chan);
+			ast_softhangup(chan, AST_SOFTHANGUP_EXPLICIT);
+			ast_channel_unlock(chan);
+		}
 	} else {
 		/* Now we signal it with SIGURG so it will break out of it's waitfor */
 		pthread_kill(thread, SIGURG);
 	}
+	AST_LIST_UNLOCK(&dial->channels);
 
 	/* Yay done with it */
 	ast_mutex_unlock(&dial->lock);
@@ -669,12 +687,14 @@ void ast_dial_hangup(struct ast_dial *dial)
 	if (!dial)
 		return;
 	
+	AST_LIST_LOCK(&dial->channels);
 	AST_LIST_TRAVERSE(&dial->channels, channel, list) {
 		if (channel->owner) {
 			ast_hangup(channel->owner);
 			channel->owner = NULL;
 		}
 	}
+	AST_LIST_UNLOCK(&dial->channels);
 
 	return;
 }
@@ -693,6 +713,7 @@ int ast_dial_destroy(struct ast_dial *dial)
 		return -1;
 	
 	/* Hangup and deallocate all the dialed channels */
+	AST_LIST_LOCK(&dial->channels);
 	AST_LIST_TRAVERSE_SAFE_BEGIN(&dial->channels, channel, list) {
 		/* Disable any enabled options */
 		for (i = 0; i < AST_DIAL_OPTION_MAX; i++) {
@@ -712,6 +733,7 @@ int ast_dial_destroy(struct ast_dial *dial)
 		free(channel);
 	}
 	AST_LIST_TRAVERSE_SAFE_END;
+	AST_LIST_UNLOCK(&dial->channels);
        
 	/* Disable any enabled options globally */
 	for (i = 0; i < AST_DIAL_OPTION_MAX; i++) {
@@ -768,6 +790,7 @@ int ast_dial_option_enable(struct ast_dial *dial, int num, enum ast_dial_option
 		return -1;
 	
 	/* Look for channel, we can sort of cheat and predict things - the last channel in the list will probably be what they want */
+	AST_LIST_LOCK(&dial->channels);
 	if (AST_LIST_LAST(&dial->channels)->num != num) {
 		AST_LIST_TRAVERSE(&dial->channels, channel, list) {
 			if (channel->num == num)
@@ -776,6 +799,7 @@ int ast_dial_option_enable(struct ast_dial *dial, int num, enum ast_dial_option
 	} else {
 		channel = AST_LIST_LAST(&dial->channels);
 	}
+	AST_LIST_UNLOCK(&dial->channels);
 
 	/* If none found, return failure */
 	if (!channel)
@@ -830,6 +854,7 @@ int ast_dial_option_disable(struct ast_dial *dial, int num, enum ast_dial_option
 		return -1;
 
 	/* Look for channel, we can sort of cheat and predict things - the last channel in the list will probably be what they want */
+	AST_LIST_LOCK(&dial->channels);
 	if (AST_LIST_LAST(&dial->channels)->num != num) {
 		AST_LIST_TRAVERSE(&dial->channels, channel, list) {
 			if (channel->num == num)
@@ -838,6 +863,7 @@ int ast_dial_option_disable(struct ast_dial *dial, int num, enum ast_dial_option
 	} else {
 		channel = AST_LIST_LAST(&dial->channels);
 	}
+	AST_LIST_UNLOCK(&dial->channels);
 
 	/* If none found, return failure */
 	if (!channel)