[asterisk-dev] [Code Review] Fix RTP reference leaks during certain SIP transfer situations

[Mark Michelson]

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

Review request for Asterisk Developers.


Summary
-------

Asterisk has an issue where under certain circumstances, it may  not properly shut down its RTP instance. The result of this is both a memory leak as well as a port leak. Eventually, the system will run out of ports in the RTP port range and will be pretty much unusable. The following conditions were necessary in order for this to occur:

1. RTP traffic had to be bridged through Asterisk, and not through the RTP engine's local or remote bridges. The reason this was necessary was so that Asterisk would send RTCP sender and receiver reports.
2. A transfer (blind or attended) had to be initiated using a REFER but without first sending a reINVITE to place a call on hold.

Circumstance (2) caused the response handler for our outbound NOTIFYs to take a different code path that would immediately cause the destruction of the SIP dialog without properly notifying the RTP engine to stop its workings. While certainly a bug, this wouldn't actually cause an issue unless Asterisk were sending RTCP. This is because Asterisk schedules RTCP transmission using the scheduler, which constantly has a reference to the RTP instance.

This patch does two things.
1. It takes the two sections for handling NOTIFY responses and puts it all inside handle_response_notify(). If any differences were present, I favored the inline code from handle_response() over what had previously been in handle_response_notify().
2. I added a new call to stop_media_flows() in the case where we have deferred a BYE due to a transfer. If we determine they are not going to send us a BYE like they are supposed to, we need to stop media so that the RTP will have its refcount decremented properly.

If anything is unclear, I can explain it in more detail.


This addresses bug ASTERISK-19192.
    https://issues.asterisk.org/jira/browse/ASTERISK-19192


Diffs
-----

  /branches/1.8/channels/chan_sip.c 351501 

Diff: https://reviewboard.asterisk.org/r/1681/diff


Testing
-------

I've tested transfers both from a Polycom phone and PJSUA. The Polycom sends a hold reINVITE before sending the REFER, and PJSUA does not. This allowed me to be sure that the updated code performed the same in both scenarios. I wasn't able to test the new addition of stop_media_flows() because all the UAs I test against are well behaved and send a BYE to us after sending a 200 OK for the NOTIFY we send.

A patch for 1.8.8.1 has been uploaded to ASTERISK-19192, but I have no feedback yet since it was only just uploaded.


Thanks,

Mark

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120119/6f06189c/attachment.htm>