Summary:	ASTERISK-22409: Local channels in a ConfBridge w/ jitterbuffer=yes leak ast_frame's after masquerade
Reporter:	Corey Farrell (coreyfarrell)	Labels:
Date Opened:	2013-08-26 22:26:33	Date Closed:	2016-09-23 14:15:29
Priority:	Major	Regression?
Status:	Closed/Complete	Components:	Applications/app_confbridge Channels/chan_local Core/Jitterbuffer Core/ManagerInterface
Versions:	11.6.0	Frequency of Occurrence	Constant
Related Issues:	is related to ASTERISK-21144 One way audio after channels are AMI Bridged out of a ConfBridge that has jitterbuffer=yes
Environment:		Attachments:	( 0) ami-action.txt ( 1) channel-debug.patch ( 2) confbridge.conf ( 3) extensions.conf ( 4) full_log ( 5) func_jitterbuffer-hook_event_cb.patch ( 6) masquerade-set_fd.patch ( 7) valgrind-leaks
Description:	I use AMI to originate calls into MeetMe.  I've recently started testing with ConfBridge, but found a memory leak that prevents me from switching.  Using Asterisk 11. To replicate: * Use attached configuration - extensions.conf, confbridge.conf * Set dialplan global MYDEVICE as appropriate * Start asterisk in valgrind - MALLOC_DEBUG might work I haven't tried * Call the ConfBridge - don't make MYDEVICE unavailable * Using AMI send action Originate - see ami-action.txt * Ensure the destination call is answered - this causes Local to optimize away Once the Local channel is optimized away, ast_frame leaks are guaranteed: * The masquerade/optimize of Local to SIP results in a couple leaks every time. * AMI: Redirect and Hangup both result in a leak every time. * Unknown additional trigger sometimes causes huge number of leaked frames.  Leaks 200k in very short time.  Still trying to find a way to reliably reproduce this one. Changing the Originate channel to 'Local/s@test-out/n' seems to prevent the leaks.  Another strange note, ringing audio is clear but once the masquerade happens the audio quality is terrible.  Either preventing the optimize or turning off jitterbuffer seams to result in good quality audio. This issue might be related to [ASTERISK-21144].
Comments:	By: Corey Farrell (coreyfarrell) 2013-08-26 22:59:37.282-0500 Sorry valgrind-leaks isn't the exact same run as full_log, but the valgrind results are always the same. channel-debug.patch was used to test a theory.  I set current->src to see if that caused a direct or indirect leak.  Since it caused an indirect leak, 'current' is directly leaked.  Not sure if this means 'current' always leaks or only in this special case. By: Corey Farrell (coreyfarrell) 2013-10-29 17:28:22.432-0500 Attached is a first attempt to fix these leaks. One concern about ast_frfree(jbframe), what if jbframe == frame?  I'm not sure if it's possible here, and I have no idea if jbframe should be free'd in that case.  freeing jbframe seems to resolve the very large memory leak. The ast_frfree of the non-static AST_FRAME_NULL resolves the leak caused by soft hangup. I first tested with ast_debug(3, "leak frame\n"); in place of the ast_frfree's, the leak count matched between asterisk full log and valgrind.  Then I tested with debug and free, verified that the newly added ast_frfree's ran, and resolved the leaks without causing any valgrind memory access error's.  All testing was done on latest branches/11. This patch does not resolve the poor audio quality caused by func_jitterbuffer.so, but not leaking is a good start. By: Corey Farrell (coreyfarrell) 2014-02-07 17:45:03.888-0600 masquerade-set_fd.patch seems to resolve the audio quality issue.  With some extra debugging I found that the channel fd AST_JITTERBUFFER_FD (6) was set on Local/s@test-out-00000000;1, but replaced with -1 from SIP/20103-00000001.  This did not result in an FD leak since the FD is destroyed by the framehook. {noformat} [2014-02-07 17:55:27.709] DEBUG[22640][C-00000000] channel.c: Actually Masquerading SIP/20103-00000001(6) into the structure of Local/s@test-out-00000000;1(6) [2014-02-07 17:55:27.709] DEBUG[22640][C-00000000] channel.c: Masq fd[0]: 26 (was -1) [2014-02-07 17:55:27.709] DEBUG[22640][C-00000000] channel.c: Masq fd[1]: 27 (was -1) [2014-02-07 17:55:27.709] DEBUG[22640][C-00000000] channel.c: Masq fd[2]: -1 (was -1) [2014-02-07 17:55:27.709] DEBUG[22640][C-00000000] channel.c: Masq fd[3]: -1 (was -1) [2014-02-07 17:55:27.709] DEBUG[22640][C-00000000] channel.c: Masq fd[4]: -1 (was -1) [2014-02-07 17:55:27.709] DEBUG[22640][C-00000000] channel.c: Masq fd[5]: -1 (was -1) [2014-02-07 17:55:27.709] DEBUG[22640][C-00000000] channel.c: Masq fd[6]: -1 (was 25) [2014-02-07 17:55:27.709] DEBUG[22640][C-00000000] channel.c: Masq fd[8]: -1 (was -1) [2014-02-07 17:55:27.709] DEBUG[22640][C-00000000] channel.c: Masq fd[9]: 28 (was 17) [2014-02-07 17:55:27.709] DEBUG[22640][C-00000000] channel.c: Masq fd[10]: 29 (was 18) {noformat} By: Richard Mudgett (rmudgett) 2014-02-07 18:46:20.606-0600 Which patches are needed to fix the issue? [^func_jitterbuffer-hook_event_cb.patch] [^masquerade-set_fd.patch] Or both? By: Corey Farrell (coreyfarrell) 2014-02-07 18:57:00.594-0600 Both. The func_jitterbuffer patch fixes 2 possible memory leaks in the framehook.  As I mentioned before I'm concerned about the possibility of frame==jbframe, but I never ran into that in testing. The masquerade patch makes sure the jitterbuffer timer isn't removed from the channel (replaced by -1).