[Home]

Summary:ASTERISK-28422: Memory Leak in Confbridge menu
Reporter:Ted G (tgwaste)Labels:
Date Opened:2019-05-21 16:55:41Date Closed:2022-08-02 10:42:48
Priority:MinorRegression?
Status:Closed/CompleteComponents:Applications/app_confbridge
Versions:16.3.0 Frequency of
Occurrence		Constant
Related
Issues:
Environment:Debian GNU/Linux 9.8 (stretch) Using AGIAttachments:
Description:When users on my system join a conference bridge some variables are being set dynamically like so:
{noformat}
ast_setvar("CONFBRIDGE(bridge,record_file)", recfn);
ast_setvar("CONFBRIDGE(user,startmuted)", mute ? "yes" : "no");
ast_setvar("CONFBRIDGE(user,music_on_hold_class)", music ? music : "default");
ast_setvar("CONFBRIDGE(user,music_on_hold_when_empty)", music ? "yes" : "no");
{noformat}
All of this works fine, however users are quite often taken off the bridge to change these settings so every time they enter the bridge these settings are applied based on whatever configuration they are using.

My assumption is that once the channel is destroyed (upon Hangup) these settings vanish.  I am not using confbridge.conf at all.

My assumption appears false however as memory usage continues to grow day by day:
{noformat}
  54616804 bytes in      15255 allocations in file confbridge/conf_config_parser.c
{noformat}
Comments:By: Asterisk Team (asteriskteam) 2019-05-21 16:55:42.675-0500

Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution.

A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report.

Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].

Please note that once your issue enters an open state it has been accepted. As Asterisk is an open source project there is no guarantee or timeframe on when your issue will be looked into. If you need expedient resolution you will need to find and pay a suitable developer. Asking for an update on your issue will not yield any progress on it and will not result in a response. All updates are posted to the issue when they occur.
By: Benjamin Keith Ford (bford) 2019-05-22 09:10:18.396-0500

We'll need some more information, such as how are you setting up your conferences? In realtime? Can you provide steps / an example of what you are doing to produce this outcome?
By: Ted G (tgwaste) 2019-05-22 11:23:09.769-0500

Hi,
 Yes, there is a lot going on here so I will try to simplify it as much as possible. So when the call is received I setup some initial CONFBRIDGE stuff (I suspect this is stacking as well):
{noformat}
 // set up bridge config
 ast_setvar("CONFBRIDGE(bridge,internal_sample_rate)", "8000");
 ast_setvar("CONFBRIDGE(bridge,record_conference)", "yes");
 ast_setvar("CONFBRIDGE(bridge,record_file_append)", "yes");
 ast_setvar("CONFBRIDGE(bridge,record_file_timestamp)", "no");

 // set up user config
 ast_setvar("CONFBRIDGE(user,quiet)", "yes");
 ast_setvar("CONFBRIDGE(user,timeout)", "0");
 ast_setvar("CONFBRIDGE(user,jitterbuffer)", "yes");

 // set up menu config (keys)
 for (i = 0; i < strlen(digits); i++) {
   sprintf(c, "%c", digits[i]);
   k = !strcmp(c, "*") ? -1 : !strcmp(c, "#") ? -2 : atoi(c);
   sprintf(var, "CONFBRIDGE(menu,%s)", c);
   sprintf(val, "dialplan_exec(confkey,%d,1),leave_conference", k);
   ast_setvar(var, val);
 }
{noformat}
Then I basically have a loop when they join the conf
{noformat}
while (1) {
 ..
 ast_confjoin(bridge, mute, moh ? confmusic_getstation() : NULL);
 ..
}
{noformat}
then in ast_confjoin I have the stuff from my OP:
{noformat}
int ast_confjoin(int bridge, int mute, char *music)
{
 char recfn[256];

 sprintf(recfn, "%s/%d.wav", path_confrec, bridge);

 ast_setvar("CONFBRIDGE(bridge,record_file)", recfn);
 ast_setvar("CONFBRIDGE(user,startmuted)", mute ? "yes" : "no");
 ast_setvar("CONFBRIDGE(user,music_on_hold_class)", music ? music : "default");
 ast_setvar("CONFBRIDGE(user,music_on_hold_when_empty)", music ? "yes" : "no");

 sprintf(astcmd, "EXEC CONFBRIDGE %d", bridge);
 strcpy(astres, ast_sendcmd(astcmd));

 ...
}
{noformat}
I suspect an easy way to reproduce this is to make one function that has all the initial CONFBRIDGE stuff along with all the looped stuff and then do:
{noformat}
ast_setvar("CONFBRIDGE(user,timeout)", "1");
{noformat}
As that will only keep you on the bridge for 1 second so it will constantly leave and join and run the CONFBRIDGE stuff over and over.

Hopefully this helps.
By: Benjamin Keith Ford (bford) 2019-05-28 14:19:20.070-0500

Thanks for the detailed feedback.

I'm not seeing the same memory differentials on my machine. Could you try the same scenario, but without any "ast_setvar" commands to set up the conference? If the memory issue does not seem to be present after that, can you try testing with individual settings (i.e., music_on_hold_when_empty, mute, etc.) and provide results to see if you can single out a setting or if it's all CONFBRIDGE settings?
By: Ted G (tgwaste) 2019-05-28 14:28:14.121-0500

Ive done the first suggestion.  When using confbridge.conf instead (which takes many similar blocks of code to achieve what I'm doing) the memory leak goes away.

Testing individual settings is going to be very time consuming.  I would probably just go back to the confbridge.conf method instead.

Funny you're not able to reproduce this.  I was able to able to reproduce on both version 16 (current) and the official version 11.  Exact same results.

May I see your code?


By: Benjamin Keith Ford (bford) 2019-05-29 10:35:32.927-0500

Sorry for the confusion, I wasn't referring to testing it over time; I was referring to testing specific options with a loop. This would be really beneficial for us to see if it's something specific or more broad. It may just be something not being freed correctly in the code, we just need to find out where to look.

I've tested via two different approaches:
# Using SET in the dialplan multiple times, then checking memory allocations.
# SET the variable multiple times via a loop in a simple AGI script.

Neither one produced any results, unfortunately:
{noformat}
                        before      after
memory show allocations: 12621704    12478607
memory show summary:     12538387    12478184

before: 1128 bytes in          3 allocations in file app_confbridge.c
after:  1128 bytes in          3 allocations in file app_confbridge.c
before: 62828 bytes in         23 allocations in file confbridge/conf_config_parser.c
after:  62828 bytes in         23 allocations in file confbridge/conf_config_parser.c
{noformat}
Other allocations were the same in suspect areas, too.

Simple scenarios for testing:
# Dialplan (run repeatedly)
{code}
exten => 123,1,Answer()
same => n,SET(CONFBRIDGE(user,startmuted)=yes)
same => n,Hangup()
{code}
# AGI
{code}
#!/bin/bash

LOOP=0

echo "ANSWER"
while [ $LOOP -lt 100 ]; do
       echo "EXEC SET CONFBRIDGE(user,startmuted)=yes"
       let LOOP=LOOP+1
done
echo "HANGUP"
{code}

Memory allocations seemed to not change at all before and after. We'll have to try narrowing down the problem more.
By: Ted G (tgwaste) 2019-05-29 12:06:54.541-0500

Ok, so at this point I believe the leak may be in the CONFBRIDGE menu config.

If you do this test you will see the memory go up very fast. This only happened once I introduced the CONFBRIDGE(menu... settings:

{code}
[inbound]
exten => _X!,1,Answer
same => n,SET(i=1)
same => n,While($[${i} < 2])
same => n,SET(CONFBRIDGE(bridge,internal_sample_rate)=8000)
same => n,SET(CONFBRIDGE(bridge,record_conference)=no)
same => n,SET(CONFBRIDGE(bridge,record_file_append)=yes)
same => n,SET(CONFBRIDGE(bridge,record_file_timestamp)=no)
same => n,SET(CONFBRIDGE(user,quiet)=yes)
same => n,SET(CONFBRIDGE(user,timeout)=1)
same => n,SET(CONFBRIDGE(user,startmuted)=no)
same => n,SET(CONFBRIDGE(user,jitterbuffer)=yes)
same => n,SET(CONFBRIDGE(user,music_on_hold_class)=default)
same => n,SET(CONFBRIDGE(user,music_on_hold_when_empty)=no)
same => n,SET(CONFBRIDGE(menu,0)=leave_conference)
same => n,SET(CONFBRIDGE(menu,1)=leave_conference)
same => n,SET(CONFBRIDGE(menu,2)=leave_conference)
same => n,SET(CONFBRIDGE(menu,3)=leave_conference)
same => n,SET(CONFBRIDGE(menu,4)=leave_conference)
same => n,SET(CONFBRIDGE(menu,5)=leave_conference)
same => n,SET(CONFBRIDGE(menu,6)=leave_conference)
same => n,SET(CONFBRIDGE(menu,7)=leave_conference)
same => n,SET(CONFBRIDGE(menu,8)=leave_conference)
same => n,SET(CONFBRIDGE(menu,9)=leave_conference)
same => n,SET(CONFBRIDGE(menu,0)=leave_conference)
same => n,SET(CONFBRIDGE(menu,*)=leave_conference)
same => n,SET(CONFBRIDGE(menu,#)=leave_conference)
same => n,CONFBRIDGE(69)
same => n,EndWhile
same => n,Hangup
{code}

By: Benjamin Keith Ford (bford) 2019-05-29 14:37:09.432-0500

Yep, that'll do it. Setting the menu options does seem to be the root cause. I'll go ahead and open up an issue for this. Thanks for tracking that down!
By: Ted G (tgwaste) 2019-05-29 14:50:35.740-0500

Thank you.  Whats the best way for me to track the issue to resolution so that I can download the fixed version?
By: Benjamin Keith Ford (bford) 2019-05-29 15:37:55.846-0500

There's no definite timeline unfortunately, as there are other issues in the queue that might be more pressing, but all updates will be shown on this public issue once a developer begins work on it and moves it through the review process.
By: Ted G (tgwaste) 2019-12-09 20:10:06.476-0600

Hello,
 Its been roughly 6 months now since opening this ticket and was just wondering if there has been any progress?

Thank you!

By: Joshua C. Colp (jcolp) 2019-12-10 05:23:01.429-0600

The last comment from [~bford] still applies here. There's been no posted updates, so there are no updates.
By: Ted G (tgwaste) 2019-12-10 10:38:38.221-0600

I would think memory leaks would have a least a small amount of priority.  ¯\_(ツ)_/¯

By: Joshua C. Colp (jcolp) 2019-12-10 11:06:25.860-0600

They do and it did get bumped a bit. That doesn't raise it to the top of the queue for Sangoma developers though, as the over all impact and usage pattern is also taken into account. It's not common for people to dynamically do their menu from the dialplan. Most of the user base just uses the .conf files and the defaults. There's also nothing preventing someone in the community from taking on this issue if they'd like or are experiencing it.
By: Sean Bright (seanbright) 2019-12-11 11:28:47.035-0600

I don't think there is an actual leak here. DTMF sequences can have multiple actions associated with them. In {{confbridge.conf}} you would do something like:

{noformat}
7=playback(foo),playback(bar),leave_conference
{noformat}

Calling the dialplan function as you are, you are just adding {{leave_conference}} actions to the end of the action list for the given DTMF sequence. To "fix" this, you could clear the menu before making any changes to it:

{noformat}
same => n,SET(CONFBRIDGE(menu,clear)=)
{noformat}

I don't know that any Asterisk change is necessary here.
By: Ted G (tgwaste) 2019-12-11 13:15:49.616-0600

Sean,
 Thanks, I will try that however I would imagine a subsequent call to a key would overwrite the previous.

7,one,two
7,three,four

I would not expect 7 to now be 7,one,two,three,four i would expect three,four to overwrite one,two


By: Sean Bright (seanbright) 2019-12-11 14:01:11.848-0600

bq. I would imagine a subsequent call to a key would overwrite the previous.

That is not how it works, so you should use {{clear}} if you would like to emulate that behavior.
By: Ted G (tgwaste) 2019-12-11 14:09:03.187-0600

Clear seems to work, however bridge and user have to be cleared as well.

I simply don't agree with how this works.  Ive never seen an environment where setting something results in an append instead of an overwrite.  But thats my opinion.

There is still a small leak though as a hangup does not completely free all that confbridge/conf_config_parser.c is using although its very negligible.

By: Ted G (tgwaste) 2019-12-11 18:36:03.395-0600

So im not convinced this is the answer.  Here is my new test.  If you test this you will clearly see the memory usage continue to rise during each iteration.  

{code}
;
; conf mem leak test
;

[inbound]
exten => _X!,1,Answer
same => n,SET(i=1)
same => n,While($[${i} < 2])
same => n,SET(CONFBRIDGE(bridge,clear)=)
same => n,SET(CONFBRIDGE(user,clear)=)
same => n,SET(CONFBRIDGE(menu,clear)=)
same => n,SET(CONFBRIDGE(bridge,internal_sample_rate)=8000)
same => n,SET(CONFBRIDGE(bridge,record_conference)=no)
same => n,SET(CONFBRIDGE(bridge,record_file_append)=yes)
same => n,SET(CONFBRIDGE(bridge,record_file_timestamp)=no)
same => n,SET(CONFBRIDGE(user,quiet)=yes)
same => n,SET(CONFBRIDGE(user,timeout)=1)
same => n,SET(CONFBRIDGE(user,startmuted)=no)
same => n,SET(CONFBRIDGE(user,jitterbuffer)=yes)
same => n,SET(CONFBRIDGE(user,music_on_hold_class)=default)
same => n,SET(CONFBRIDGE(user,music_on_hold_when_empty)=no)
same => n,SET(CONFBRIDGE(menu,0)=leave_conference)
same => n,SET(CONFBRIDGE(menu,1)=leave_conference)
same => n,SET(CONFBRIDGE(menu,2)=leave_conference)
same => n,SET(CONFBRIDGE(menu,3)=leave_conference)
same => n,SET(CONFBRIDGE(menu,4)=leave_conference)
same => n,SET(CONFBRIDGE(menu,5)=leave_conference)
same => n,SET(CONFBRIDGE(menu,6)=leave_conference)
same => n,SET(CONFBRIDGE(menu,7)=leave_conference)
same => n,SET(CONFBRIDGE(menu,8)=leave_conference)
same => n,SET(CONFBRIDGE(menu,9)=leave_conference)
same => n,SET(CONFBRIDGE(menu,0)=leave_conference)
same => n,SET(CONFBRIDGE(menu,*)=leave_conference)
same => n,SET(CONFBRIDGE(menu,#)=leave_conference)
same => n,CONFBRIDGE(69)
same => n,SET(CONFBRIDGE(bridge,clear)=)
same => n,SET(CONFBRIDGE(user,clear)=)
same => n,SET(CONFBRIDGE(menu,clear)=)
same => n,EndWhile
same => n,Hangup
{code}
By: Sean Bright (seanbright) 2019-12-11 18:52:16.484-0600

Thanks for the updated example.

I don’t quite understand what the goal of your example code is but a potential workaround would be to not set these values over and over again in an endless loop. You could set them once before entering the infinite loop which should at least decrease how much memory is being used. That might help hold you over until someone has time to look at the code more closely.
By: Ted G (tgwaste) 2019-12-12 10:41:07.673-0600

I am currently setting the digits in confbridge.conf as a hold over.  I tried the setting it once thing but that still causes me issues.  I have hundreds of callers constantly moving in and out of bridges so over the minutes, hours, days... this adds up to a lot of memory.   I will continue to use confbridge.conf for now but my goal was always to avoid having to maintain asterisk config files and do everything dynamically in code.  The example code I provided was to demonstrate the leak and show (in just a few seconds) what happens over the course of days.
By: Friendly Automation (friendly-automation) 2022-08-02 10:42:50.933-0500

Change 18929 merged by Friendly Automation:
app_confbridge: Fix memory leak on updated menu options.

[https://gerrit.asterisk.org/c/asterisk/+/18929|https://gerrit.asterisk.org/c/asterisk/+/18929]
By: Friendly Automation (friendly-automation) 2022-08-08 05:19:59.097-0500

Change 18928 merged by George Joseph:
app_confbridge: Fix memory leak on updated menu options.

[https://gerrit.asterisk.org/c/asterisk/+/18928|https://gerrit.asterisk.org/c/asterisk/+/18928]
By: Friendly Automation (friendly-automation) 2022-08-08 05:20:03.644-0500

Change 18930 merged by George Joseph:
app_confbridge: Fix memory leak on updated menu options.

[https://gerrit.asterisk.org/c/asterisk/+/18930|https://gerrit.asterisk.org/c/asterisk/+/18930]
By: Friendly Automation (friendly-automation) 2022-08-08 05:20:06.123-0500

Change 18887 merged by George Joseph:
app_confbridge: Fix memory leak on updated menu options.

[https://gerrit.asterisk.org/c/asterisk/+/18887|https://gerrit.asterisk.org/c/asterisk/+/18887]
By: Friendly Automation (friendly-automation) 2022-08-08 05:20:09.335-0500

Change 18927 merged by George Joseph:
app_confbridge: Fix memory leak on updated menu options.

[https://gerrit.asterisk.org/c/asterisk/+/18927|https://gerrit.asterisk.org/c/asterisk/+/18927]