diff options
author | Russell Bryant <russell@russellbryant.com> | 2011-09-20 01:11:18 +0000 |
---|---|---|
committer | Russell Bryant <russell@russellbryant.com> | 2011-09-20 01:11:18 +0000 |
commit | 14d3f891e0cd8985ce7305dff62d134edfabd02f (patch) | |
tree | b729442d903e535e20a73a7212ca93e2bb964878 /res | |
parent | 098efb66412cd1a262c8a95427948008113cd43e (diff) |
Merged revisions 336878 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/10
................
r336878 | russell | 2011-09-19 20:03:55 -0500 (Mon, 19 Sep 2011) | 43 lines
Merged revisions 336877 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8
........
r336877 | russell | 2011-09-19 19:56:20 -0500 (Mon, 19 Sep 2011) | 36 lines
Fix crashes in ast_rtcp_write().
This patch addresses crashes related to RTCP handling. The backtraces just
show a crash in ast_rtcp_write() where it appears that the RTP instance is no
longer valid. There is a race condition with scheduled RTCP transmissions and
the destruction of the RTP instance. This patch utilizes the fact that
ast_rtp_instance is a reference counted object and ensures that it will not get
destroyed while a reference is still around due to scheduled RTCP
transmissions.
RTCP transmissions are scheduled and executed from the chan_sip scheduler
context. This scheduler context is processed in the SIP monitor thread. The
destruction of an RTP instance occurs when the associated sip_pvt gets
destroyed (which happens when the sip_pvt reference count reaches 0). However,
the SIP monitor thread is not the only thread that can cause a sip_pvt to get
destroyed. The sip_hangup function, executed from a channel thread, also
decrements the reference count on a sip_pvt and could cause it to get
destroyed.
While this is being changed anyway, the patch also removes calling
ast_sched_del() from within the RTCP scheduler callback. It's not helpful.
Simply returning 0 prevents the callback from being rescheduled.
(closes issue ASTERISK-18570)
Related issues that look like they are the same problem:
(issue ASTERISK-17560)
(issue ASTERISK-15406)
(issue ASTERISK-15257)
(issue ASTERISK-13334)
(issue ASTERISK-9977)
(issue ASTERISK-9716)
Review: https://reviewboard.asterisk.org/r/1444/
........
................
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@336879 65c4cc65-6c06-0410-ace0-fbb531ad65f3
Diffstat (limited to 'res')
-rw-r--r-- | res/res_rtp_asterisk.c | 55 |
1 files changed, 42 insertions, 13 deletions
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c index 06af4832d..f4e8299bd 100644 --- a/res/res_rtp_asterisk.c +++ b/res/res_rtp_asterisk.c @@ -522,7 +522,11 @@ static int ast_rtp_destroy(struct ast_rtp_instance *instance) /* Destroy RTCP if it was being used */ if (rtp->rtcp) { - AST_SCHED_DEL(rtp->sched, rtp->rtcp->schedid); + /* + * It is not possible for there to be an active RTCP scheduler + * entry at this point since it holds a reference to the + * RTP instance while it's active. + */ close(rtp->rtcp->s); ast_free(rtp->rtcp); } @@ -830,8 +834,9 @@ static int ast_rtcp_write_rr(struct ast_rtp_instance *instance) return 0; if (ast_sockaddr_isnull(&rtp->rtcp->them)) { - ast_log(LOG_ERROR, "RTCP RR transmission error, rtcp halted\n"); - AST_SCHED_DEL(rtp->sched, rtp->rtcp->schedid); + /* + * RTCP was stopped. + */ return 0; } @@ -886,8 +891,6 @@ static int ast_rtcp_write_rr(struct ast_rtp_instance *instance) if (res < 0) { ast_log(LOG_ERROR, "RTCP RR transmission error, rtcp halted: %s\n",strerror(errno)); - /* Remove the scheduler */ - AST_SCHED_DEL(rtp->sched, rtp->rtcp->schedid); return 0; } @@ -932,8 +935,9 @@ static int ast_rtcp_write_sr(struct ast_rtp_instance *instance) return 0; if (ast_sockaddr_isnull(&rtp->rtcp->them)) { /* This'll stop rtcp for this rtp session */ - ast_verbose("RTCP SR transmission error, rtcp halted\n"); - AST_SCHED_DEL(rtp->sched, rtp->rtcp->schedid); + /* + * RTCP was stopped. + */ return 0; } @@ -985,7 +989,6 @@ static int ast_rtcp_write_sr(struct ast_rtp_instance *instance) ast_log(LOG_ERROR, "RTCP SR transmission error to %s, rtcp halted %s\n", ast_sockaddr_stringify(&rtp->rtcp->them), strerror(errno)); - AST_SCHED_DEL(rtp->sched, rtp->rtcp->schedid); return 0; } @@ -1044,13 +1047,24 @@ static int ast_rtcp_write(const void *data) struct ast_rtp *rtp = ast_rtp_instance_get_data(instance); int res; - if (!rtp || !rtp->rtcp) + if (!rtp || !rtp->rtcp || rtp->rtcp->schedid == -1) { + ao2_ref(instance, -1); return 0; + } - if (rtp->txcount > rtp->rtcp->lastsrtxcount) + if (rtp->txcount > rtp->rtcp->lastsrtxcount) { res = ast_rtcp_write_sr(instance); - else + } else { res = ast_rtcp_write_rr(instance); + } + + if (!res) { + /* + * Not being rescheduled. + */ + ao2_ref(instance, -1); + rtp->rtcp->schedid = -1; + } return res; } @@ -1162,7 +1176,12 @@ static int ast_rtp_raw_write(struct ast_rtp_instance *instance, struct ast_frame if (rtp->rtcp && rtp->rtcp->schedid < 1) { ast_debug(1, "Starting RTCP transmission on RTP instance '%p'\n", instance); + ao2_ref(instance, +1); rtp->rtcp->schedid = ast_sched_add(rtp->sched, ast_rtcp_calc_interval(rtp), ast_rtcp_write, instance); + if (rtp->rtcp->schedid < 0) { + ao2_ref(instance, -1); + ast_log(LOG_WARNING, "scheduling RTCP transmission failed.\n"); + } } } @@ -2180,7 +2199,12 @@ static struct ast_frame *ast_rtp_read(struct ast_rtp_instance *instance, int rtc /* Do not schedule RR if RTCP isn't run */ if (rtp->rtcp && !ast_sockaddr_isnull(&rtp->rtcp->them) && rtp->rtcp->schedid < 1) { /* Schedule transmission of Receiver Report */ + ao2_ref(instance, +1); rtp->rtcp->schedid = ast_sched_add(rtp->sched, ast_rtcp_calc_interval(rtp), ast_rtcp_write, instance); + if (rtp->rtcp->schedid < 0) { + ao2_ref(instance, -1); + ast_log(LOG_WARNING, "scheduling RTCP transmission failed.\n"); + } } if ((int)rtp->lastrxseqno - (int)seqno > 100) /* if so it would indicate that the sender cycled; allow for misordering */ rtp->cycles += RTP_SEQ_MOD; @@ -2592,9 +2616,14 @@ static void ast_rtp_stop(struct ast_rtp_instance *instance) struct ast_rtp *rtp = ast_rtp_instance_get_data(instance); struct ast_sockaddr addr = { {0,} }; - if (rtp->rtcp) { - AST_SCHED_DEL(rtp->sched, rtp->rtcp->schedid); + if (rtp->rtcp && rtp->rtcp->schedid > 0) { + if (!ast_sched_del(rtp->sched, rtp->rtcp->schedid)) { + /* successfully cancelled scheduler entry. */ + ao2_ref(instance, -1); + } + rtp->rtcp->schedid = -1; } + if (rtp->red) { AST_SCHED_DEL(rtp->sched, rtp->red->schedid); free(rtp->red); |