[asterisk-dev] [Code Review] Fix crashes in ast_rtcp_write()

[Russell Bryant]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1444/
-----------------------------------------------------------

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/b000162d/attachment.htm>