mirror of
https://github.com/asterisk/asterisk.git
synced 2025-10-26 06:26:41 +00:00
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/branches/1.8@336877 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
@@ -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");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2177,7 +2196,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;
|
||||
@@ -2591,9 +2615,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);
|
||||
|
||||
Reference in New Issue
Block a user