1 files changed, 174 insertions, 29 deletions

diff --git a/apps/app_chanspy.c b/apps/app_chanspy.c
index 77a4ffe85..21e466fb3 100644
--- a/apps/app_chanspy.c
+++ b/apps/app_chanspy.c
@@ -222,23 +222,31 @@ static struct ast_generator spygen = {
 	.generate = spy_generate,
 };
 
-static int start_spying(struct ast_channel *chan, struct ast_channel *spychan, struct ast_audiohook *audiohook)
+static int start_spying(struct ast_channel *chan, const char *spychan_name, struct ast_audiohook *audiohook) 
 {
 	int res = 0;
 	struct ast_channel *peer = NULL;
 
-	ast_log(LOG_NOTICE, "Attaching %s to %s\n", spychan->name, chan->name);
+	ast_log(LOG_NOTICE, "Attaching %s to %s\n", spychan_name, chan->name);
 
 	res = ast_audiohook_attach(chan, audiohook);
 
-	if (!res && ast_test_flag(chan, AST_FLAG_NBRIDGE) && (peer = ast_bridged_channel(chan)))
+	if (!res && ast_test_flag(chan, AST_FLAG_NBRIDGE) && (peer = ast_bridged_channel(chan))) {
+		ast_channel_unlock(chan);
 		ast_softhangup(peer, AST_SOFTHANGUP_UNBRIDGE);
+	} else
+		ast_channel_unlock(chan);
 
 	return res;
 }
 
-static int channel_spy(struct ast_channel *chan, struct ast_channel *spyee, int *volfactor, int fd,
-	const struct ast_flags *flags, char *exitcontext)
+struct chanspy_ds {
+	struct ast_channel *chan;
+	ast_mutex_t lock;
+};
+
+static int channel_spy(struct ast_channel *chan, struct chanspy_ds *spyee_chanspy_ds, 
+	int *volfactor, int fd, const struct ast_flags *flags, char *exitcontext) 
 {
 	struct chanspy_translation_helper csth;
 	int running = 0, res, x = 0;
@@ -246,6 +254,24 @@ static int channel_spy(struct ast_channel *chan, struct ast_channel *spyee, int
 	char *name;
 	struct ast_frame *f;
 	struct ast_silence_generator *silgen = NULL;
+	struct ast_channel *spyee = NULL;
+	const char *spyer_name;
+
+	ast_channel_lock(chan);
+	spyer_name = ast_strdupa(chan->name);
+	ast_channel_unlock(chan);
+
+	ast_mutex_lock(&spyee_chanspy_ds->lock);
+	if (spyee_chanspy_ds->chan) {
+		spyee = spyee_chanspy_ds->chan;
+		ast_channel_lock(spyee);
+	}
+	ast_mutex_unlock(&spyee_chanspy_ds->lock);
+
+	if (!spyee)
+		return 0;
+
+	/* We now hold the channel lock on spyee */
 
 	if (ast_check_hangup(chan) || ast_check_hangup(spyee))
 		return 0;
@@ -257,16 +283,18 @@ static int channel_spy(struct ast_channel *chan, struct ast_channel *spyee, int
 
 	ast_audiohook_init(&csth.spy_audiohook, AST_AUDIOHOOK_TYPE_SPY, "ChanSpy");
 
-	if (start_spying(spyee, chan, &csth.spy_audiohook)) {
+	if (start_spying(spyee, spyer_name, &csth.spy_audiohook)) { /* Unlocks spyee */
 		ast_audiohook_destroy(&csth.spy_audiohook);
 		return 0;
 	}
 
 	if (ast_test_flag(flags, OPTION_WHISPER)) {
 		ast_audiohook_init(&csth.whisper_audiohook, AST_AUDIOHOOK_TYPE_WHISPER, "ChanSpy");
-		start_spying(spyee, chan, &csth.whisper_audiohook);
+		start_spying(spyee, spyer_name, &csth.whisper_audiohook); /* Unlocks spyee */
 	}
 
+	spyee = NULL;
+
 	csth.volfactor = *volfactor;
 
 	if (csth.volfactor) {
@@ -380,12 +408,82 @@ static int channel_spy(struct ast_channel *chan, struct ast_channel *spyee, int
 	return running;
 }
 
-static struct ast_channel *next_channel(const struct ast_channel *last, const char *spec,
-	const char *exten, const char *context)
+/*!
+ * \note This relies on the embedded lock to be recursive, as it may be called
+ * due to a call to chanspy_ds_free with the lock held there.
+ */
+static void chanspy_ds_destroy(void *data)
+{
+	struct chanspy_ds *chanspy_ds = data;
+
+	/* Setting chan to be NULL is an atomic operation, but we don't want this
+	 * value to change while this lock is held.  The lock is held elsewhere
+	 * while it performs non-atomic operations with this channel pointer */
+
+	ast_mutex_lock(&chanspy_ds->lock);
+	chanspy_ds->chan = NULL;
+	ast_mutex_unlock(&chanspy_ds->lock);
+}
+
+static const struct ast_datastore_info chanspy_ds_info = {
+	.type = "chanspy",
+	.destroy = chanspy_ds_destroy,
+};
+
+static struct chanspy_ds *chanspy_ds_free(struct chanspy_ds *chanspy_ds)
+{
+	if (!chanspy_ds)
+		return NULL;
+
+	ast_mutex_lock(&chanspy_ds->lock);
+	if (chanspy_ds->chan) {
+		struct ast_datastore *datastore;
+		struct ast_channel *chan;
+
+		chan = chanspy_ds->chan;
+
+		ast_channel_lock(chan);
+		if ((datastore = ast_channel_datastore_find(chan, &chanspy_ds_info, NULL))) {
+			ast_channel_datastore_remove(chan, datastore);
+			/* chanspy_ds->chan is NULL after this call */
+			ast_channel_datastore_free(datastore);
+		}
+		ast_channel_unlock(chan);
+	}
+	ast_mutex_unlock(&chanspy_ds->lock);
+
+	return NULL;
+}
+
+/*! \note Returns the channel in the chanspy_ds locked as well as the chanspy_ds locked */
+static struct chanspy_ds *setup_chanspy_ds(struct ast_channel *chan, struct chanspy_ds *chanspy_ds)
+{
+	struct ast_datastore *datastore = NULL;
+
+	ast_mutex_lock(&chanspy_ds->lock);
+
+	chanspy_ds->chan = chan;
+
+	if (!(datastore = ast_channel_datastore_alloc(&chanspy_ds_info, NULL))) {
+		chanspy_ds = chanspy_ds_free(chanspy_ds);
+		ast_channel_unlock(chan);
+		return NULL;
+	}
+
+	datastore->data = chanspy_ds;
+
+	ast_channel_datastore_add(chan, datastore);
+
+	return chanspy_ds;
+}
+
+static struct chanspy_ds *next_channel(struct ast_channel *chan,
+	const struct ast_channel *last, const char *spec,
+	const char *exten, const char *context, struct chanspy_ds *chanspy_ds)
 {
 	struct ast_channel *this;
 
-	redo:
+redo:
 	if (!ast_strlen_zero(spec))
 		this = ast_walk_channel_by_name_prefix_locked(last, spec, strlen(spec));
 
@@ -394,20 +492,21 @@ static struct ast_channel *next_channel(const struct ast_channel *last, const ch
 	else
 		this = ast_channel_walk_locked(last);
 
-	if (this) {
+	if (!this)
+		return NULL;
+
+	if (!strncmp(this->name, "Zap/pseudo", 10)) {
 		ast_channel_unlock(this);
-		if (!strncmp(this->name, "Zap/pseudo", 10))
-			goto redo;
+		goto redo;
 	}
 
-	return this;
+	return setup_chanspy_ds(this, chanspy_ds);
 }
 
 static int common_exec(struct ast_channel *chan, const struct ast_flags *flags,
 	int volfactor, const int fd, const char *mygroup, const char *myenforced,
 	const char *spec, const char *exten, const char *context)
 {
-	struct ast_channel *peer, *prev, *next;
 	char nameprefix[AST_NAME_STRLEN];
 	char peer_name[AST_NAME_STRLEN + 5];
 	char exitcontext[AST_MAX_CONTEXT] = "";
@@ -417,6 +516,7 @@ static int common_exec(struct ast_channel *chan, const struct ast_flags *flags,
 	char *ptr;
 	int num;
 	int num_spyed_upon = 1;
+	struct chanspy_ds chanspy_ds;
 
 	if (ast_test_flag(flags, OPTION_EXIT)) {
 		const char *c;
@@ -428,6 +528,8 @@ static int common_exec(struct ast_channel *chan, const struct ast_flags *flags,
 			ast_copy_string(exitcontext, chan->context, sizeof(exitcontext));
 	}
 
+	ast_mutex_init(&chanspy_ds.lock);
+
 	if (chan->_state != AST_STATE_UP)
 		ast_answer(chan);
 
@@ -436,6 +538,9 @@ static int common_exec(struct ast_channel *chan, const struct ast_flags *flags,
 	waitms = 100;
 
 	for (;;) {
+		struct chanspy_ds *peer_chanspy_ds = NULL, *next_chanspy_ds = NULL;
+		struct ast_channel *prev = NULL, *peer = NULL;
+
 		if (!ast_test_flag(flags, OPTION_QUIET) && num_spyed_upon) {
 			res = ast_streamfile(chan, "beep", chan->language);
 			if (!res)
@@ -472,12 +577,13 @@ static int common_exec(struct ast_channel *chan, const struct ast_flags *flags,
 
 		/* reset for the next loop around, unless overridden later */
 		waitms = 100;
-		peer = prev = next = NULL;
 		num_spyed_upon = 0;
 
-		for (peer = next_channel(peer, spec, exten, context);
-			peer;
-			prev = peer, peer = next ? next : next_channel(peer, spec, exten, context), next = NULL) {
+		for (peer_chanspy_ds = next_channel(chan, prev, spec, exten, context, &chanspy_ds);
+		     peer_chanspy_ds;
+			 chanspy_ds_free(peer_chanspy_ds), prev = peer,
+		     peer_chanspy_ds = next_chanspy_ds ? next_chanspy_ds : 
+			 	next_channel(chan, prev, spec, exten, context, &chanspy_ds), next_chanspy_ds = NULL) {
 			const char *group;
 			int igrp = !mygroup;
 			char *groups[25];
@@ -490,18 +596,32 @@ static int common_exec(struct ast_channel *chan, const struct ast_flags *flags,
 			char *ext;
 			char *form_enforced;
 			int ienf = !myenforced;
+			struct ast_channel *peer;
 
-			if (peer == prev)
+			peer = peer_chanspy_ds->chan;
+
+			ast_mutex_unlock(&peer_chanspy_ds->lock);
+
+			if (peer == prev) {
+				ast_channel_unlock(peer);
+				chanspy_ds_free(peer_chanspy_ds);
 				break;
+			}
 
-			if (peer == chan)
+			if (peer == chan) {
+				ast_channel_unlock(peer);
 				continue;
+			}
 
-			if (ast_test_flag(flags, OPTION_BRIDGED) && !ast_bridged_channel(peer))
+			if (ast_test_flag(flags, OPTION_BRIDGED) && !ast_bridged_channel(peer)) {
+				ast_channel_unlock(peer);
 				continue;
+			}
 
-			if (ast_check_hangup(peer) || ast_test_flag(peer, AST_FLAG_SPYING))
+			if (ast_check_hangup(peer) || ast_test_flag(peer, AST_FLAG_SPYING)) {
+				ast_channel_unlock(peer);
 				continue;
+			}
 
 			if (mygroup) {
 				if ((group = pbx_builtin_getvar_helper(peer, "SPYGROUP"))) {
@@ -518,8 +638,10 @@ static int common_exec(struct ast_channel *chan, const struct ast_flags *flags,
 				}
 			}
 
-			if (!igrp)
+			if (!igrp) {
+				ast_channel_unlock(peer);
 				continue;
+			}
 
 			if (myenforced) {
 
@@ -559,6 +681,12 @@ static int common_exec(struct ast_channel *chan, const struct ast_flags *flags,
 
 			for (s = peer_name; s < ptr; s++)
 				*s = tolower(*s);
+		
+			/* We have to unlock the peer channel here to avoid a deadlock.
+			 * So, when we need it again, we have to lock the datastore and get
+			 * the pointer from there to see if the channel is still valid. */
+			ast_channel_unlock(peer);
+			peer = NULL;
 
 			if (!ast_test_flag(flags, OPTION_QUIET)) {
 				if (ast_fileexists(peer_name, NULL, NULL) != -1) {
@@ -574,8 +702,8 @@ static int common_exec(struct ast_channel *chan, const struct ast_flags *flags,
 			}
 
 			waitms = 5000;
-			res = channel_spy(chan, peer, &volfactor, fd, flags, exitcontext);
-			num_spyed_upon++;
+			res = channel_spy(chan, peer_chanspy_ds, &volfactor, fd, flags, exitcontext);
+			num_spyed_upon++;	
 
 			if (res == -1) {
 				goto exit;
@@ -583,13 +711,28 @@ static int common_exec(struct ast_channel *chan, const struct ast_flags *flags,
 				res = 0;
 				goto exit;
 			} else if (res > 1 && spec) {
+				struct ast_channel *next;
+
 				snprintf(nameprefix, AST_NAME_STRLEN, "%s/%d", spec, res);
+
 				if ((next = ast_get_channel_by_name_prefix_locked(nameprefix, strlen(nameprefix)))) {
-					ast_channel_unlock(next);
+					peer_chanspy_ds = chanspy_ds_free(peer_chanspy_ds);
+					next_chanspy_ds = setup_chanspy_ds(next, &chanspy_ds);
 				} else {
-					/* stay on this channel */
-					next = peer;
+					/* stay on this channel, if it is still valid */
+
+					ast_mutex_lock(&peer_chanspy_ds->lock);
+					if (peer_chanspy_ds->chan) {
+						ast_channel_lock(peer_chanspy_ds->chan);
+						next_chanspy_ds = peer_chanspy_ds;
+						peer_chanspy_ds = NULL;
+					} else {
+						/* the channel is gone */
+						ast_mutex_unlock(&peer_chanspy_ds->lock);
+						next_chanspy_ds = NULL;
+					}
 				}
+
 				peer = NULL;
 			}
 		}
@@ -600,6 +743,8 @@ exit:
 
 	ast_channel_setoption(chan, AST_OPTION_TXGAIN, &zero_volume, sizeof(zero_volume), 0);
 
+	ast_mutex_destroy(&chanspy_ds.lock);
+
 	return res;
 }