Index: channels/chan_sip.c =================================================================== --- channels/chan_sip.c (revision 317665) +++ channels/chan_sip.c (working copy) @@ -1280,6 +1280,10 @@ static void check_pendings(struct sip_pvt *p); static void *sip_park_thread(void *stuff); static int sip_park(struct ast_channel *chan1, struct ast_channel *chan2, struct sip_request *req, int seqno, char *parkexten); + +static void *sip_pickup_thread(void *stuff); +static int sip_pickup(struct ast_channel *chan); + static int sip_sipredirect(struct sip_pvt *p, const char *dest); static int is_method_allowed(unsigned int *allowed_methods, enum sipmethod method); @@ -20871,6 +20875,45 @@ return 0; } + +/*! \brief SIP pickup support function + * Starts in a new thread, then pickup the call + */ +static void *sip_pickup_thread(void *stuff) +{ + struct ast_channel *chan; + chan = stuff; + + if (ast_pickup_call(chan)) { + chan->hangupcause = AST_CAUSE_CALL_REJECTED; + } else { + chan->hangupcause = AST_CAUSE_NORMAL_CLEARING; + } + ast_hangup(chan); + ast_channel_unref(chan); + chan = NULL; + return NULL; +} + +/*! \brief Pickup a call using the subsystem in features.c + * This is executed in a separate thread + */ +static int sip_pickup(struct ast_channel *chan) +{ + pthread_t threadid; + + ast_channel_ref(chan); + + if (ast_pthread_create_detached_background(&threadid, NULL, sip_pickup_thread, chan)) { + ast_debug(1, "Unable to start Group pickup thread on channel %s\n", chan->name); + ast_channel_unref(chan); + return -1; + } + ast_debug(1, "Started Group pickup thread on channel %s\n", chan->name); + return 0; +} + + /*! \brief Turn off generator data XXX Does this function belong in the SIP channel? */ @@ -22282,29 +22325,29 @@ /* Unlock locks so ast_hangup can do its magic */ ast_channel_unlock(c); + *nounlock = 1; sip_pvt_unlock(p); ast_hangup(c); sip_pvt_lock(p); c = NULL; } } else { /* Pickup call in call group */ - ast_channel_unlock(c); - *nounlock = 1; - if (ast_pickup_call(c)) { - ast_log(LOG_NOTICE, "Nothing to pick up for %s\n", p->callid); - transmit_response_reliable(p, "503 Unavailable", req); + if (sip_pickup(c)) { + ast_log(LOG_WARNING, "Failed to start Group pickup by %s\n", c->name); + transmit_response_reliable(p, "480 Temporarily Unavailable", req); sip_alreadygone(p); + c->hangupcause = AST_CAUSE_FAILURE; + /* Unlock locks so ast_hangup can do its magic */ + ast_channel_unlock(c); + *nounlock = 1; + + p->invitestate = INV_COMPLETED; sip_pvt_unlock(p); - c->hangupcause = AST_CAUSE_CALL_REJECTED; - } else { - sip_pvt_unlock(p); - c->hangupcause = AST_CAUSE_NORMAL_CLEARING; + ast_hangup(c); + sip_pvt_lock(p); + c = NULL; } - p->invitestate = INV_COMPLETED; - ast_hangup(c); - sip_pvt_lock(p); - c = NULL; } break; case AST_STATE_RING: Index: apps/app_directed_pickup.c =================================================================== --- apps/app_directed_pickup.c (revision 317665) +++ apps/app_directed_pickup.c (working copy) @@ -97,60 +97,17 @@ static const char app2[] = "PickupChan"; /*! \todo This application should return a result code, like PICKUPRESULT */ -/* Perform actual pickup between two channels */ -static int pickup_do(struct ast_channel *chan, struct ast_channel *target) -{ - int res = 0; - struct ast_party_connected_line connected_caller; - struct ast_channel *chans[2] = { chan, target }; - - ast_debug(1, "Call pickup on '%s' by '%s'\n", target->name, chan->name); - ast_cel_report_event(target, AST_CEL_PICKUP, NULL, NULL, chan); - - ast_party_connected_line_init(&connected_caller); - ast_party_connected_line_copy(&connected_caller, &target->connected); - connected_caller.source = AST_CONNECTED_LINE_UPDATE_SOURCE_ANSWER; - if (ast_channel_connected_line_macro(NULL, chan, &connected_caller, 0, 0)) { - ast_channel_update_connected_line(chan, &connected_caller, NULL); - } - ast_party_connected_line_free(&connected_caller); - - ast_channel_lock(chan); - ast_connected_line_copy_from_caller(&connected_caller, &chan->caller); - ast_channel_unlock(chan); - connected_caller.source = AST_CONNECTED_LINE_UPDATE_SOURCE_ANSWER; - ast_channel_queue_connected_line_update(chan, &connected_caller, NULL); - ast_party_connected_line_free(&connected_caller); - - if ((res = ast_answer(chan))) { - ast_log(LOG_WARNING, "Unable to answer '%s'\n", chan->name); - return -1; - } - - if ((res = ast_queue_control(chan, AST_CONTROL_ANSWER))) { - ast_log(LOG_WARNING, "Unable to queue answer on '%s'\n", chan->name); - return -1; - } - - if ((res = ast_channel_masquerade(target, chan))) { - ast_log(LOG_WARNING, "Unable to masquerade '%s' into '%s'\n", chan->name, target->name); - return -1; - } - - /* If you want UniqueIDs, set channelvars in manager.conf to CHANNEL(uniqueid) */ - ast_manager_event_multichan(EVENT_FLAG_CALL, "Pickup", 2, chans, - "Channel: %s\r\nTargetChannel: %s\r\n", chan->name, target->name); - - return res; -} - /* Helper function that determines whether a channel is capable of being picked up */ static int can_pickup(struct ast_channel *chan) { - if (!chan->pbx && (chan->_state == AST_STATE_RINGING || chan->_state == AST_STATE_RING || chan->_state == AST_STATE_DOWN)) + if (!chan->pbx && !chan->masq && + !ast_test_flag(chan, AST_FLAG_ZOMBIE) && + (chan->_state == AST_STATE_RINGING || + chan->_state == AST_STATE_RING || + chan->_state == AST_STATE_DOWN)) { return 1; - else - return 0; + } + return 0; } struct pickup_by_name_args { @@ -213,9 +170,8 @@ /* Just check that we are not picking up the SAME as target */ if (chan != target) { - res = pickup_do(chan, target); + res = ast_do_pickup(chan, target); } - ast_channel_unlock(target); target = ast_channel_unref(target); @@ -236,6 +192,7 @@ while ((target = ast_channel_iterator_next(iter))) { ast_channel_lock(target); if ((chan != target) && can_pickup(target)) { + ast_log(LOG_NOTICE, "%s pickup by %s\n", target->name, chan->name); break; } ast_channel_unlock(target); @@ -245,7 +202,7 @@ ast_channel_iterator_destroy(iter); if (target) { - res = pickup_do(chan, target); + res = ast_do_pickup(chan, target); ast_channel_unlock(target); target = ast_channel_unref(target); } @@ -277,16 +234,58 @@ struct ast_channel *target; int res = -1; - if ((target = ast_channel_callback(find_by_mark, NULL, (char *) mark, 0))) { - ast_channel_lock(target); - res = pickup_do(chan, target); - ast_channel_unlock(target); - target = ast_channel_unref(target); + if (!(target = ast_channel_callback(find_by_mark, NULL, (char *) mark, 0))) { + return res; } + 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); + } + ast_channel_unlock(target); + target = ast_channel_unref(target); + return res; } +static int find_channel_by_group(void *obj, void *arg, void *data, int flags) +{ + 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); + + ast_channel_unlock(chan); + return i ? CMP_MATCH | CMP_STOP : 0; +} + +static int pickup_by_group(struct ast_channel *chan) +{ + 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)) { + 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); + + return res; +} + /* application entry point for Pickup() */ static int pickup_exec(struct ast_channel *chan, const char *data) { @@ -295,10 +294,10 @@ char *exten = NULL, *context = NULL; if (ast_strlen_zero(data)) { - res = ast_pickup_call(chan); + res = pickup_by_group(chan); return res; } - + /* Parse extension (and context if there) */ while (!ast_strlen_zero(tmp) && (exten = strsep(&tmp, "&"))) { if ((context = strchr(exten, '@'))) @@ -341,7 +340,11 @@ if ((target = ast_channel_callback(find_by_part, NULL, (char *) part, 0))) { ast_channel_lock(target); - res = pickup_do(chan, 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); + } ast_channel_unlock(target); target = ast_channel_unref(target); } Index: include/asterisk/features.h =================================================================== --- include/asterisk/features.h (revision 317665) +++ include/asterisk/features.h (working copy) @@ -122,6 +122,14 @@ /*! \brief Pickup a call */ int ast_pickup_call(struct ast_channel *chan); +/*! + * \brief Pickup a call target + * \note This function assumes that target is locked + * \retval 0 on success. + * \retval -1 on failure. + */ +int ast_do_pickup(struct ast_channel *chan, struct ast_channel *target); + /*! * \brief register new feature into feature_set * \param feature an ast_call_feature object which contains a keysequence Index: main/features.c =================================================================== --- main/features.c (revision 317665) +++ main/features.c (working copy) @@ -5693,15 +5693,17 @@ { struct ast_channel *c = data; struct ast_channel *chan = obj; + int i; - int i = !chan->pbx && + i = !chan->pbx && /* Accessing 'chan' here is safe without locking, because there is no way for - the channel do disappear from under us at this point. pickupgroup *could* + 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) && - (chan->pickupgroup & c->callgroup) && + (c->pickupgroup & chan->callgroup) && ((chan->_state == AST_STATE_RINGING) || (chan->_state == AST_STATE_RING)) && - !c->masq; + !chan->masq && + !ast_test_flag(chan, AST_FLAG_ZOMBIE); return i ? CMP_MATCH | CMP_STOP : 0; } @@ -5713,68 +5715,106 @@ * Walk list of channels, checking it is not itself, channel is pbx one, * check that the callgroup for both channels are the same and the channel is ringing. * Answer calling channel, flag channel as answered on queue, masq channels together. -*/ + */ int ast_pickup_call(struct ast_channel *chan) { - struct ast_channel *cur, *chans[2] = { chan, }; - struct ast_party_connected_line connected_caller; - int res; - const char *chan_name; - const char *cur_name; + struct ast_channel *target; + int res = -1; + ast_debug(1, "pickup attempt by %s\n", chan->name); - if (!(cur = ast_channel_callback(find_channel_by_group, NULL, chan, 0))) { - ast_debug(1, "No call pickup possible...\n"); + /* 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); + + /* 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); + } + } else { + ast_channel_unlock(target); + } + target = ast_channel_unref(target); + } + + if (res < 0) { + ast_debug(1, "No call pickup possible... for %s\n", chan->name); if (!ast_strlen_zero(pickupfailsound)) { + ast_answer(chan); ast_stream_and_wait(chan, pickupfailsound, ""); } - return -1; } - chans[1] = cur; + return res; +} - ast_channel_lock_both(cur, chan); +/*! + * \brief Pickup a call target, Common Code. + * \param chan channel that initiated pickup. + * \param target channel. + * + * Answer calling channel, flag channel as answered on queue, masq channels together. + */ +int ast_do_pickup(struct ast_channel *chan, struct ast_channel *target) +{ + struct ast_party_connected_line connected_caller; + struct ast_channel *chans[2] = { chan, target }; - cur_name = ast_strdupa(cur->name); - chan_name = ast_strdupa(chan->name); + ast_debug(1, "Call pickup on '%s' by '%s'\n", target->name, chan->name); + ast_cel_report_event(target, AST_CEL_PICKUP, NULL, NULL, chan); - ast_debug(1, "Call pickup on chan '%s' by '%s'\n", cur_name, chan_name); - - connected_caller = cur->connected; + ast_party_connected_line_init(&connected_caller); + ast_party_connected_line_copy(&connected_caller, &target->connected); connected_caller.source = AST_CONNECTED_LINE_UPDATE_SOURCE_ANSWER; if (ast_channel_connected_line_macro(NULL, chan, &connected_caller, 0, 0)) { ast_channel_update_connected_line(chan, &connected_caller, NULL); } + ast_party_connected_line_free(&connected_caller); - ast_party_connected_line_collect_caller(&connected_caller, &chan->caller); + ast_channel_lock(chan); + ast_connected_line_copy_from_caller(&connected_caller, &chan->caller); + ast_channel_unlock(chan); connected_caller.source = AST_CONNECTED_LINE_UPDATE_SOURCE_ANSWER; ast_channel_queue_connected_line_update(chan, &connected_caller, NULL); + ast_party_connected_line_free(&connected_caller); - ast_channel_unlock(cur); - ast_channel_unlock(chan); - if (ast_answer(chan)) { - ast_log(LOG_WARNING, "Unable to answer '%s'\n", chan_name); + ast_log(LOG_WARNING, "Unable to answer '%s'\n", chan->name); + return -1; } if (ast_queue_control(chan, AST_CONTROL_ANSWER)) { - ast_log(LOG_WARNING, "Unable to queue answer on '%s'\n", chan_name); + ast_log(LOG_WARNING, "Unable to queue answer on '%s'\n", chan->name); + return -1; } - if ((res = ast_channel_masquerade(cur, chan))) { - ast_log(LOG_WARNING, "Unable to masquerade '%s' into '%s'\n", chan_name, cur_name); + if (ast_channel_masquerade(target, chan)) { + ast_log(LOG_WARNING, "Unable to masquerade '%s' into '%s'\n", chan->name, target->name); + return -1; } - if (!ast_strlen_zero(pickupsound)) { - ast_stream_and_wait(cur, pickupsound, ""); - } - /* If you want UniqueIDs, set channelvars in manager.conf to CHANNEL(uniqueid) */ ast_manager_event_multichan(EVENT_FLAG_CALL, "Pickup", 2, chans, - "Channel: %s\r\nTargetChannel: %s\r\n", chan->name, cur->name); + "Channel: %s\r\nTargetChannel: %s\r\n", chan->name, target->name); - cur = ast_channel_unref(cur); - - return res; + return 0; } static char *app_bridge = "Bridge";