Merged revisions 291655 via svnmerge from

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

................
  r291655 | rmudgett | 2010-10-13 18:36:50 -0500 (Wed, 13 Oct 2010) | 27 lines
  
  Merged revisions 291643 via svnmerge from 
  https://origsvn.digium.com/svn/asterisk/branches/1.4
  
  ........
    r291643 | rmudgett | 2010-10-13 18:29:58 -0500 (Wed, 13 Oct 2010) | 20 lines
    
    Deadlock between dahdi_exception() and dahdi_indicate().
    
    There is a deadlock between dahdi_exception() and dahdi_indicate() for
    analog ports.  The call-waiting and three-way-calling feature can
    experience deadlock if these features are trying to do something and an
    event from the bridged channel happens at the same time.
    
    Deadlock avoidance code added to obtain necessary channel locks before
    attemting an operation with call-waiting and three-way-calling.
    
    (closes issue #16847)
    Reported by: shin-shoryuken
    Patches:
          issue_16847_v1.4.patch uploaded by rmudgett (license 664)
          issue_16847_v1.6.2.patch uploaded by rmudgett (license 664)
          issue_16847_v1.8_v2.patch uploaded by rmudgett (license 664)
    Tested by: alecdavis, rmudgett
    
    Review: https://reviewboard.asterisk.org/r/971/
  ........
................


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

3 files changed, 274 insertions, 94 deletions

diff --git a/channels/chan_dahdi.c b/channels/chan_dahdi.c
index 4d6a3fae4..de3c0c237 100644
--- a/channels/chan_dahdi.c
+++ b/channels/chan_dahdi.c
@@ -2058,6 +2058,13 @@ static void my_unlock_private(void *pvt)
 	ast_mutex_unlock(&p->lock);
 }
 
+static void my_deadlock_avoidance_private(void *pvt)
+{
+	struct dahdi_pvt *p = pvt;
+
+	DEADLOCK_AVOIDANCE(&p->lock);
+}
+
 /* linear_mode = 0 - turn linear mode off, >0 - turn linear mode on
 * 	returns the last value of the linear setting 
 */ 
@@ -3479,6 +3486,7 @@ static struct analog_callback dahdi_analog_callbacks =
 	.all_subchannels_hungup = my_all_subchannels_hungup,
 	.lock_private = my_lock_private,
 	.unlock_private = my_unlock_private,
+	.deadlock_avoidance_private = my_deadlock_avoidance_private,
 	.handle_dtmfup = my_handle_dtmfup,
 	.wink = my_wink,
 	.new_ast_channel = my_new_analog_ast_channel,
@@ -3537,19 +3545,43 @@ static int dahdi_get_index(struct ast_channel *ast, struct dahdi_pvt *p, int nul
 	return res;
 }
 
-static void wakeup_sub(struct dahdi_pvt *p, int a)
+/*!
+ * \internal
+ * \brief Obtain the specified subchannel owner lock if the owner exists.
+ *
+ * \param pvt Channel private struct.
+ * \param sub_idx Subchannel owner to lock.
+ *
+ * \note Assumes the pvt->lock is already obtained.
+ *
+ * \note
+ * Because deadlock avoidance may have been necessary, you need to confirm
+ * the state of things before continuing.
+ *
+ * \return Nothing
+ */
+static void dahdi_lock_sub_owner(struct dahdi_pvt *pvt, int sub_idx)
 {
 	for (;;) {
-		if (p->subs[a].owner) {
-			if (ast_channel_trylock(p->subs[a].owner)) {
-				DEADLOCK_AVOIDANCE(&p->lock);
-			} else {
-				ast_queue_frame(p->subs[a].owner, &ast_null_frame);
-				ast_channel_unlock(p->subs[a].owner);
-				break;
-			}
-		} else
+		if (!pvt->subs[sub_idx].owner) {
+			/* No subchannel owner pointer */
+			break;
+		}
+		if (!ast_channel_trylock(pvt->subs[sub_idx].owner)) {
+			/* Got subchannel owner lock */
 			break;
+		}
+		/* We must unlock the private to avoid the possibility of a deadlock */
+		DEADLOCK_AVOIDANCE(&pvt->lock);
+	}
+}
+
+static void wakeup_sub(struct dahdi_pvt *p, int a)
+{
+	dahdi_lock_sub_owner(p, a);
+	if (p->subs[a].owner) {
+		ast_queue_frame(p->subs[a].owner, &ast_null_frame);
+		ast_channel_unlock(p->subs[a].owner);
 	}
 }
 
@@ -11241,9 +11273,13 @@ static void *do_monitor(void *data)
 					if (last) {
 						struct analog_pvt *analog_p = last->sig_pvt;
 						/* Only allow MWI to be initiated on a quiescent fxs port */
-						if (!last->mwisendactive &&	last->sig & __DAHDI_SIG_FXO &&
-								!analog_p->fxsoffhookstate && !last->owner &&
-								!ast_strlen_zero(last->mailbox) && (thispass - analog_p->onhooktime > 3)) {
+						if (analog_p
+							&& !last->mwisendactive
+							&& (last->sig & __DAHDI_SIG_FXO)
+							&& !analog_p->fxsoffhookstate
+							&& !last->owner
+							&& !ast_strlen_zero(last->mailbox)
+							&& (thispass - analog_p->onhooktime > 3)) {
 							res = has_voicemail(last);
 							if (analog_p->msgstate != res) {
 								/* Set driver resources for signalling VMWI */
diff --git a/channels/sig_analog.c b/channels/sig_analog.c
index 89e4b827f..005f193f3 100644
--- a/channels/sig_analog.c
+++ b/channels/sig_analog.c
@@ -508,19 +508,59 @@ static void analog_all_subchannels_hungup(struct analog_pvt *p)
 	}
 }
 
+#if 0
 static void analog_unlock_private(struct analog_pvt *p)
 {
 	if (p->calls->unlock_private) {
 		p->calls->unlock_private(p->chan_pvt);
 	}
 }
+#endif
 
+#if 0
 static void analog_lock_private(struct analog_pvt *p)
 {
 	if (p->calls->lock_private) {
 		p->calls->lock_private(p->chan_pvt);
 	}
 }
+#endif
+
+/*!
+ * \internal
+ * \brief Obtain the specified subchannel owner lock if the owner exists.
+ *
+ * \param pvt Analog private struct.
+ * \param sub_idx Subchannel owner to lock.
+ *
+ * \note Assumes the analog_lock_private(pvt->chan_pvt) is already obtained.
+ *
+ * \note
+ * Because deadlock avoidance may have been necessary, you need to confirm
+ * the state of things before continuing.
+ *
+ * \return Nothing
+ */
+static void analog_lock_sub_owner(struct analog_pvt *pvt, enum analog_sub sub_idx)
+{
+	for (;;) {
+		if (!pvt->subs[sub_idx].owner) {
+			/* No subchannel owner pointer */
+			break;
+		}
+		if (!ast_channel_trylock(pvt->subs[sub_idx].owner)) {
+			/* Got subchannel owner lock */
+			break;
+		}
+		/* We must unlock the private to avoid the possibility of a deadlock */
+		if (pvt->calls->deadlock_avoidance_private) {
+			pvt->calls->deadlock_avoidance_private(pvt->chan_pvt);
+		} else {
+			/* Don't use 100% CPU if required callback not present. */
+			usleep(1);
+		}
+	}
+}
 
 static int analog_off_hook(struct analog_pvt *p)
 {
@@ -605,6 +645,20 @@ static int analog_is_dialing(struct analog_pvt *p, enum analog_sub index)
 	return -1;
 }
 
+/*!
+ * \internal
+ * \brief Attempt to transfer 3-way call.
+ *
+ * \param p Analog private structure.
+ *
+ * \note
+ * On entry these locks are held: real-call, private, 3-way call.
+ *
+ * \retval 1 Transfer successful.  3-way call is unlocked and subchannel is unalloced.
+ *         Swapped real and 3-way subchannel.
+ * \retval 0 Transfer successful.  3-way call is unlocked and subchannel is unalloced.
+ * \retval -1 on error.  Caller must unlock 3-way call.
+ */
 static int analog_attempt_transfer(struct analog_pvt *p)
 {
 	/* In order to transfer, we need at least one of the channels to
@@ -612,11 +666,18 @@ static int analog_attempt_transfer(struct analog_pvt *p)
 	   together (but then, why would we want to?) */
 	if (ast_bridged_channel(p->subs[ANALOG_SUB_REAL].owner)) {
 		/* The three-way person we're about to transfer to could still be in MOH, so
-		   stop if now if appropriate */
+		   stop it now if appropriate */
 		if (ast_bridged_channel(p->subs[ANALOG_SUB_THREEWAY].owner)) {
 			ast_queue_control(p->subs[ANALOG_SUB_THREEWAY].owner, AST_CONTROL_UNHOLD);
 		}
 		if (p->subs[ANALOG_SUB_REAL].owner->_state == AST_STATE_RINGING) {
+			/*
+			 * This may not be safe.
+			 * We currently hold the locks on the real-call, private, and 3-way call.
+			 * We could possibly avoid this here by using an ast_queue_control() instead.
+			 * However, the following ast_channel_masquerade() is going to be locking
+			 * the bridged channel again anyway.
+			 */
 			ast_indicate(ast_bridged_channel(p->subs[ANALOG_SUB_REAL].owner), AST_CONTROL_RINGING);
 		}
 		if (p->subs[ANALOG_SUB_THREEWAY].owner->_state == AST_STATE_RING) {
@@ -636,6 +697,13 @@ static int analog_attempt_transfer(struct analog_pvt *p)
 	} else if (ast_bridged_channel(p->subs[ANALOG_SUB_THREEWAY].owner)) {
 		ast_queue_control(p->subs[ANALOG_SUB_REAL].owner, AST_CONTROL_UNHOLD);
 		if (p->subs[ANALOG_SUB_THREEWAY].owner->_state == AST_STATE_RINGING) {
+			/*
+			 * This may not be safe.
+			 * We currently hold the locks on the real-call, private, and 3-way call.
+			 * We could possibly avoid this here by using an ast_queue_control() instead.
+			 * However, the following ast_channel_masquerade() is going to be locking
+			 * the bridged channel again anyway.
+			 */
 			ast_indicate(ast_bridged_channel(p->subs[ANALOG_SUB_THREEWAY].owner), AST_CONTROL_RINGING);
 		}
 		if (p->subs[ANALOG_SUB_REAL].owner->_state == AST_STATE_RING) {
@@ -656,7 +724,6 @@ static int analog_attempt_transfer(struct analog_pvt *p)
 	} else {
 		ast_debug(1, "Neither %s nor %s are in a bridge, nothing to transfer\n",
 					p->subs[ANALOG_SUB_REAL].owner->name, p->subs[ANALOG_SUB_THREEWAY].owner->name);
-		ast_softhangup_nolock(p->subs[ANALOG_SUB_THREEWAY].owner, AST_SOFTHANGUP_DEV);
 		return -1;
 	}
 	return 0;
@@ -1196,7 +1263,8 @@ int analog_hangup(struct analog_pvt *p, struct ast_channel *ast)
 		p->subs[index].owner = NULL;
 		p->polarity = POLARITY_IDLE;
 		analog_set_linear_mode(p, index, 0);
-		if (index == ANALOG_SUB_REAL) {
+		switch (index) {
+		case ANALOG_SUB_REAL:
 			if (p->subs[ANALOG_SUB_CALLWAIT].allocd && p->subs[ANALOG_SUB_THREEWAY].allocd) {
 				ast_debug(1, "Normal call hung up with both three way call and a call waiting call in place?\n");
 				if (p->subs[ANALOG_SUB_CALLWAIT].inthreeway) {
@@ -1215,15 +1283,23 @@ int analog_hangup(struct analog_pvt *p, struct ast_channel *ast)
 						/* This was part of a three way call.  Immediately make way for
 						   another call */
 						ast_debug(1, "Call was complete, setting owner to former third call\n");
+						p->subs[ANALOG_SUB_REAL].inthreeway = 0;
 						p->owner = p->subs[ANALOG_SUB_REAL].owner;
 					} else {
 						/* This call hasn't been completed yet...  Set owner to NULL */
 						ast_debug(1, "Call was incomplete, setting owner to NULL\n");
 						p->owner = NULL;
 					}
-					p->subs[ANALOG_SUB_REAL].inthreeway = 0;
 				}
 			} else if (p->subs[ANALOG_SUB_CALLWAIT].allocd) {
+				/* Need to hold the lock for real-call, private, and call-waiting call */
+				analog_lock_sub_owner(p, ANALOG_SUB_CALLWAIT);
+				if (!p->subs[ANALOG_SUB_CALLWAIT].owner) {
+					/* The call waiting call dissappeared. */
+					p->owner = NULL;
+					break;
+				}
+
 				/* Move to the call-wait and switch back to them. */
 				analog_swap_subs(p, ANALOG_SUB_CALLWAIT, ANALOG_SUB_REAL);
 				analog_unalloc_sub(p, ANALOG_SUB_CALLWAIT);
@@ -1234,6 +1310,8 @@ int analog_hangup(struct analog_pvt *p, struct ast_channel *ast)
 				if (ast_bridged_channel(p->subs[ANALOG_SUB_REAL].owner)) {
 					ast_queue_control(p->subs[ANALOG_SUB_REAL].owner, AST_CONTROL_UNHOLD);
 				}
+				/* Unlock the call-waiting call that we swapped to real-call. */
+				ast_channel_unlock(p->subs[ANALOG_SUB_REAL].owner);
 			} else if (p->subs[ANALOG_SUB_THREEWAY].allocd) {
 				analog_swap_subs(p, ANALOG_SUB_THREEWAY, ANALOG_SUB_REAL);
 				analog_unalloc_sub(p, ANALOG_SUB_THREEWAY);
@@ -1241,17 +1319,21 @@ int analog_hangup(struct analog_pvt *p, struct ast_channel *ast)
 					/* This was part of a three way call.  Immediately make way for
 					   another call */
 					ast_debug(1, "Call was complete, setting owner to former third call\n");
+					p->subs[ANALOG_SUB_REAL].inthreeway = 0;
 					p->owner = p->subs[ANALOG_SUB_REAL].owner;
 				} else {
 					/* This call hasn't been completed yet...  Set owner to NULL */
 					ast_debug(1, "Call was incomplete, setting owner to NULL\n");
 					p->owner = NULL;
 				}
-				p->subs[ANALOG_SUB_REAL].inthreeway = 0;
 			}
-		} else if (index == ANALOG_SUB_CALLWAIT) {
-			/* Ditch the holding callwait call, and immediately make it availabe */
+			break;
+		case ANALOG_SUB_CALLWAIT:
+			/* Ditch the holding callwait call, and immediately make it available */
 			if (p->subs[ANALOG_SUB_CALLWAIT].inthreeway) {
+				/* Need to hold the lock for call-waiting call, private, and 3-way call */
+				analog_lock_sub_owner(p, ANALOG_SUB_THREEWAY);
+
 				/* This is actually part of a three way, placed on hold.  Place the third part
 				   on music on hold now */
 				if (p->subs[ANALOG_SUB_THREEWAY].owner && ast_bridged_channel(p->subs[ANALOG_SUB_THREEWAY].owner)) {
@@ -1263,27 +1345,42 @@ int analog_hangup(struct analog_pvt *p, struct ast_channel *ast)
 				/* Make it the call wait now */
 				analog_swap_subs(p, ANALOG_SUB_CALLWAIT, ANALOG_SUB_THREEWAY);
 				analog_unalloc_sub(p, ANALOG_SUB_THREEWAY);
+				if (p->subs[ANALOG_SUB_CALLWAIT].owner) {
+					/* Unlock the 3-way call that we swapped to call-waiting call. */
+					ast_channel_unlock(p->subs[ANALOG_SUB_CALLWAIT].owner);
+				}
 			} else {
 				analog_unalloc_sub(p, ANALOG_SUB_CALLWAIT);
 			}
-		} else if (index == ANALOG_SUB_THREEWAY) {
+			break;
+		case ANALOG_SUB_THREEWAY:
+			/* Need to hold the lock for 3-way call, private, and call-waiting call */
+			analog_lock_sub_owner(p, ANALOG_SUB_CALLWAIT);
 			if (p->subs[ANALOG_SUB_CALLWAIT].inthreeway) {
 				/* The other party of the three way call is currently in a call-wait state.
 				   Start music on hold for them, and take the main guy out of the third call */
+				p->subs[ANALOG_SUB_CALLWAIT].inthreeway = 0;
 				if (p->subs[ANALOG_SUB_CALLWAIT].owner && ast_bridged_channel(p->subs[ANALOG_SUB_CALLWAIT].owner)) {
 					ast_queue_control_data(p->subs[ANALOG_SUB_CALLWAIT].owner, AST_CONTROL_HOLD,
 						S_OR(p->mohsuggest, NULL),
 						!ast_strlen_zero(p->mohsuggest) ? strlen(p->mohsuggest) + 1 : 0);
 				}
-				p->subs[ANALOG_SUB_CALLWAIT].inthreeway = 0;
+			}
+			if (p->subs[ANALOG_SUB_CALLWAIT].owner) {
+				ast_channel_unlock(p->subs[ANALOG_SUB_CALLWAIT].owner);
 			}
 			p->subs[ANALOG_SUB_REAL].inthreeway = 0;
 			/* If this was part of a three way call index, let us make
 			   another three way call */
 			analog_unalloc_sub(p, ANALOG_SUB_THREEWAY);
-		} else {
-			/* This wasn't any sort of call, but how are we an index? */
-			ast_log(LOG_WARNING, "Index found but not any type of call?\n");
+			break;
+		default:
+			/*
+			 * Should never happen.
+			 * This wasn't any sort of call, so how are we an index?
+			 */
+			ast_log(LOG_ERROR, "Index found but not any type of call?\n");
+			break;
 		}
 	}
 
@@ -1599,9 +1696,8 @@ static void *__analog_ss_thread(void *data)
 	}
 
 	ast_verb(3, "Starting simple switch on '%s'\n", chan->name);
-	index = analog_get_index(chan, p, 1);
+	index = analog_get_index(chan, p, 0);
 	if (index < 0) {
-		ast_log(LOG_WARNING, "Huh?\n");
 		ast_hangup(chan);
 		goto quit;
 	}
@@ -2453,10 +2549,18 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 	ast_log(LOG_DEBUG, "%s %d\n", __FUNCTION__, p->channel);
 
 	index = analog_get_index(ast, p, 0);
+	if (index < 0) {
+		return &ast_null_frame;
+	}
+	if (index != ANALOG_SUB_REAL) {
+		ast_log(LOG_ERROR, "We got an event on a non real sub.  Fix it!\n");
+	}
+
 	mysig = p->sig;
 	if (p->outsigmod > -1) {
 		mysig = p->outsigmod;
 	}
+
 	p->subs[index].f.frametype = AST_FRAME_NULL;
 	p->subs[index].f.subclass.integer = 0;
 	p->subs[index].f.datalen = 0;
@@ -2467,14 +2571,6 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 	p->subs[index].f.data.ptr = NULL;
 	f = &p->subs[index].f;
 
-	if (index < 0) {
-		return &p->subs[index].f;
-	}
-
-	if (index != ANALOG_SUB_REAL) {
-		ast_log(LOG_ERROR, "We got an event on a non real sub.  Fix it!\n");
-	}
-
 	res = analog_get_event(p);
 
 	ast_debug(1, "Got event %s(%d) on channel %d (index %d)\n", analog_event2str(res), res, p->channel, index);
@@ -2583,6 +2679,17 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 			if (index == ANALOG_SUB_REAL) {
 				/* The normal line was hung up */
 				if (p->subs[ANALOG_SUB_CALLWAIT].owner) {
+					/* Need to hold the lock for real-call, private, and call-waiting call */
+					analog_lock_sub_owner(p, ANALOG_SUB_CALLWAIT);
+					if (!p->subs[ANALOG_SUB_CALLWAIT].owner) {
+						/*
+						 * The call waiting call dissappeared.
+						 * This is now a normal hangup.
+						 */
+						analog_set_echocanceller(p, 0);
+						return NULL;
+					}
+
 					/* There's a call waiting call, so ring the phone, but make it unowned in the mean time */
 					analog_swap_subs(p, ANALOG_SUB_CALLWAIT, ANALOG_SUB_REAL);
 					ast_verb(3, "Channel %d still has (callwait) call, ringing phone\n", p->channel);
@@ -2593,64 +2700,58 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 					if (p->subs[ANALOG_SUB_REAL].owner->_state != AST_STATE_UP) {
 						analog_set_dialing(p, 1);
 					}
+					/* Unlock the call-waiting call that we swapped to real-call. */
+					ast_channel_unlock(p->subs[ANALOG_SUB_REAL].owner);
 					analog_ring(p);
 				} else if (p->subs[ANALOG_SUB_THREEWAY].owner) {
 					unsigned int mssinceflash;
-					/* Here we have to retain the lock on both the main channel, the 3-way channel, and
-					   the private structure -- not especially easy or clean */
-					while (p->subs[ANALOG_SUB_THREEWAY].owner && ast_channel_trylock(p->subs[ANALOG_SUB_THREEWAY].owner)) {
-						/* Yuck, didn't get the lock on the 3-way, gotta release everything and re-grab! */
-						analog_unlock_private(p);
-						CHANNEL_DEADLOCK_AVOIDANCE(ast);
-						/* We can grab ast and p in that order, without worry.  We should make sure
-						   nothing seriously bad has happened though like some sort of bizarre double
-						   masquerade! */
-						analog_lock_private(p);
-						if (p->owner != ast) {
-							ast_log(LOG_WARNING, "This isn't good...\n");
-							return NULL;
-						}
-					}
+
+					/* Need to hold the lock for real-call, private, and 3-way call */
+					analog_lock_sub_owner(p, ANALOG_SUB_THREEWAY);
 					if (!p->subs[ANALOG_SUB_THREEWAY].owner) {
 						ast_log(LOG_NOTICE, "Whoa, threeway disappeared kinda randomly.\n");
+						/* Just hangup */
 						return NULL;
 					}
+					if (p->owner != ast) {
+						ast_channel_unlock(p->subs[ANALOG_SUB_THREEWAY].owner);
+						ast_log(LOG_WARNING, "This isn't good...\n");
+						/* Just hangup */
+						return NULL;
+					}
+
 					mssinceflash = ast_tvdiff_ms(ast_tvnow(), p->flashtime);
 					ast_debug(1, "Last flash was %d ms ago\n", mssinceflash);
 					if (mssinceflash < MIN_MS_SINCE_FLASH) {
 						/* It hasn't been long enough since the last flashook.  This is probably a bounce on
 						   hanging up.  Hangup both channels now */
-						if (p->subs[ANALOG_SUB_THREEWAY].owner) {
-							ast_queue_hangup_with_cause(p->subs[ANALOG_SUB_THREEWAY].owner, AST_CAUSE_NO_ANSWER);
-						}
-						ast_softhangup_nolock(p->subs[ANALOG_SUB_THREEWAY].owner, AST_SOFTHANGUP_DEV);
 						ast_debug(1, "Looks like a bounced flash, hanging up both calls on %d\n", p->channel);
+						ast_queue_hangup_with_cause(p->subs[ANALOG_SUB_THREEWAY].owner, AST_CAUSE_NO_ANSWER);
+						ast_softhangup_nolock(p->subs[ANALOG_SUB_THREEWAY].owner, AST_SOFTHANGUP_DEV);
 						ast_channel_unlock(p->subs[ANALOG_SUB_THREEWAY].owner);
 					} else if ((ast->pbx) || (ast->_state == AST_STATE_UP)) {
 						if (p->transfer) {
 							/* Only attempt transfer if the phone is ringing; why transfer to busy tone eh? */
 							if (!p->transfertobusy && ast->_state == AST_STATE_BUSY) {
-								ast_channel_unlock(p->subs[ANALOG_SUB_THREEWAY].owner);
 								/* Swap subs and dis-own channel */
 								analog_swap_subs(p, ANALOG_SUB_THREEWAY, ANALOG_SUB_REAL);
+								/* Unlock the 3-way call that we swapped to real-call. */
+								ast_channel_unlock(p->subs[ANALOG_SUB_REAL].owner);
 								p->owner = NULL;
 								/* Ring the phone */
 								analog_ring(p);
 							} else {
-								if ((res = analog_attempt_transfer(p)) < 0) {
+								res = analog_attempt_transfer(p);
+								if (res < 0) {
+									/* Transfer attempt failed. */
 									ast_softhangup_nolock(p->subs[ANALOG_SUB_THREEWAY].owner, AST_SOFTHANGUP_DEV);
-									if (p->subs[ANALOG_SUB_THREEWAY].owner) {
-										ast_channel_unlock(p->subs[ANALOG_SUB_THREEWAY].owner);
-									}
+									ast_channel_unlock(p->subs[ANALOG_SUB_THREEWAY].owner);
 								} else if (res) {
 									/* this isn't a threeway call anymore */
 									p->subs[ANALOG_SUB_REAL].inthreeway = 0;
 									p->subs[ANALOG_SUB_THREEWAY].inthreeway = 0;
 
 									/* Don't actually hang up at this point */
-									if (p->subs[ANALOG_SUB_THREEWAY].owner) {
-										ast_channel_unlock(&p->subs[ANALOG_SUB_THREEWAY].owner);
-									}
 									break;
 								}
 							}
@@ -2659,15 +2760,14 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 							p->subs[ANALOG_SUB_THREEWAY].inthreeway = 0;
 						} else {
 							ast_softhangup_nolock(p->subs[ANALOG_SUB_THREEWAY].owner, AST_SOFTHANGUP_DEV);
-							if (p->subs[ANALOG_SUB_THREEWAY].owner) {
-								ast_channel_unlock(p->subs[ANALOG_SUB_THREEWAY].owner);
-							}
+							ast_channel_unlock(p->subs[ANALOG_SUB_THREEWAY].owner);
 						}
 					} else {
 						ast_cel_report_event(ast, AST_CEL_BLINDTRANSFER, NULL, ast->linkedid, NULL);
-						ast_channel_unlock(p->subs[ANALOG_SUB_THREEWAY].owner);
 						/* Swap subs and dis-own channel */
 						analog_swap_subs(p, ANALOG_SUB_THREEWAY, ANALOG_SUB_REAL);
+						/* Unlock the 3-way call that we swapped to real-call. */
+						ast_channel_unlock(p->subs[ANALOG_SUB_REAL].owner);
 						p->owner = NULL;
 						/* Ring the phone */
 						analog_ring(p);
@@ -2905,32 +3005,43 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 			}
 
 			if (p->subs[ANALOG_SUB_CALLWAIT].owner) {
-				/* Swap to call-wait */
-				int previous_state = p->subs[ANALOG_SUB_CALLWAIT].owner->_state;
-				if (p->subs[ANALOG_SUB_CALLWAIT].owner->_state == AST_STATE_RINGING) {
-					ast_setstate(p->subs[ANALOG_SUB_CALLWAIT].owner, AST_STATE_UP);
+				/* Need to hold the lock for real-call, private, and call-waiting call */
+				analog_lock_sub_owner(p, ANALOG_SUB_CALLWAIT);
+				if (!p->subs[ANALOG_SUB_CALLWAIT].owner) {
+					/*
+					 * The call waiting call dissappeared.
+					 * Let's just ignore this flash-hook.
+					 */
+					ast_log(LOG_NOTICE, "Whoa, the call-waiting call disappeared.\n");
+					goto winkflashdone;
 				}
+
+				/* Swap to call-wait */
 				analog_swap_subs(p, ANALOG_SUB_REAL, ANALOG_SUB_CALLWAIT);
 				analog_play_tone(p, ANALOG_SUB_REAL, -1);
 				p->owner = p->subs[ANALOG_SUB_REAL].owner;
 				ast_debug(1, "Making %s the new owner\n", p->owner->name);
-				if (previous_state == AST_STATE_RINGING) {
+				if (p->subs[ANALOG_SUB_REAL].owner->_state == AST_STATE_RINGING) {
+					ast_setstate(p->subs[ANALOG_SUB_REAL].owner, AST_STATE_UP);
 					ast_queue_control(p->subs[ANALOG_SUB_REAL].owner, AST_CONTROL_ANSWER);
 				}
 				analog_stop_callwait(p);
+
 				/* Start music on hold if appropriate */
 				if (!p->subs[ANALOG_SUB_CALLWAIT].inthreeway && ast_bridged_channel(p->subs[ANALOG_SUB_CALLWAIT].owner)) {
 					ast_queue_control_data(p->subs[ANALOG_SUB_CALLWAIT].owner, AST_CONTROL_HOLD,
 						S_OR(p->mohsuggest, NULL),
 						!ast_strlen_zero(p->mohsuggest) ? strlen(p->mohsuggest) + 1 : 0);
 				}
-				ast_queue_control(p->subs[ANALOG_SUB_REAL].owner, AST_CONTROL_ANSWER);
 				if (ast_bridged_channel(p->subs[ANALOG_SUB_REAL].owner)) {
 					ast_queue_control_data(p->subs[ANALOG_SUB_REAL].owner, AST_CONTROL_HOLD,
 						S_OR(p->mohsuggest, NULL),
 						!ast_strlen_zero(p->mohsuggest) ? strlen(p->mohsuggest) + 1 : 0);
 				}
 				ast_queue_control(p->subs[ANALOG_SUB_REAL].owner, AST_CONTROL_UNHOLD);
+
+				/* Unlock the call-waiting call that we swapped to real-call. */
+				ast_channel_unlock(p->subs[ANALOG_SUB_REAL].owner);
 			} else if (!p->subs[ANALOG_SUB_THREEWAY].owner) {
 				if (!p->threewaycalling) {
 					/* Just send a flash if no 3-way calling */
@@ -2957,10 +3068,10 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 					/* XXX This section needs much more error checking!!! XXX */
 					/* Start a 3-way call if feasible */
 					if (!((ast->pbx) ||
-					      (ast->_state == AST_STATE_UP) ||
-					      (ast->_state == AST_STATE_RING))) {
+						(ast->_state == AST_STATE_UP) ||
+						(ast->_state == AST_STATE_RING))) {
 						ast_debug(1, "Flash when call not up or ringing\n");
-							goto winkflashdone;
+						goto winkflashdone;
 					}
 					if (analog_alloc_sub(p, ANALOG_SUB_THREEWAY)) {
 						ast_log(LOG_WARNING, "Unable to allocate three-way subchannel\n");
@@ -2968,6 +3079,13 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 					}
 					/* Make new channel */
 					chan = analog_new_ast_channel(p, AST_STATE_RESERVED, 0, ANALOG_SUB_THREEWAY, NULL);
+					if (!chan) {
+						ast_log(LOG_WARNING,
+							"Cannot allocate new call structure on channel %d\n",
+							p->channel);
+						analog_unalloc_sub(p, ANALOG_SUB_THREEWAY);
+						goto winkflashdone;
+					}
 					if (p->dahditrcallerid) {
 						if (!p->origcid_num) {
 							p->origcid_num = ast_strdup(p->cid_num);
@@ -2989,9 +3107,7 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 					p->ss_astchan = p->owner = chan;
 					pthread_attr_init(&attr);
 					pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
-					if (!chan) {
-						ast_log(LOG_WARNING, "Cannot allocate new structure on channel %d\n", p->channel);
-					} else if (ast_pthread_create(&threadid, &attr, __analog_ss_thread, p)) {
+					if (ast_pthread_create(&threadid, &attr, __analog_ss_thread, p)) {
 						ast_log(LOG_WARNING, "Unable to start simple switch on channel %d\n", p->channel);
 						res = analog_play_tone(p, ANALOG_SUB_REAL, ANALOG_TONE_CONGESTION);
 						analog_set_echocanceller(p, 1);
@@ -3022,6 +3138,20 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 				}
 			} else {
 				/* Already have a 3 way call */
+				enum analog_sub orig_3way_sub;
+
+				/* Need to hold the lock for real-call, private, and 3-way call */
+				analog_lock_sub_owner(p, ANALOG_SUB_THREEWAY);
+				if (!p->subs[ANALOG_SUB_THREEWAY].owner) {
+					/*
+					 * The 3-way call dissappeared.
+					 * Let's just ignore this flash-hook.
+					 */
+					ast_log(LOG_NOTICE, "Whoa, the 3-way call disappeared.\n");
+					goto winkflashdone;
+				}
+				orig_3way_sub = ANALOG_SUB_THREEWAY;
+
 				if (p->subs[ANALOG_SUB_THREEWAY].inthreeway) {
 					/* Call is already up, drop the last person */
 					ast_debug(1, "Got flash with three way call up, dropping last call on %d\n", p->channel);
@@ -3030,6 +3160,7 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 						(p->subs[ANALOG_SUB_THREEWAY].owner->_state == AST_STATE_UP)) {
 						/* Swap back -- we're dropping the real 3-way that isn't finished yet*/
 						analog_swap_subs(p, ANALOG_SUB_THREEWAY, ANALOG_SUB_REAL);
+						orig_3way_sub = ANALOG_SUB_REAL;
 						p->owner = p->subs[ANALOG_SUB_REAL].owner;
 					}
 					/* Drop the last call and stop the conference */
@@ -3041,7 +3172,6 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 					/* Lets see what we're up to */
 					if (((ast->pbx) || (ast->_state == AST_STATE_UP)) &&
 						(p->transfertobusy || (ast->_state != AST_STATE_BUSY))) {
-						int otherindex = ANALOG_SUB_THREEWAY;
 						struct ast_channel *other = ast_bridged_channel(p->subs[ANALOG_SUB_THREEWAY].owner);
 						int way3bridge = 0, cdr3way = 0;
 
@@ -3061,10 +3191,10 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 						p->subs[ANALOG_SUB_REAL].inthreeway = 1;
 						if (ast->_state == AST_STATE_UP) {
 							analog_swap_subs(p, ANALOG_SUB_THREEWAY, ANALOG_SUB_REAL);
-							otherindex = ANALOG_SUB_REAL;
+							orig_3way_sub = ANALOG_SUB_REAL;
 						}
-						if (p->subs[otherindex].owner && ast_bridged_channel(p->subs[otherindex].owner)) {
-							ast_queue_control(p->subs[otherindex].owner, AST_CONTROL_UNHOLD);
+						if (ast_bridged_channel(p->subs[orig_3way_sub].owner)) {
+							ast_queue_control(p->subs[orig_3way_sub].owner, AST_CONTROL_UNHOLD);
 						}
 						p->owner = p->subs[ANALOG_SUB_REAL].owner;
 						if (ast->_state == AST_STATE_RINGING) {
@@ -3073,18 +3203,20 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 							analog_play_tone(p, ANALOG_SUB_THREEWAY, ANALOG_TONE_RINGTONE);
 						}
 					} else {
-						ast_verb(3, "Dumping incomplete call on on %s\n", p->subs[ANALOG_SUB_THREEWAY].owner->name);
+						ast_verb(3, "Dumping incomplete call on %s\n", p->subs[ANALOG_SUB_THREEWAY].owner->name);
 						analog_swap_subs(p, ANALOG_SUB_THREEWAY, ANALOG_SUB_REAL);
+						orig_3way_sub = ANALOG_SUB_REAL;
 						ast_softhangup_nolock(p->subs[ANALOG_SUB_THREEWAY].owner, AST_SOFTHANGUP_DEV);
 						p->owner = p->subs[ANALOG_SUB_REAL].owner;
-						if (p->subs[ANALOG_SUB_REAL].owner && ast_bridged_channel(p->subs[ANALOG_SUB_REAL].owner)) {
+						if (ast_bridged_channel(p->subs[ANALOG_SUB_REAL].owner)) {
 							ast_queue_control(p->subs[ANALOG_SUB_REAL].owner, AST_CONTROL_UNHOLD);
 						}
 						analog_set_echocanceller(p, 1);
 					}
 				}
+				ast_channel_unlock(p->subs[orig_3way_sub].owner);
 			}
-		winkflashdone:
+winkflashdone:
 			analog_update_conf(p);
 			break;
 		case ANALOG_SIG_EM:
@@ -3275,13 +3407,15 @@ static struct ast_frame *__analog_handle_event(struct analog_pvt *p, struct ast_
 struct ast_frame *analog_exception(struct analog_pvt *p, struct ast_channel *ast)
 {
 	int res;
-	int usedindex=-1;
 	int index;
 	struct ast_frame *f;
 
 	ast_log(LOG_DEBUG, "%s %d\n", __FUNCTION__, p->channel);
 
 	index = analog_get_index(ast, p, 1);
+	if (index < 0) {
+		index = ANALOG_SUB_REAL;
+	}
 
 	p->subs[index].f.frametype = AST_FRAME_NULL;
 	p->subs[index].f.datalen = 0;
@@ -3293,7 +3427,6 @@ struct ast_frame *analog_exception(struct analog_pvt *p, struct ast_channel *ast
 	p->subs[index].f.src = "dahdi_exception";
 	p->subs[index].f.data.ptr = NULL;
 
-
 	if (!p->owner) {
 		/* If nobody owns us, absorb the event appropriately, otherwise
 		   we loop indefinitely.  This occurs when, during call waiting, the
@@ -3307,6 +3440,14 @@ struct ast_frame *analog_exception(struct analog_pvt *p, struct ast_channel *ast
 			(res != ANALOG_EVENT_HOOKCOMPLETE)) {
 			ast_debug(1, "Restoring owner of channel %d on event %d\n", p->channel, res);
 			p->owner = p->subs[ANALOG_SUB_REAL].owner;
+			if (p->owner && ast != p->owner) {
+				/*
+				 * Could this even happen?
+				 * Possible deadlock because we do not have the real-call lock.
+				 */
+				ast_log(LOG_WARNING, "Event %s on %s is not restored owner %s\n",
+					analog_event2str(res), ast->name, p->owner->name);
+			}
 			if (p->owner && ast_bridged_channel(p->owner)) {
 				ast_queue_control(p->owner, AST_CONTROL_UNHOLD);
 			}
@@ -3319,7 +3460,8 @@ struct ast_frame *analog_exception(struct analog_pvt *p, struct ast_channel *ast
 				analog_ring(p);
 				analog_stop_callwait(p);
 			} else {
-				ast_log(LOG_WARNING, "Absorbed on hook, but nobody is left!?!?\n");
+				ast_log(LOG_WARNING, "Absorbed %s, but nobody is left!?!?\n",
+					analog_event2str(res));
 			}
 			analog_update_conf(p);
 			break;
@@ -3327,7 +3469,7 @@ struct ast_frame *analog_exception(struct analog_pvt *p, struct ast_channel *ast
 			analog_set_echocanceller(p, 1);
 			analog_off_hook(p);
 			if (p->owner && (p->owner->_state == AST_STATE_RINGING)) {
-				ast_queue_control(p->subs[ANALOG_SUB_REAL].owner, AST_CONTROL_ANSWER);
+				ast_queue_control(p->owner, AST_CONTROL_ANSWER);
 				analog_set_dialing(p, 0);
 			}
 			break;
@@ -3342,10 +3484,7 @@ struct ast_frame *analog_exception(struct analog_pvt *p, struct ast_channel *ast
 				ast_verb(3, "Channel %d flashed to other channel %s\n", p->channel, p->owner->name);
 				if (p->owner->_state != AST_STATE_UP) {
 					/* Answer if necessary */
-					usedindex = analog_get_index(p->owner, p, 0);
-					if (usedindex > -1) {
-						ast_queue_control(p->subs[usedindex].owner, AST_CONTROL_ANSWER);
-					}
+					ast_queue_control(p->owner, AST_CONTROL_ANSWER);
 					ast_setstate(p->owner, AST_STATE_UP);
 				}
 				analog_stop_callwait(p);
@@ -3353,12 +3492,14 @@ struct ast_frame *analog_exception(struct analog_pvt *p, struct ast_channel *ast
 					ast_queue_control(p->owner, AST_CONTROL_UNHOLD);
 				}
 			} else {
-				ast_log(LOG_WARNING, "Absorbed on hook, but nobody is left!?!?\n");
+				ast_log(LOG_WARNING, "Absorbed %s, but nobody is left!?!?\n",
+					analog_event2str(res));
 			}
 			analog_update_conf(p);
 			break;
 		default:
 			ast_log(LOG_WARNING, "Don't know how to absorb event %s\n", analog_event2str(res));
+			break;
 		}
 		f = &p->subs[index].f;
 		return f;
diff --git a/channels/sig_analog.h b/channels/sig_analog.h
index d88e622e7..c1b611527 100644
--- a/channels/sig_analog.h
+++ b/channels/sig_analog.h
@@ -129,6 +129,9 @@ struct analog_callback {
 	void (* const unlock_private)(void *pvt);
 	/* Lock the private in the signalling private structure.  ... */
 	void (* const lock_private)(void *pvt);
+	/* Do deadlock avoidance for the private signaling structure lock.  */
+	void (* const deadlock_avoidance_private)(void *pvt);
+
 	/* Function which is called back to handle any other DTMF up events that are received.  Called by analog_handle_event.  Why is this
 	 * important to use, instead of just directly using events received before they are passed into the library?  Because sometimes,
 	 * (CWCID) the library absorbs DTMF events received. */