Merged revisions 221769 via svnmerge from

https://origsvn.digium.com/svn/asterisk/branches/1.4

........
  r221769 | rmudgett | 2009-10-01 18:18:28 -0500 (Thu, 01 Oct 2009) | 26 lines
  
  Occasionally losing use of B channels in chan_misdn.
  
  I have not been able to reproduce the problem of losing channels.
  However, I have seen in the code a reentrancy problem that might give
  these symptoms.
  
  The reentrancy patch does several things:
  1) Guards B channel and B channel structure allocation.
  2) Makes the B channel structure find routines more precise in locating records.
  3) Never leave a B channel allocated if we received cause 44.
  
  The last item may cause temporary outgoing call problems, but they should
  clear when the line becomes idle.
  
  (closes issue #15490)
  Reported by: slutec18
  Patches:
        issue15490_channel_alloc_reentrancy.patch uploaded by rmudgett (license 664)
  Tested by: rmudgett, slutec18
  
  (closes issue #15458)
  Reported by: FabienToune
  Patches:
        issue15458_channel_alloc_reentrancy.patch uploaded by rmudgett (license 664)
  Tested by: FabienToune, rmudgett, slutec18
........


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@221844 65c4cc65-6c06-0410-ace0-fbb531ad65f3

channels/misdn/isdn_lib.c

@@ -528,6 +528,7 @@ static int find_free_chan_in_stack(struct misdn_stack *stack, struct misdn_bchan
 
 #if defined(AST_MISDN_ENHANCEMENTS)
 	if (bc->is_register_pool) {
+		pthread_mutex_lock(&stack->st_lock);
 		for (i = MAX_BCHANS + 1; i < ARRAY_LEN(stack->channels); ++i) {
 			if (!stack->channels[i]) {
 				chan = i + 1;
@@ -547,6 +548,7 @@ static int find_free_chan_in_stack(struct misdn_stack *stack, struct misdn_bchan
 
 		--channel;
 
+		pthread_mutex_lock(&stack->st_lock);
 		bnums = stack->pri ? stack->b_num : stack->b_num - 1;
 		if (dec) {
 			for (i = bnums; i >= 0; --i) {
@@ -572,6 +574,7 @@ static int find_free_chan_in_stack(struct misdn_stack *stack, struct misdn_bchan
 	}
 
 	if (!chan) {
+		pthread_mutex_unlock(&stack->st_lock);
 		cb_log(1, stack->port, " !! NO FREE CHAN IN STACK\n");
 		dump_chan_list(stack);
 		bc->out_cause = AST_CAUSE_NORMAL_CIRCUIT_CONGESTION;
@@ -579,10 +582,12 @@ static int find_free_chan_in_stack(struct misdn_stack *stack, struct misdn_bchan
 	}
 
 	if (set_chan_in_stack(stack, chan) < 0) {
+		pthread_mutex_unlock(&stack->st_lock);
 		cb_log(0, stack->port, "Channel Already in use:%d\n", chan);
 		bc->out_cause = AST_CAUSE_REQUESTED_CHAN_UNAVAIL;
 		return -1;
 	}
+	pthread_mutex_unlock(&stack->st_lock);
 
 	bc->channel = chan;
 	return 0;
@@ -865,15 +870,15 @@ static void clear_l3(struct misdn_stack *stack)
 	if (global_state == MISDN_INITIALIZED) {
 		for (i = 0; i <= stack->b_num; ++i) {
 			cb_event(EVENT_CLEANUP, &stack->bc[i], NULL);
-			empty_chan_in_stack(stack, i + 1);
 			empty_bc(&stack->bc[i]);
 			clean_up_bc(&stack->bc[i]);
+			empty_chan_in_stack(stack, i + 1);
 			stack->bc[i].in_use = 0;
 		}
 #if defined(AST_MISDN_ENHANCEMENTS)
 		for (i = MAX_BCHANS + 1; i < ARRAY_LEN(stack->bc); ++i) {
-			empty_chan_in_stack(stack, i + 1);
 			empty_bc(&stack->bc[i]);
+			empty_chan_in_stack(stack, i + 1);
 			stack->bc[i].in_use = 0;
 		}
 #endif	/* defined(AST_MISDN_ENHANCEMENTS) */
@@ -1328,6 +1333,8 @@ static struct misdn_stack *stack_init(int midev, int port, int ptp)
 	msg_queue_init(&stack->downqueue);
 	msg_queue_init(&stack->upqueue);
 
+	pthread_mutex_init(&stack->st_lock, NULL);
+
 	/* query port's requirements */
 	ret = mISDN_get_stack_info(midev, port, buff, sizeof(buff));
 	if (ret < 0) {
@@ -1496,6 +1503,8 @@ static void stack_destroy(struct misdn_stack *stack)
 
 	if (stack->upper_id)
 		mISDN_write_frame(stack->midev, buf, stack->upper_id, MGR_DELLAYER | REQUEST, 0, 0, NULL, TIMEOUT_1SEC);
+
+	pthread_mutex_destroy(&stack->st_lock);
 }
 
 
@@ -1547,14 +1556,14 @@ static struct misdn_bchannel *find_bc_by_masked_l3id(struct misdn_stack *stack,
 	int i;
 
 	for (i = 0; i <= stack->b_num; ++i) {
-		if ((stack->bc[i].l3_id & mask) == (l3id & mask)) {
+		if (stack->bc[i].in_use && (stack->bc[i].l3_id & mask) == (l3id & mask)) {
 			return &stack->bc[i];
 		}
 	}
 #if defined(AST_MISDN_ENHANCEMENTS)
 	/* Search the B channel records for a REGISTER signaling link. */
 	for (i = MAX_BCHANS + 1; i < ARRAY_LEN(stack->bc); ++i) {
-		if ((stack->bc[i].l3_id & mask) == (l3id & mask)) {
+		if (stack->bc[i].in_use && (stack->bc[i].l3_id & mask) == (l3id & mask)) {
 			return &stack->bc[i];
 		}
 	}
@@ -1568,14 +1577,14 @@ struct misdn_bchannel *find_bc_by_l3id(struct misdn_stack *stack, unsigned long
 	int i;
 
 	for (i = 0; i <= stack->b_num; ++i) {
-		if (stack->bc[i].l3_id == l3id) {
+		if (stack->bc[i].in_use && stack->bc[i].l3_id == l3id) {
 			return &stack->bc[i];
 		}
 	}
 #if defined(AST_MISDN_ENHANCEMENTS)
 	/* Search the B channel records for a REGISTER signaling link. */
 	for (i = MAX_BCHANS + 1; i < ARRAY_LEN(stack->bc); ++i) {
-		if (stack->bc[i].l3_id == l3id) {
+		if (stack->bc[i].in_use && stack->bc[i].l3_id == l3id) {
 			return &stack->bc[i];
 		}
 	}
@@ -1588,11 +1597,11 @@ static struct misdn_bchannel *find_bc_by_addr(unsigned long addr)
 	struct misdn_stack *stack;
 	int i;
 
-	for (stack=glob_mgr->stack_list;
+	for (stack = glob_mgr->stack_list; stack; stack = stack->next) {
-	     stack;
+		for (i = 0; i <= stack->b_num; i++) {
-	     stack=stack->next) {
+			if (stack->bc[i].in_use
-		for (i=0; i<=stack->b_num; i++) {
+				&& ((stack->bc[i].addr & STACK_ID_MASK) == (addr & STACK_ID_MASK)
-			if ( (stack->bc[i].addr&STACK_ID_MASK)==(addr&STACK_ID_MASK) ||  stack->bc[i].layer_id== addr ) {
+					|| stack->bc[i].layer_id == addr)) {
 				return &stack->bc[i];
 			}
 		}
@@ -1606,11 +1615,9 @@ static struct misdn_bchannel *find_bc_by_confid(unsigned long confid)
 	struct misdn_stack *stack;
 	int i;
 
-	for (stack=glob_mgr->stack_list;
+	for (stack = glob_mgr->stack_list; stack; stack = stack->next) {
-	     stack;
+		for (i = 0; i <= stack->b_num; i++) {
-	     stack=stack->next) {
+			if (stack->bc[i].in_use && stack->bc[i].conf_id == confid) {
-		for (i=0; i<=stack->b_num; i++) {
-			if ( stack->bc[i].conf_id==confid ) {
 				return &stack->bc[i];
 			}
 		}
@@ -1624,10 +1631,12 @@ static struct misdn_bchannel *find_bc_by_channel(int port, int channel)
 	struct misdn_stack *stack = find_stack_by_port(port);
 	int i;
 
-	if (!stack) return NULL;
+	if (!stack) {
+		return NULL;
+	}
 
-	for (i=0; i<=stack->b_num; i++) {
+	for (i = 0; i <= stack->b_num; i++) {
-		if ( stack->bc[i].channel== channel ) {
+		if (stack->bc[i].in_use && stack->bc[i].channel == channel) {
 			return &stack->bc[i];
 		}
 	}
@@ -1814,6 +1823,7 @@ static int handle_cr ( struct misdn_stack *stack, iframe_t *frm)
 /* Empties bc if it's reserved (no SETUP out yet) */
 void misdn_lib_release(struct misdn_bchannel *bc)
 {
+	int channel;
 	struct misdn_stack *stack=get_stack_by_bc(bc);
 
 	if (!stack) {
@@ -1821,11 +1831,12 @@ void misdn_lib_release(struct misdn_bchannel *bc)
 		return;
 	}
 
-	if (bc->channel>0)
+	channel = bc->channel;
-		empty_chan_in_stack(stack,bc->channel);
-
 	empty_bc(bc);
 	clean_up_bc(bc);
+	if (channel > 0) {
+		empty_chan_in_stack(stack, channel);
+	}
 	bc->in_use=0;
 }
 
@@ -2871,20 +2882,18 @@ static int handle_frm(msg_t *msg)
 		bc->cause = tmpcause;
 		bc->out_cause = tmp_out_cause;
 		clean_up_bc(bc);
+		bc->in_use = 0;
 
 		if (tmpcause == AST_CAUSE_REQUESTED_CHAN_UNAVAIL) {
-			cb_log(0, stack->port, "**** Received CAUSE:%d, so not cleaning up channel %d\n", AST_CAUSE_REQUESTED_CHAN_UNAVAIL, channel);
+			cb_log(0, stack->port, "**** Received CAUSE:%d, restarting channel %d\n", AST_CAUSE_REQUESTED_CHAN_UNAVAIL, channel);
-			cb_log(0, stack->port, "**** This channel is now no longer available,\nplease try to restart it with 'misdn send restart <port> <channel>'\n");
-			set_chan_in_stack(stack, channel);
-			bc->channel = channel;
 			misdn_lib_send_restart(stack->port, channel);
-		} else if (channel > 0) {
+		}
+		if (channel > 0) {
 			empty_chan_in_stack(stack, channel);
 		}
-		bc->in_use = 0;
 		break;
 	case EVENT_RESTART:
-		cb_log(0, stack->port, "**** Received RESTART_ACK channel:%d\n", bc->restart_channel);
+		cb_log(0, stack->port, "**** Received RESTART channel:%d\n", bc->restart_channel);
 		empty_chan_in_stack(stack, bc->restart_channel);
 		break;
 	default:
@@ -3246,11 +3255,12 @@ struct misdn_bchannel *manager_find_bc_by_pid(int pid)
 	struct misdn_stack *stack;
 	int i;
 
-	for (stack=glob_mgr->stack_list;
+	for (stack = glob_mgr->stack_list; stack; stack = stack->next) {
-	     stack;
+		for (i = 0; i <= stack->b_num; i++) {
-	     stack=stack->next) {
+			if (stack->bc[i].in_use && stack->bc[i].pid == pid) {
-		for (i=0; i<=stack->b_num; i++)
+				return &stack->bc[i];
-			if (stack->bc[i].pid == pid) return &stack->bc[i];
+			}
+		}
 	}
 
 	return NULL;
@@ -3282,7 +3292,6 @@ static void prepare_bc(struct misdn_bchannel*bc, int channel)
 {
 	bc->channel = channel;
 	bc->channel_preselected = channel?1:0;
-	bc->in_use = 1;
 	bc->need_disconnect=1;
 	bc->need_release=1;
 	bc->need_release_complete=1;
@@ -3296,6 +3305,8 @@ static void prepare_bc(struct misdn_bchannel*bc, int channel)
 	bc->b_stid=0;
 	bc->layer_id=0;
 #endif
+
+	bc->in_use = 1;
 }
 
 struct misdn_bchannel *misdn_lib_get_free_bc(int port, int channel, int inout, int dec)
@@ -3309,8 +3320,6 @@ struct misdn_bchannel *misdn_lib_get_free_bc(int port, int channel, int inout, i
 		return NULL;
 	}
 
-	usleep(1000);
-
 	/* Find the port stack structure */
 	stack = find_stack_by_port(port);
 	if (!stack) {
@@ -3323,20 +3332,24 @@ struct misdn_bchannel *misdn_lib_get_free_bc(int port, int channel, int inout, i
 		return NULL;
 	}
 
+    pthread_mutex_lock(&stack->st_lock);
 	if (channel > 0) {
 		if (channel <= stack->b_num) {
 			for (i = 0; i < stack->b_num; i++) {
 				if (stack->bc[i].channel == channel) {
 					if (test_inuse(&stack->bc[i])) {
+						pthread_mutex_unlock(&stack->st_lock);
 						cb_log(0, port, "Requested channel:%d on port:%d is already in use\n", channel, port);
 						return NULL;
 					} else {
 						prepare_bc(&stack->bc[i], channel);
+						pthread_mutex_unlock(&stack->st_lock);
 						return &stack->bc[i];
 					}
 				}
 			}
 		} else {
+			pthread_mutex_unlock(&stack->st_lock);
 			cb_log(0, port, "Requested channel:%d is out of bounds on port:%d\n", channel, port);
 			return NULL;
 		}
@@ -3354,6 +3367,7 @@ struct misdn_bchannel *misdn_lib_get_free_bc(int port, int channel, int inout, i
 
 				prepare_bc(&stack->bc[i], channel);
 				stack->bc[i].dec = 1;
+				pthread_mutex_unlock(&stack->st_lock);
 				return &stack->bc[i];
 			}
 		}
@@ -3366,10 +3380,12 @@ struct misdn_bchannel *misdn_lib_get_free_bc(int port, int channel, int inout, i
 				}
 
 				prepare_bc(&stack->bc[i], channel);
+				pthread_mutex_unlock(&stack->st_lock);
 				return &stack->bc[i];
 			}
 		}
 	}
+	pthread_mutex_unlock(&stack->st_lock);
 
 	cb_log(1, port, "There is no free channel on port (%d)\n", port);
 	return NULL;
@@ -3402,15 +3418,18 @@ struct misdn_bchannel *misdn_lib_get_register_bc(int port)
 		return NULL;
 	}
 
+    pthread_mutex_lock(&stack->st_lock);
 	for (index = MAX_BCHANS + 1; index < ARRAY_LEN(stack->bc); ++index) {
 		bc = &stack->bc[index];
 		if (!test_inuse(bc)) {
 			prepare_bc(bc, 0);
 			bc->need_disconnect = 0;
 			bc->need_release = 0;
+			pthread_mutex_unlock(&stack->st_lock);
 			return bc;
 		}
 	}
+	pthread_mutex_unlock(&stack->st_lock);
 
 	cb_log(1, port, "There is no free REGISTER link on port (%d)\n", port);
 	return NULL;
@@ -4116,7 +4135,7 @@ int misdn_lib_send_restart(int port, int channel)
 		/* clean up chan in stack, to be sure we don't think it's
 		 * in use anymore */
 		for (cnt=0; cnt<=stack->b_num; cnt++) {
-			if (stack->bc[cnt].channel == channel) {
+			if (stack->bc[cnt].in_use && stack->bc[cnt].channel == channel) {
 				empty_bc(&stack->bc[cnt]);
 				clean_up_bc(&stack->bc[cnt]);
 				stack->bc[cnt].in_use=0;

channels/misdn/isdn_lib_intern.h

@@ -67,6 +67,9 @@ struct misdn_stack {
 	manager_t mgr;
 	pthread_mutex_t nstlock;
 
+	/*! \brief Stack struct critical section lock. */
+	pthread_mutex_t st_lock;
+
 	/*! \brief D Channel mISDN driver stack ID (Parent stack ID) */
 	int d_stid;
 
@@ -134,7 +137,7 @@ struct misdn_stack {
 	 */
 	char channels[MAX_BCHANS + 1 + MISDN_MAX_REGISTER_LINKS];
 
-	/*! \brief List of holded channels */
+	/*! \brief List of held channels */
 	struct misdn_bchannel *holding;
 
 	/*! \brief Next stack in the list of stacks */