[asterisk-dev] [Code Review] Fix crashes in ast_rtcp_write()
David Vossel
reviewboard at asterisk.org
Mon Sep 19 10:06:57 CDT 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1444/#review4362
-----------------------------------------------------------
Ship it!
A 5 second delayed destruction isn't that big of a deal. I'm fine with this and completely agree that introducing any sort of synchronization here would just make this more complicated than is necessary. Great Work!
- David
On Sept. 18, 2011, 9:20 p.m., Russell Bryant wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1444/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2011, 9:20 p.m.)
>
>
> Review request for Asterisk Developers.
>
>
> Summary
> -------
>
> I opened ASTERISK-18570 for this issue. The rest of the issues are ones that I have found so far that appear to be the same problem.
>
> 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.
>
>
> This addresses bugs ASTERISK-13334, ASTERISK-15257, ASTERISK-15406, ASTERISK-17560, ASTERISK-18570, ASTERISK-9716, and ASTERISK-9977.
> https://issues.asterisk.org/jira/browse/ASTERISK-13334
> https://issues.asterisk.org/jira/browse/ASTERISK-15257
> https://issues.asterisk.org/jira/browse/ASTERISK-15406
> https://issues.asterisk.org/jira/browse/ASTERISK-17560
> https://issues.asterisk.org/jira/browse/ASTERISK-18570
> https://issues.asterisk.org/jira/browse/ASTERISK-9716
> https://issues.asterisk.org/jira/browse/ASTERISK-9977
>
>
> Diffs
> -----
>
> /branches/1.8/res/res_rtp_asterisk.c 335789
>
> Diff: https://reviewboard.asterisk.org/r/1444/diff
>
>
> Testing
> -------
>
> This patch has been applied to a test environment with a couple of servers running Asterisk 1.8.7.0-rc1. The servers have processed over 1 million calls without hitting this crash.
>
>
> Thanks,
>
> Russell
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110919/a33c35e1/attachment.htm>
More information about the asterisk-dev
mailing list