[asterisk-commits] russell: trunk r336879 - in /trunk: ./ res/res_rtp_asterisk.c
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Mon Sep 19 20:11:23 CDT 2011
Author: russell
Date: Mon Sep 19 20:11:18 2011
New Revision: 336879
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=336879
Log:
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/
........
................
Modified:
trunk/ (props changed)
trunk/res/res_rtp_asterisk.c
Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-10-merged' - no diff available.
Modified: trunk/res/res_rtp_asterisk.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/res_rtp_asterisk.c?view=diff&rev=336879&r1=336878&r2=336879
==============================================================================
--- trunk/res/res_rtp_asterisk.c (original)
+++ trunk/res/res_rtp_asterisk.c Mon Sep 19 20:11:18 2011
@@ -522,7 +522,11 @@
/* 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
/* 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 @@
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);
More information about the asterisk-commits
mailing list