Index: apps/app_directed_pickup.c
===================================================================
--- apps/app_directed_pickup.c	(revision 319526)
+++ apps/app_directed_pickup.c	(working copy)
@@ -158,23 +158,22 @@
 	return ast_channel_callback(pickup_by_name_cb, NULL, &pickup_args, 0);
 }
 
-/*! \brief Attempt to pick up specified channel named , does not use context */
+/*! \brief Attempt to pick up named channel, does not use context */
 static int pickup_by_channel(struct ast_channel *chan, char *pickup)
 {
-	int res = 0;
+	int res = -1;
 	struct ast_channel *target;
 
-	if (!(target = my_ast_get_channel_by_name_locked(pickup))) {
-		return -1;
+	target = my_ast_get_channel_by_name_locked(pickup);
+	if (target) {
+		/* Just check that we are not picking up the SAME as target. (i.e. ourself) */
+		if (chan != target) {
+			res = ast_do_pickup(chan, target);
+		}
+		ast_channel_unlock(target);
+		target = ast_channel_unref(target);
 	}
 
-	/* Just check that we are not picking up the SAME as target */
-	if (chan != target) {
-		res = ast_do_pickup(chan, target);
-	}
-	ast_channel_unlock(target);
-	target = ast_channel_unref(target);
-
 	return res;
 }
 
@@ -215,17 +214,16 @@
 	struct ast_channel *c = obj;
 	const char *mark = data;
 	const char *tmp;
-	int res;
 
 	ast_channel_lock(c);
-
-	res = (tmp = pbx_builtin_getvar_helper(c, PICKUPMARK)) &&
-		!strcasecmp(tmp, mark) &&
-		can_pickup(c);
-
+	tmp = pbx_builtin_getvar_helper(c, PICKUPMARK);
+	if (tmp && !strcasecmp(tmp, mark) && can_pickup(c)) {
+		/* Return with the channel still locked on purpose */
+		return CMP_MATCH | CMP_STOP;
+	}
 	ast_channel_unlock(c);
 
-	return res ? CMP_MATCH | CMP_STOP : 0;
+	return 0;
 }
 
 /* Attempt to pick up specified mark */
@@ -234,18 +232,13 @@
 	struct ast_channel *target;
 	int res = -1;
 
-	if (!(target = ast_channel_callback(find_by_mark, NULL, (char *) mark, 0))) {
-		return res;
-	}
-
-	ast_channel_lock(target);
-	if (can_pickup(target)) {
+	/* The found channel is already locked. */
+	target = ast_channel_callback(find_by_mark, NULL, (char *) mark, 0);
+	if (target) {
 		res = ast_do_pickup(chan, target);
-	} else {
-		ast_log(LOG_WARNING, "target has gone, or not ringing anymore for %s\n", chan->name);
+		ast_channel_unlock(target);
+		target = ast_channel_unref(target);
 	}
-	ast_channel_unlock(target);
-	target = ast_channel_unref(target);
 
 	return res;
 }
@@ -254,14 +247,15 @@
 {
 	struct ast_channel *chan = obj;
 	struct ast_channel *c = data;
-	int i;
 
 	ast_channel_lock(chan);
-	i = (c != chan) && (c->pickupgroup & chan->callgroup) &&
-		can_pickup(chan);
-
+	if (c != chan && (c->pickupgroup & chan->callgroup) && can_pickup(chan)) {
+		/* Return with the channel still locked on purpose */
+		return CMP_MATCH | CMP_STOP;
+	}
 	ast_channel_unlock(chan);
-	return i ? CMP_MATCH | CMP_STOP : 0;
+
+	return 0;
 }
 
 static int pickup_by_group(struct ast_channel *chan)
@@ -269,19 +263,14 @@
 	struct ast_channel *target;
 	int res = -1;
 
-	if (!(target = ast_channel_callback(find_channel_by_group, NULL, chan, 0))) {
-		return res;
-	}
-
-	ast_log(LOG_NOTICE, "%s, pickup attempt by %s\n", target->name, chan->name);
-	ast_channel_lock(target);
-	if (can_pickup(target)) {
+	/* The found channel is already locked. */
+	target = ast_channel_callback(find_channel_by_group, NULL, chan, 0);
+	if (target) {
+		ast_log(LOG_NOTICE, "pickup %s attempt by %s\n", target->name, chan->name);
 		res = ast_do_pickup(chan, target);
-	} else {
-		ast_log(LOG_WARNING, "target has gone, or not ringing anymore for %s\n", chan->name);
+		ast_channel_unlock(target);
+		target = ast_channel_unref(target);
 	}
-	ast_channel_unlock(target);
-	target = ast_channel_unref(target);
 
 	return res;
 }
@@ -289,13 +278,11 @@
 /* application entry point for Pickup() */
 static int pickup_exec(struct ast_channel *chan, const char *data)
 {
-	int res = 0;
 	char *tmp = ast_strdupa(data);
 	char *exten = NULL, *context = NULL;
 
 	if (ast_strlen_zero(data)) {
-		res = pickup_by_group(chan);
-		return res;
+		return pickup_by_group(chan) ? 0 : -1;
 	}
 
 	/* Parse extension (and context if there) */
@@ -303,16 +290,21 @@
 		if ((context = strchr(exten, '@')))
 			*context++ = '\0';
 		if (!ast_strlen_zero(context) && !strcasecmp(context, PICKUPMARK)) {
-			if (!pickup_by_mark(chan, exten))
-				break;
+			if (!pickup_by_mark(chan, exten)) {
+				/* Pickup successful.  Stop the dialplan this channel is a zombie. */
+				return -1;
+			}
 		} else {
-			if (!pickup_by_exten(chan, exten, !ast_strlen_zero(context) ? context : chan->context))
-				break;
+			if (!pickup_by_exten(chan, exten, !ast_strlen_zero(context) ? context : chan->context)) {
+				/* Pickup successful.  Stop the dialplan this channel is a zombie. */
+				return -1;
+			}
 		}
 		ast_log(LOG_NOTICE, "No target channel found for %s.\n", exten);
 	}
 
-	return res;
+	/* Pickup failed.  Keep going in the dialplan. */
+	return 0;
 }
 
 /* Find channel for pick up specified by partial channel name */ 
@@ -320,16 +312,16 @@
 {
 	struct ast_channel *c = obj; 
 	const char *part = data;
-	int res = 0;
 	int len = strlen(part);
 
 	ast_channel_lock(c);
-	if (len <= strlen(c->name)) {
-		res = !(strncmp(c->name, part, len)) && (can_pickup(c));
+	if (len <= strlen(c->name) && !strncmp(c->name, part, len) && can_pickup(c)) {
+		/* Return with the channel still locked on purpose */
+		return CMP_MATCH | CMP_STOP;
 	}
 	ast_channel_unlock(c);
 
-	return res ? CMP_MATCH | CMP_STOP : 0;
+	return 0;
 }
 
 /* Attempt to pick up specified by partial channel name */ 
@@ -338,13 +330,10 @@
 	struct ast_channel *target;
 	int res = -1;
 
-	if ((target = ast_channel_callback(find_by_part, NULL, (char *) part, 0))) {
-		ast_channel_lock(target);
-		if (can_pickup(target)) {
-			res = ast_do_pickup(chan, target);
-		} else {
-			ast_log(LOG_WARNING, "target has gone, or not ringing anymore for %s\n", chan->name);
-		}
+	/* The found channel is already locked. */
+	target = ast_channel_callback(find_by_part, NULL, (char *) part, 0);
+	if (target) {
+		res = ast_do_pickup(chan, target);
 		ast_channel_unlock(target);
 		target = ast_channel_unref(target);
 	}
@@ -355,7 +344,6 @@
 /* application entry point for PickupChan() */
 static int pickupchan_exec(struct ast_channel *chan, const char *data)
 {
-	int res = 0;
 	int partial_pickup = 0;
 	char *pickup = NULL;
 	char *parse = ast_strdupa(data);
@@ -367,7 +355,8 @@
 
 	if (ast_strlen_zero(args.channel)) {
 		ast_log(LOG_WARNING, "PickupChan requires an argument (channel)!\n");
-		return -1;
+		/* Pickup failed.  Keep going in the dialplan. */
+		return 0;
 	}
 
 	if (!ast_strlen_zero(args.options) && strchr(args.options, 'p')) {
@@ -381,16 +370,19 @@
 		} else {
 			if (partial_pickup) {
 				if (!pickup_by_part(chan, pickup)) {
-					break;
+					/* Pickup successful.  Stop the dialplan this channel is a zombie. */
+					return -1;
 				}
 			} else if (!pickup_by_channel(chan, pickup)) {
-				break;
+				/* Pickup successful.  Stop the dialplan this channel is a zombie. */
+				return -1;
 			}
 			ast_log(LOG_NOTICE, "No target channel found for %s.\n", pickup);
 		}
 	}
 
-	return res;
+	/* Pickup failed.  Keep going in the dialplan. */
+	return 0;
 }
 
 static int unload_module(void)
Index: main/features.c
===================================================================
--- main/features.c	(revision 319526)
+++ main/features.c	(working copy)
@@ -5685,21 +5685,21 @@
 
 static int find_channel_by_group(void *obj, void *arg, void *data, int flags)
 {
-	struct ast_channel *c = data;
 	struct ast_channel *chan = obj;
-	int i;
+	struct ast_channel *c = data;
 
-	i = !chan->pbx &&
-		/* Accessing 'chan' here is safe without locking, because there is no way for
-		   the channel to disappear from under us at this point.  pickupgroup *could*
-		   change while we're here, but that isn't a problem. */
-		(c != chan) &&
-		(c->pickupgroup & chan->callgroup) &&
+	ast_channel_lock(chan);
+	if (c != chan && (c->pickupgroup & chan->callgroup) &&
+		!chan->pbx &&
 		((chan->_state == AST_STATE_RINGING) || (chan->_state == AST_STATE_RING)) &&
 		!chan->masq &&
-		!ast_test_flag(chan, AST_FLAG_ZOMBIE);
+		!ast_test_flag(chan, AST_FLAG_ZOMBIE)) {
+		/* Return with the channel still locked on purpose */
+		return CMP_MATCH | CMP_STOP;
+	}
+	ast_channel_unlock(chan);
 
-	return i ? CMP_MATCH | CMP_STOP : 0;
+	return 0;
 }
 
 /*!
@@ -5716,35 +5716,24 @@
 	int res = -1;
 	ast_debug(1, "pickup attempt by %s\n", chan->name);
 
-	/* If 2 callers try to pickup the same call, allow 1 to win, and other to fail early.*/
-	if ((target = ast_channel_callback(find_channel_by_group, NULL, chan, 0))) {
-		ast_channel_lock(target);
+	/* The found channel is already locked. */
+	target = ast_channel_callback(find_channel_by_group, NULL, chan, 0);
+	if (target) {
+		ast_log(LOG_NOTICE, "pickup %s attempt by %s\n", target->name, chan->name);
 
-		/* We now have the target locked, but is the target still available to pickup */
-		if (!target->masq && !ast_test_flag(target, AST_FLAG_ZOMBIE) &&
-			((target->_state == AST_STATE_RINGING) || (target->_state == AST_STATE_RING))) {
-
-			ast_log(LOG_NOTICE, "pickup %s attempt by %s\n", target->name, chan->name);
-
-			/* A 2nd pickup attempt will bail out when we unlock the target */
-			if (!(res = ast_do_pickup(chan, target))) {
-				ast_channel_unlock(target);
-				if (!(res = ast_do_masquerade(target))) {
-					if (!ast_strlen_zero(pickupsound)) {
-						ast_channel_lock(target);
-						ast_stream_and_wait(target, pickupsound, "");
-						ast_channel_unlock(target);
-					}
-				} else {
-					ast_log(LOG_WARNING, "Failed to perform masquerade\n");
-				}
-			} else {
-				ast_log(LOG_WARNING, "pickup %s failed by %s\n", target->name, chan->name);
-				ast_channel_unlock(target);
-			}
+		res = ast_do_pickup(chan, target);
+		if (!res) {
+			/*!
+			 * \todo We are not the bridge thread when we inject this sound
+			 * so we need to hold the target channel lock while the sound is
+			 * played.  A better way needs to be found as this pauses the
+			 * system.
+			 */
+			ast_stream_and_wait(target, pickupsound, "");
 		} else {
-			ast_channel_unlock(target);
+			ast_log(LOG_WARNING, "pickup %s failed by %s\n", target->name, chan->name);
 		}
+		ast_channel_unlock(target);
 		target = ast_channel_unref(target);
 	}
 
@@ -5808,6 +5797,11 @@
 	ast_manager_event_multichan(EVENT_FLAG_CALL, "Pickup", 2, chans,
 		"Channel: %s\r\nTargetChannel: %s\r\n", chan->name, target->name);
 
+	/* Do the masquerade manually to make sure that is is completed. */
+	ast_channel_unlock(target);
+	ast_do_masquerade(target);
+	ast_channel_lock(target);
+
 	return 0;
 }