[Asterisk-code-review] chan unistim: Fix hold function ability to lock/crash asterisk (asterisk[master])

[Igor Goncharovsky]

Igor Goncharovsky has uploaded this change for review. ( https://gerrit.asterisk.org/7882


Change subject: chan_unistim: Fix hold function ability to lock/crash asterisk
......................................................................

chan_unistim: Fix hold function ability to lock/crash asterisk

This patch fix chan_unistim hold functions to correctly support
hold function in different states possible in case of multiple lines
established on the phone

ASTERISK-26596 #close

Change-Id: Ib1e04e482e7c8939607a42d7fddacc07e26e14d4
---
M channels/chan_unistim.c
1 file changed, 45 insertions(+), 18 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/82/7882/1

diff --git a/channels/chan_unistim.c b/channels/chan_unistim.c
index fe947ed..becebd0 100644
--- a/channels/chan_unistim.c
+++ b/channels/chan_unistim.c
@@ -358,6 +358,7 @@
 	int softkey;			/*! Softkey assigned */
 	pthread_t ss_thread;		/*! unistim_ss thread handle */
 	int alreadygone;
+	int holding;			/*! this subchannel holds someone */
 	signed char ringvolume;
 	signed char ringstyle;
 	int moh;					/*!< Music on hold in progress */
@@ -2496,6 +2497,24 @@
 	return sub;
 }
 
+static struct unistim_subchannel* get_sub_holding(struct unistim_device *device, int type, int holding)
+{
+	struct unistim_subchannel *sub = NULL;
+
+	AST_LIST_LOCK(&device->subs);
+	AST_LIST_TRAVERSE(&device->subs, sub, list) {
+		if (!sub) {
+			continue;
+		}
+		if (sub->subtype == type && sub->holding == holding) {
+			break;
+		}
+	}
+	AST_LIST_UNLOCK(&device->subs);
+
+	return sub;
+}
+
 static void sub_start_silence(struct unistimsession *pte, struct unistim_subchannel *sub)
 {
 	/* Silence our channel */
@@ -2533,13 +2552,12 @@
 		return;
 	}
 	sub->moh = 1;
-	sub->subtype = SUB_ONHOLD;
+	sub->holding = 1;
 	send_favorite_short(sub->softkey, FAV_ICON_ONHOLD_BLACK + FAV_BLINK_SLOW, pte);
 	send_select_output(pte, pte->device->output, pte->device->volume, MUTE_ON);
 	send_stop_timer(pte);
 	if (sub->owner) {
 		ast_queue_hold(sub->owner, NULL);
-		send_end_call(pte);
 	}
 	return;
 }
@@ -2554,7 +2572,7 @@
 	}
 
 	sub->moh = 0;
-	sub->subtype = SUB_REAL;
+	sub->holding = 0;
 	send_favorite_short(sub->softkey, FAV_ICON_OFFHOOK_BLACK, pte);
 	send_select_output(pte, pte->device->output, pte->device->volume, MUTE_OFF);
 	send_start_timer(pte);
@@ -3355,12 +3373,12 @@
 static void handle_key_fav(struct unistimsession *pte, char keycode)
 {
 	int keynum = keycode - KEY_FAV0;
-	struct unistim_subchannel *sub;
-
-	sub = get_sub(pte->device, SUB_REAL);
-
+	struct unistim_subchannel *sub, *sub_key = NULL;
+	sub = get_sub_holding(pte->device, SUB_REAL, 0);
+	
 	/* Make an action on selected favorite key */
 	if (!pte->device->ssub[keynum]) { /* Key have no assigned call */
+		sub = get_sub_holding(pte->device, SUB_REAL, 0);
 		send_favorite_selected(FAV_LINE_ICON, pte);
 		if (is_key_line(pte->device, keynum)) {
 			if (unistimdebug) {
@@ -3385,22 +3403,25 @@
 			key_favorite(pte, keycode);
 		}
 	} else {
-		sub = pte->device->ssub[keynum];
+		sub_key = pte->device->ssub[keynum];
 		/* Favicon have assigned sub, activate it and put current on hold */
-		if (sub->subtype == SUB_REAL) {
-			sub_hold(pte, sub);
+		if (sub_key->subtype == SUB_REAL && !sub_key->holding) {
+			sub_hold(pte, sub_key);
 			show_main_page(pte);
-		} else if (sub->subtype == SUB_RING) {
-			sub->softkey = keynum;
-			handle_call_incoming(pte);
-		} else if (sub->subtype == SUB_ONHOLD) {
+		} else if (sub_key->subtype == SUB_REAL && sub_key->holding) { 
+			/* We are going to unhold line (we should put active line on hold, of any) */
 			if (pte->state == STATE_DIALPAGE){
 				send_tone(pte, 0, 0);
 			}
-			send_callerid_screen(pte, sub);
-			sub_unhold(pte, sub);
+			sub_hold(pte, sub);
+			send_callerid_screen(pte, sub_key);
+			sub_unhold(pte, sub_key);
 			pte->state = STATE_CALL;
-		}
+		} else if (sub_key->subtype == SUB_RING) {
+			sub_hold(pte, sub);
+			sub_key->softkey = keynum;
+			handle_call_incoming(pte);
+		} 
 	}
 }
 
@@ -3469,8 +3490,13 @@
 	case KEY_ONHOLD:
 		if (!sub) {
 			if(pte->device->ssub[pte->device->selected]) {
-				sub_hold(pte, pte->device->ssub[pte->device->selected]);
+				sub = pte->device->ssub[pte->device->selected];
+			} else {
+				break;
 			}
+		}
+		if (sub->holding) {
+			sub_unhold(pte, sub);
 		} else {
 			sub_hold(pte, sub);
 		}
@@ -5428,6 +5454,7 @@
 					}
 					if (sub->owner) {
 						/* Allocate additional channel if asterisk channel already here */
+						ast_log(LOG_WARNING, "Allocate ONHOLD channel!!!");
 						sub = unistim_alloc_sub(d, SUB_ONHOLD);
 					}
 					sub->ringvolume = -1;

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib1e04e482e7c8939607a42d7fddacc07e26e14d4
Gerrit-Change-Number: 7882
Gerrit-PatchSet: 1
Gerrit-Owner: Igor Goncharovsky <igor.goncharovsky at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180108/c66a85d3/attachment.html>