Summary:	ASTERISK-29184: memory leak manager.c:purge_old_stuff not scheduled when manager.conf enabled=no
Reporter:	Diederik de Groot (dkdegroot)	Labels:	ami core manager memory-leak
Date Opened:	2020-11-28 05:56:00.000-0600	Date Closed:
Priority:	Major	Regression?	Yes
Status:	Open/New	Components:	Core/ManagerInterface
Versions:	GIT 16.15.0 17.8.1 18.0.1	Frequency of Occurrence	Constant
Related Issues:
Environment:	Any	Attachments:	( 0) eventq.log
Description:	With enabled=off in manager.conf the `purge_old_stuff` function from main/manager.c is never scheduled and doesn't run. Meaning that any time the append_event is called (for example through __manager_event_sessions_va / manager_event) new memory is allocated for the event, it is inserted in the `eventqent`, but never released. Causing a non negligible memory consumption over time. Enabling manager/ami and reloading resolves the memory leak. Possible/Potential Solutions: - append_event could bail out early and not allocate memory. - purge_old_stuff could be scheduled even if ami/manager is not enabled. - purge_old_stuff could be scheduled on some other monitoring thread.
Comments:	By: Asterisk Team (asteriskteam) 2020-11-28 05:56:01.903-0600 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. Please note that log messages and other files should not be sent to the Sangoma Asterisk Team unless explicitly asked for. All files should be placed on this issue in a sanitized fashion as needed. 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. Please note that by submitting data, code, or documentation to Sangoma through JIRA, you accept the Terms of Use present at [https://www.asterisk.org/terms-of-use/|https://www.asterisk.org/terms-of-use/]. By: Sean Bright (seanbright) 2020-11-30 10:42:13.800-0600 Hi [~dkdegroot] - can you attach the output of {{manager show eventq}} please? By: Diederik de Groot (dkdegroot) 2020-12-03 04:53:46.042-0600 manager show eventq output By: Sean Bright (seanbright) 2020-12-03 08:06:43.945-0600 Are you running a patched version of Asterisk? I ask because, for example, the string {{SCCPDevice}} does not show up in the Asterisk 18 source code but it is in your event queue output. By: Diederik de Groot (dkdegroot) 2020-12-03 10:46:13.934-0600 Not a patches version of asterisk, just running chan-sccp thirdparty channel driver. But i think i can reproduce the same issue with other channel drivers. I will try to make another report tomorrow. By: Asterisk Team (asteriskteam) 2020-12-17 12:00:03.137-0600 Suspended due to lack of activity. This issue will be automatically re-opened if the reporter posts a comment. If you are not the reporter and would like this re-opened please create a new issue instead. If the new issue is related to this one a link will be created during the triage process. Further information on issue tracker usage can be found in the Asterisk Issue Guidlines [1]. [1] https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines By: Diederik de Groot (dkdegroot) 2020-12-18 01:02:04.236-0600 I think i have found the cause but was not able to find a solution. The "any_manager_listeners" macro is used to check if there are any listeners and "__ast_manager_event_multichan" skips adding events if no one is listening. However chan-sccp uses a manager_hook and there is actively listening even if manager.conf is disabled. But with manager disabled and a manager_hook registered the purge method ought to be running to cleanup. Or there would have to be a way to check if manager is enabled from an external module like chan-sccp to prevent it from registering the manager_hook. The any_manager_listeners macro includes a check to see if there are manager_hooks registed. The purge is not active when manager_hooks are but manager is not enabled. By: Asterisk Team (asteriskteam) 2020-12-18 01:02:04.925-0600 This issue has been reopened as a result of your commenting on it as the reporter. It will be triaged once again as applicable. By: Sean Bright (seanbright) 2020-12-18 08:45:56.307-0600 bq. Or there would have to be a way to check if manager is enabled from an external module like chan-sccp to prevent it from registering the manager_hook. [{{check_manager_enabled()}} in {{manager.h}}|https://github.com/asterisk/asterisk/blob/9c3b57822a325cd81597efb9b9489197470e3091/include/asterisk/manager.h#L117-L118] Although we should probably be doing more to protect against this kind of thing in the manager code itself. By: Diederik de Groot (dkdegroot) 2020-12-18 09:11:27.955-0600 Thank Sean for looking into the issue. I know check_manager_enabled() is in the manager.h and it would be usable for modules compiled during asterisk build, but it is not exported so not available to external modules (I tried to use it last week but it could not find a reference to the implementation during module loading). It would help if check_manager_enabled() would be added to the exported symbols to make it available. Instead adding a specific entry to asterisk.exports for check_manager_enabled() it might be easier to rename/wrap it to "ast_check_manager_enabled()" or "ast_is_manager_enabled()". When making this change it would be a good idea to also include "check_webmanager_enabled()" for a similar procedure at the same time. Sorry for not mentioning my trial of check_manager_enabled in previous comments. By: Sean Bright (seanbright) 2020-12-18 09:38:46.987-0600 [~dkdegroot], I've put up [a review with your suggestions|https://gerrit.asterisk.org/c/asterisk/+/15226]. It does not resolve this issue but at least you can now check the manager status from third-party modules. By: Diederik de Groot (dkdegroot) 2020-12-18 12:11:31.616-0600 Thanks you very much, the impact for third party module should be 0 as they would not have been able to use the previous version. I will adapt chan-sccp to use the new functionality and fix it from this side. Thanks for helping out, fixing the memory leak that resulted from it. By: Diederik de Groot (dkdegroot) 2020-12-19 04:06:43.563-0600 @Sean Bright, The submmitted patch looks fine. I adapted the chan-sccp code to use the new "ast_manager_check_enabled" function. Thank you. Do you think this change will/could be backported to earlier asterisk versions ? By: Joshua C. Colp (jcolp) 2020-12-19 04:43:13.013-0600 From a project perspective changes only go into current supported bug fix branches (currently 16 and 18), which will then appear in the next releases of those branches. By: Diederik de Groot (dkdegroot) 2020-12-19 04:58:25.997-0600 ast-16 is what i was hoping for ! Thanks for the heads up. The ast_manager_check_enabled is a little inflexible and simple. But i do not think more is need in regard to this issue report. In most asterisk server setup's manager.conf is configured early on in the project and does not undergo changes often. Meaning manager_enabled is either on or off for the life time of the setup/instance.