diff --git a/bridges/bridge_native_rtp.c b/bridges/bridge_native_rtp.c index 27d8d77cac869ff8af1f1054b24cb28107ccd781..7edcfd71aadfffe3ef19788a080e5b8e72c7832d 100644 --- a/bridges/bridge_native_rtp.c +++ b/bridges/bridge_native_rtp.c @@ -112,7 +112,16 @@ static enum ast_rtp_glue_result native_rtp_bridge_get(struct ast_channel *c0, st return audio_glue0_res; } -static int native_rtp_bridge_start(struct ast_bridge *bridge, struct ast_channel *target) +/*! + * \internal + * \brief Start native RTP bridging of two channels + * + * \param bridge The bridge that had native RTP bridging happening on it + * \param target If remote RTP bridging, the channel that is unheld. + * + * \note Bridge must be locked when calling this function. + */ +static void native_rtp_bridge_start(struct ast_bridge *bridge, struct ast_channel *target) { struct ast_bridge_channel *c0 = AST_LIST_FIRST(&bridge->channels); struct ast_bridge_channel *c1 = AST_LIST_LAST(&bridge->channels); @@ -128,18 +137,12 @@ static int native_rtp_bridge_start(struct ast_bridge *bridge, struct ast_channel RAII_VAR(struct ast_format_cap *, cap1, ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_NOLOCK), ast_format_cap_destroy); if (c0 == c1) { - return 0; + return; } + ast_channel_lock_both(c0->chan, c1->chan); native_type = native_rtp_bridge_get(c0->chan, c1->chan, &glue0, &glue1, &instance0, &instance1, &vinstance0, &vinstance1); - if (glue0->get_codec) { - glue0->get_codec(c0->chan, cap0); - } - if (glue1->get_codec) { - glue1->get_codec(c1->chan, cap1); - } - switch (native_type) { case AST_RTP_GLUE_RESULT_LOCAL: if (ast_rtp_instance_get_engine(instance0)->local_bridge) { @@ -155,6 +158,12 @@ static int native_rtp_bridge_start(struct ast_bridge *bridge, struct ast_channel break; case AST_RTP_GLUE_RESULT_REMOTE: + if (glue0->get_codec) { + glue0->get_codec(c0->chan, cap0); + } + if (glue1->get_codec) { + glue1->get_codec(c1->chan, cap1); + } /* If we have a target, it's the channel that received the UNHOLD or UPDATE_RTP_PEER frame and was told to resume */ if (!target) { @@ -180,7 +189,8 @@ static int native_rtp_bridge_start(struct ast_bridge *bridge, struct ast_channel break; } - return 0; + ast_channel_unlock(c0->chan); + ast_channel_unlock(c1->chan); } static void native_rtp_bridge_stop(struct ast_bridge *bridge, struct ast_channel *target) @@ -202,6 +212,7 @@ static void native_rtp_bridge_stop(struct ast_bridge *bridge, struct ast_channel return; } + ast_channel_lock_both(c0->chan, c1->chan); native_type = native_rtp_bridge_get(c0->chan, c1->chan, &glue0, &glue1, &instance0, &instance1, &vinstance0, &vinstance1); switch (native_type) { @@ -241,6 +252,9 @@ static void native_rtp_bridge_stop(struct ast_bridge *bridge, struct ast_channel ast_debug(2, "Discontinued RTP bridging of '%s' and '%s' - media will flow through Asterisk core\n", ast_channel_name(c0->chan), ast_channel_name(c1->chan)); + + ast_channel_unlock(c0->chan); + ast_channel_unlock(c1->chan); } /*! \brief Frame hook that is called to intercept hold/unhold */ @@ -252,16 +266,23 @@ static struct ast_frame *native_rtp_framehook(struct ast_channel *chan, struct a return f; } - ast_channel_lock(chan); bridge = ast_channel_get_bridge(chan); - ast_channel_unlock(chan); if (bridge) { + /* native_rtp_bridge_start/stop are not being called from bridging + core so we need to lock the bridge prior to calling these functions + Unfortunately that means unlocking the channel, but as it + should not be modified this should be okay...hopefully */ + ast_channel_unlock(chan); + ast_bridge_lock(bridge); if (f->subclass.integer == AST_CONTROL_HOLD) { native_rtp_bridge_stop(bridge, chan); } else if ((f->subclass.integer == AST_CONTROL_UNHOLD) || (f->subclass.integer == AST_CONTROL_UPDATE_RTP_PEER)) { native_rtp_bridge_start(bridge, chan); } + ast_bridge_unlock(bridge); + ast_channel_lock(chan); + } return f; @@ -412,7 +433,8 @@ static int native_rtp_bridge_join(struct ast_bridge *bridge, struct ast_bridge_c return -1; } - return native_rtp_bridge_start(bridge, NULL); + native_rtp_bridge_start(bridge, NULL); + return 0; } static void native_rtp_bridge_unsuspend(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel) diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c index 8db3f094fce1596114b1edcbb345802ed0909ddb..21a79f0edb98b6c6dde9f4e67f89a6f1c8e26713 100644 --- a/channels/chan_pjsip.c +++ b/channels/chan_pjsip.c @@ -292,14 +292,11 @@ static int chan_pjsip_set_rtp_peer(struct ast_channel *chan, struct chan_pjsip_pvt *pvt = channel->pvt; struct ast_sip_session *session = channel->session; int changed = 0; - struct ast_channel *bridge_peer; /* Don't try to do any direct media shenanigans on early bridges */ - bridge_peer = ast_channel_bridge_peer(chan); - if ((rtp || vrtp || tpeer) && !bridge_peer) { + if ((rtp || vrtp || tpeer) && !ast_channel_is_bridged(chan)) { return 0; } - ast_channel_cleanup(bridge_peer); if (nat_active && session->endpoint->media.direct_media.disable_on_nat) { return 0; diff --git a/channels/chan_sip.c b/channels/chan_sip.c index 5d729cd0201684e221f7ace14b96573bed9d1080..a7649127e79d95af9d0ace28b1bc8d9143ff2fd6 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -32667,11 +32667,8 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i struct sip_pvt *p; int changed = 0; - /* Lock the channel and the private safely. */ - ast_channel_lock(chan); p = ast_channel_tech_pvt(chan); if (!p) { - ast_channel_unlock(chan); return -1; } sip_pvt_lock(p); @@ -32679,7 +32676,6 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i /* I suppose it could be argued that if this happens it is a bug. */ ast_debug(1, "The private is not owned by channel %s anymore.\n", ast_channel_name(chan)); sip_pvt_unlock(p); - ast_channel_unlock(chan); return 0; } @@ -32688,14 +32684,12 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i !ast_channel_is_bridged(chan) && !sip_cfg.directrtpsetup) { sip_pvt_unlock(p); - ast_channel_unlock(chan); return 0; } if (p->alreadygone) { /* If we're destroyed, don't bother */ sip_pvt_unlock(p); - ast_channel_unlock(chan); return 0; } @@ -32704,7 +32698,6 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i */ if (nat_active && !ast_test_flag(&p->flags[0], SIP_DIRECT_MEDIA_NAT)) { sip_pvt_unlock(p); - ast_channel_unlock(chan); return 0; } @@ -32770,7 +32763,6 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i */ ast_clear_flag(&p->flags[2], SIP_PAGE3_DIRECT_MEDIA_OUTGOING); sip_pvt_unlock(p); - ast_channel_unlock(chan); return 0; } @@ -32791,7 +32783,6 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i /* Reset lastrtprx timer */ p->lastrtprx = p->lastrtptx = time(NULL); sip_pvt_unlock(p); - ast_channel_unlock(chan); return 0; } diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h index 7db2b3c3a2914315c834beb7d6edaa0ff472e3bb..03d9aef68a3ed9be38d03e3a27146c2296b03f54 100644 --- a/include/asterisk/channel.h +++ b/include/asterisk/channel.h @@ -4214,6 +4214,8 @@ int ast_channel_is_bridged(const struct ast_channel *chan); * \note The returned peer channel is the current peer in the * bridge when called. * + * \note Absolutely _NO_ channel locks should be held when calling this function. + * * \retval NULL Channel not in a bridge or the bridge is not two-party. * \retval non-NULL Reffed peer channel at time of calling. */ diff --git a/main/channel.c b/main/channel.c index 58b50c820bb8a294e3155219fc8bbb8fb590eb5d..d1643658e1e05bd6667c87d67d306146c856c996 100644 --- a/main/channel.c +++ b/main/channel.c @@ -6869,13 +6869,11 @@ void ast_do_masquerade(struct ast_channel *original) } ast_debug(1, "Done Masquerading %s (%d)\n", ast_channel_name(original), ast_channel_state(original)); + ast_channel_unlock(original); if ((bridged = ast_channel_bridge_peer(original))) { - ast_channel_unlock(original); ast_indicate(bridged, AST_CONTROL_SRCCHANGE); ast_channel_unref(bridged); - } else { - ast_channel_unlock(original); } ast_indicate(original, AST_CONTROL_SRCCHANGE);