diff options
author | Richard Mudgett <rmudgett@digium.com> | 2014-04-15 17:07:20 +0000 |
---|---|---|
committer | Richard Mudgett <rmudgett@digium.com> | 2014-04-15 17:07:20 +0000 |
commit | d28af99e65c79f5bb1d336218f37de32313181db (patch) | |
tree | 164f1f4724c00609c38da49803a9b40e81f21c9a /channels/chan_sip.c | |
parent | c6a2a513c22f226f17def26b5283eaad8e367c15 (diff) |
chan_sip.c: Fix channel staging assertion failure.
The failing assertion ensures that the final snapshot gets generated so
CDR records can get finalized. The only place where a channel staging
snapshot flag could be left set is in chan_sip.c:handle_request_bye().
The function could return before clearing the flag because the channel
could dissappear while the function had to have the channel unlocked.
* Fixed handle_request_bye() channel snapshot staging coverage area to not
have a return in the middle of it and be unable to clear the staging flag.
* Pushed the channel snapshot staging coverage area into
ast_rtp_instance_set_stats_vars() to ensure that the staging is not
interrutped.
* Made callers of ast_rtp_instance_set_stats_vars() not call it with any
channels or channel driver private locks held to eliminate the deadlock
potential. The callers must hold references to the passed in channel and
rtp objects.
* Eliminated sip_hangup() trying to get the bridge peer. It is futile at
this point because the channel could never be in a bridge.
Review: https://reviewboard.asterisk.org/r/3431/
........
Merged revisions 412385 from http://svn.asterisk.org/svn/asterisk/branches/12
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@412386 65c4cc65-6c06-0410-ace0-fbb531ad65f3
Diffstat (limited to 'channels/chan_sip.c')
-rw-r--r-- | channels/chan_sip.c | 84 |
1 files changed, 46 insertions, 38 deletions
diff --git a/channels/chan_sip.c b/channels/chan_sip.c index ef57015dc..eb0a5f295 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -7248,40 +7248,29 @@ static int sip_hangup(struct ast_channel *ast) } if (!p->pendinginvite) { - RAII_VAR(struct ast_channel *, bridge, ast_channel_bridge_peer(oldowner), ast_channel_cleanup); - char quality_buf[AST_MAX_USER_FIELD], *quality; - - /* We need to get the lock on bridge because ast_rtp_instance_set_stats_vars will attempt - * to lock the bridge. This may get hairy... - */ - while (bridge && ast_channel_trylock(bridge)) { - sip_pvt_unlock(p); - do { - CHANNEL_DEADLOCK_AVOIDANCE(oldowner); - } while (sip_pvt_trylock(p)); - } - - if (p->rtp || p->vrtp || p->trtp) { - ast_channel_stage_snapshot(oldowner); - } + char *quality; + char quality_buf[AST_MAX_USER_FIELD]; if (p->rtp) { - ast_rtp_instance_set_stats_vars(oldowner, p->rtp); - } + struct ast_rtp_instance *p_rtp; - if (bridge) { - struct sip_pvt *q = ast_channel_tech_pvt(bridge); - - if (IS_SIP_TECH(ast_channel_tech(bridge)) && q && q->rtp) { - ast_rtp_instance_set_stats_vars(bridge, q->rtp); - } - ast_channel_unlock(bridge); + p_rtp = p->rtp; + ao2_ref(p_rtp, +1); + ast_channel_unlock(oldowner); + sip_pvt_unlock(p); + ast_rtp_instance_set_stats_vars(oldowner, p_rtp); + ao2_ref(p_rtp, -1); + ast_channel_lock(oldowner); + sip_pvt_lock(p); } /* * The channel variables are set below just to get the AMI * VarSet event because the channel is being hungup. */ + if (p->rtp || p->vrtp || p->trtp) { + ast_channel_stage_snapshot(oldowner); + } if (p->rtp && (quality = ast_rtp_instance_get_quality(p->rtp, AST_RTP_INSTANCE_STAT_FIELD_QUALITY, quality_buf, sizeof(quality_buf)))) { if (p->do_history) { append_history(p, "RTCPaudio", "Quality:%s", quality); @@ -26443,10 +26432,6 @@ static int handle_request_bye(struct sip_pvt *p, struct sip_request *req) } } - if ((p->rtp || p->vrtp || p->trtp) && p->owner) { - ast_channel_stage_snapshot(p->owner); - } - /* Get RTCP quality before end of call */ if (p->rtp) { if (p->do_history) { @@ -26467,22 +26452,49 @@ static int handle_request_bye(struct sip_pvt *p, struct sip_request *req) if (p->owner) { RAII_VAR(struct ast_channel *, owner_relock, NULL, ast_channel_cleanup); RAII_VAR(struct ast_channel *, owner_ref, NULL, ast_channel_cleanup); + struct ast_rtp_instance *p_rtp; /* Grab a reference to p->owner to prevent it from going away */ owner_ref = ast_channel_ref(p->owner); + p_rtp = p->rtp; + ao2_ref(p_rtp, +1); + /* Established locking order here is bridge, channel, pvt * and the bridge and channel will be locked during * ast_rtp_instance_set_stats_vars */ ast_channel_unlock(owner_ref); sip_pvt_unlock(p); - ast_rtp_instance_set_stats_vars(owner_ref, p->rtp); - if (peer_channel && IS_SIP_TECH(ast_channel_tech(peer_channel))) { - struct sip_pvt *q = ast_channel_tech_pvt(peer_channel); - if (q && q->rtp) { - ast_rtp_instance_set_stats_vars(peer_channel, q->rtp); + ast_rtp_instance_set_stats_vars(owner_ref, p_rtp); + ao2_ref(p_rtp, -1); + + if (peer_channel) { + ast_channel_lock(peer_channel); + if (IS_SIP_TECH(ast_channel_tech(peer_channel))) { + struct sip_pvt *peer_pvt; + + peer_pvt = ast_channel_tech_pvt(peer_channel); + if (peer_pvt) { + ao2_ref(peer_pvt, +1); + sip_pvt_lock(peer_pvt); + if (peer_pvt->rtp) { + struct ast_rtp_instance *peer_rtp; + + peer_rtp = peer_pvt->rtp; + ao2_ref(peer_rtp, +1); + ast_channel_unlock(peer_channel); + sip_pvt_unlock(peer_pvt); + ast_rtp_instance_set_stats_vars(peer_channel, peer_rtp); + ao2_ref(peer_rtp, -1); + ast_channel_lock(peer_channel); + sip_pvt_lock(peer_pvt); + } + sip_pvt_unlock(peer_pvt); + ao2_ref(peer_pvt, -1); + } } + ast_channel_unlock(peer_channel); } owner_relock = sip_pvt_lock_full(p); @@ -26511,10 +26523,6 @@ static int handle_request_bye(struct sip_pvt *p, struct sip_request *req) } } - if ((p->rtp || p->vrtp || p->trtp) && p->owner) { - ast_channel_stage_snapshot_done(p->owner); - } - stop_media_flows(p); /* Immediately stop RTP, VRTP and UDPTL as applicable */ stop_session_timer(p); /* Stop Session-Timer */ |