The magic trick to avoid this crash is not to

try to find the channel by name in the list,
which is slow and resource consuming, but rather
to pay attention to the result codes from the
ast_bridge_call, to which I added the 
AST_PBX_NO_HANGUP_PEER_PARKED value, which
now are returned when a channel is parked.

If you get AST_PBX_KEEPALIVE,
then don't touch the channel pointer.

If you get AST_PBX_NO_HANGUP_PEER, or
AST_PBX_NO_HANGUP_PEER_PARKED, then don't
touch the peer pointer.

Updated the several places where the results
from a bridge were not being properly obeyed,
and fixed some code I had introduced so that
the results of the bridge were not overridden 
(in trunk).

All the places that previously tested for 
AST_PBX_NO_HANGUP_PEER now have to check for
both AST_PBX_NO_HANGUP_PEER and AST_PBX_NO_HANGUP_PEER_PARKED.

I tested this against the 4 common parking
scenarios:


1. A calls B; B answers; A parks B; B hangs up while A is getting the parking
slot announcement, immediately after being put on hold.

2. A calls B; B answers; A parks B; B hangs up after A has been hung up, but
before the park times out.

3. A calls B; B answers; B parks A; A hangs up while B is getting the parking slot announcement, immediately after being put on hold.

4. A calls B; B answers; B parks A; A hangs up after B has been hung up, but before the park times out.


No crash.

I also ran the scenarios above against valgrind, and accesses looked good.




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

2 files changed, 30 insertions, 26 deletions

diff --git a/apps/app_dial.c b/apps/app_dial.c
index 6c835fcec..97186f275 100644
--- a/apps/app_dial.c
+++ b/apps/app_dial.c
@@ -1788,7 +1788,7 @@ static int dial_exec_full(struct ast_channel *chan, void *data, struct ast_flags
 			}
 			res = ast_bridge_call(chan,peer,&config);
 			time(&end_time);
-			{
+			if (res != AST_PBX_KEEPALIVE) { /* if keepalive is set, don't even think about accessing chan! */
 				char toast[80];
 				snprintf(toast, sizeof(toast), "%ld", (long)(end_time - answer_time));
 				pbx_builtin_setvar_helper(chan, "ANSWEREDTIME", toast);
@@ -1797,36 +1797,40 @@ static int dial_exec_full(struct ast_channel *chan, void *data, struct ast_flags
 			time(&end_time);
 			res = -1;
 		}
-		{
+		if (res != AST_PBX_KEEPALIVE) { /* if keepalive is set, don't even think about accessing chan! */
 			char toast[80];
 			snprintf(toast, sizeof(toast), "%ld", (long)(end_time - start_time));
 			pbx_builtin_setvar_helper(chan, "DIALEDTIME", toast);
 		}
-		
-		if (res != AST_PBX_NO_HANGUP_PEER) {
-			if (!chan->_softhangup)
+		if (res != AST_PBX_NO_HANGUP_PEER && res != AST_PBX_NO_HANGUP_PEER_PARKED) {
+			if (res != AST_PBX_KEEPALIVE && !chan->_softhangup)
 				chan->hangupcause = peer->hangupcause;
 			ast_hangup(peer);
 		}
 	}	
 out:
-	if (moh) {
-		moh = 0;
-		ast_moh_stop(chan);
-	} else if (sentringing) {
-		sentringing = 0;
-		ast_indicate(chan, -1);
-	}
-	ast_rtp_early_bridge(chan, NULL);
-	hanguptree(outgoing, NULL);
-	pbx_builtin_setvar_helper(chan, "DIALSTATUS", status);
-	if (option_debug)
-		ast_log(LOG_DEBUG, "Exiting with DIALSTATUS=%s.\n", status);
-	
-	if ((ast_test_flag(peerflags, OPT_GO_ON)) && (!chan->_softhangup) && (res != AST_PBX_KEEPALIVE)) {
-		if (calldurationlimit)
-			chan->whentohangup = 0;
-		res = 0;
+	/* cleaning up chan is not a good idea here if AST_PBX_KEEPALIVE
+	   is returned; chan will get the love it needs from another
+	   thread */
+	if (res != AST_PBX_KEEPALIVE) {
+		if (moh) {
+			moh = 0;
+			ast_moh_stop(chan);
+		} else if (sentringing) {
+			sentringing = 0;
+			ast_indicate(chan, -1);
+		}
+		ast_rtp_early_bridge(chan, NULL);
+		hanguptree(outgoing, NULL);
+		pbx_builtin_setvar_helper(chan, "DIALSTATUS", status);
+		if (option_debug)
+			ast_log(LOG_DEBUG, "Exiting with DIALSTATUS=%s.\n", status);
+		
+		if ((ast_test_flag(peerflags, OPT_GO_ON)) && (!chan->_softhangup) && (res != AST_PBX_KEEPALIVE)) {
+			if (calldurationlimit)
+				chan->whentohangup = 0;
+			res = 0;
+		}
 	}
 
 done:
diff --git a/apps/app_queue.c b/apps/app_queue.c
index f238c1a12..dfb148c9c 100644
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -3143,7 +3143,7 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
 		setup_transfer_datastore(qe, member, callstart, callcompletedinsl);
 		bridge = ast_bridge_call(qe->chan,peer, &bridge_config);
 
-		if (!attended_transfer_occurred(qe->chan)) {
+		if (bridge != AST_PBX_KEEPALIVE && !attended_transfer_occurred(qe->chan)) {
 			struct ast_datastore *transfer_ds;
 			if (strcasecmp(oldcontext, qe->chan->context) || strcasecmp(oldexten, qe->chan->exten)) {
 				ast_queue_log(queuename, qe->chan->uniqueid, member->membername, "TRANSFER", "%s|%s|%ld|%ld",
@@ -3152,7 +3152,7 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
 			} else if (qe->chan->_softhangup) {
 				ast_queue_log(queuename, qe->chan->uniqueid, member->membername, "COMPLETECALLER", "%ld|%ld|%d",
 					(long) (callstart - qe->start), (long) (time(NULL) - callstart), qe->opos);
-				if (qe->parent->eventwhencalled)
+				if (bridge != AST_PBX_NO_HANGUP_PEER && bridge != AST_PBX_NO_HANGUP_PEER_PARKED && qe->parent->eventwhencalled)
 					manager_event(EVENT_FLAG_AGENT, "AgentComplete",
 							"Queue: %s\r\n"
 							"Uniqueid: %s\r\n"
@@ -3169,7 +3169,7 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
 			} else {
 				ast_queue_log(queuename, qe->chan->uniqueid, member->membername, "COMPLETEAGENT", "%ld|%ld|%d",
 					(long) (callstart - qe->start), (long) (time(NULL) - callstart), qe->opos);
-				if (qe->parent->eventwhencalled)
+				if (bridge != AST_PBX_NO_HANGUP_PEER && bridge != AST_PBX_NO_HANGUP_PEER_PARKED && qe->parent->eventwhencalled)
 					manager_event(EVENT_FLAG_AGENT, "AgentComplete",
 							"Queue: %s\r\n"
 							"Uniqueid: %s\r\n"
@@ -3193,7 +3193,7 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
 			update_queue(qe->parent, member, callcompletedinsl);
 		}
 
-		if (bridge != AST_PBX_NO_HANGUP_PEER)
+		if (bridge != AST_PBX_NO_HANGUP_PEER && bridge != AST_PBX_NO_HANGUP_PEER_PARKED)
 			ast_hangup(peer);
 		res = bridge ? bridge : 1;
 		ao2_ref(member, -1);