[asterisk-commits] mjordan: branch certified-1.8.15 r375587 - in /certified/branches/1.8.15: ./ ...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Fri Nov 2 10:23:19 CDT 2012
Author: mjordan
Date: Fri Nov 2 10:23:12 2012
New Revision: 375587
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=375587
Log:
Multiple revisions 370205,370273,370360
........
r370205 | kpfleming | 2012-07-18 14:12:03 -0500 (Wed, 18 Jul 2012) | 18 lines
Resolve severe memory leak in CEL logging modules.
A customer reported a significant memory leak using Asterisk 1.8. They
have tracked it down to ast_cel_fabricate_channel_from_event() in
main/cel.c, which is called by both in-tree CEL logging modules
(cel_custom.c and cel_sqlite3_custom.c) for each and every CEL event
that they log.
The cause was an incorrect assumption about how data attached to an
ast_channel would be handled when the channel is destroyed; the data
is now stored in a datastore attached to the channel, which is
destroyed along with the channel at the proper time.
(closes issue AST-916)
Reported by: Thomas Arimont
Review: https://reviewboard.asterisk.org/r/2053/
........
r370273 | mjordan | 2012-07-19 17:00:14 -0500 (Thu, 19 Jul 2012) | 14 lines
Fix compilation error when MALLOC_DEBUG is enabled
To fix a memory leak in CEL, a channel datastore was introduced whose
destruction function pointer was pointed to the ast_free macro. Without
MALLOC_DEBUG enabled this compiles as fine, as ast_free is defined as free.
With MALLOC_DEBUG enabled, however, ast_free takes on a definition from a
different place then utils.h, and became undefined. This patch resolves this
by using a reference to ast_free_ptr. When MALLOC_DEBUG is enabled, this
calls ast_free; when MALLOC_DEBUG is not enabled, this is defined to be
ast_free, which is defined to be free.
(issue AST-916)
Reported by: Thomas Arimont
........
r370360 | kpfleming | 2012-07-23 09:41:03 -0500 (Mon, 23 Jul 2012) | 10 lines
Free any datastores attached to dummy channels.
Revision 370205 added the use of a datastore attached to a dummy channel to
resolve a memory leak, but ast_dummy_channel_destructor() in this branch did
not free datastores, resulting in a continued (but slightly smaller) memory
leak. This patch backports the change to free said datastores from the Asterisk
trunk.
(related to issue AST-916)
........
Merged revisions 370205,370273,370360 from http://svn.asterisk.org/svn/asterisk/branches/1.8
Modified:
certified/branches/1.8.15/ (props changed)
certified/branches/1.8.15/main/cel.c
certified/branches/1.8.15/main/channel.c
Propchange: certified/branches/1.8.15/
------------------------------------------------------------------------------
Binary property 'branch-1.8-merged' - no diff available.
Modified: certified/branches/1.8.15/main/cel.c
URL: http://svnview.digium.com/svn/asterisk/certified/branches/1.8.15/main/cel.c?view=diff&rev=375587&r1=375586&r2=375587
==============================================================================
--- certified/branches/1.8.15/main/cel.c (original)
+++ certified/branches/1.8.15/main/cel.c Fri Nov 2 10:23:12 2012
@@ -390,6 +390,14 @@
ao2_ref(lid, -1);
}
+/* Note that no 'chan_fixup' function is provided for this datastore type,
+ * because the channels that will use it will never be involved in masquerades.
+ */
+static const struct ast_datastore_info fabricated_channel_datastore = {
+ .type = "CEL fabricated channel",
+ .destroy = ast_free_ptr,
+};
+
struct ast_channel *ast_cel_fabricate_channel_from_event(const struct ast_event *event)
{
struct varshead *headp;
@@ -399,6 +407,8 @@
struct ast_cel_event_record record = {
.version = AST_CEL_EVENT_RECORD_VERSION,
};
+ struct ast_datastore *datastore;
+ char *app_data;
/* do not call ast_channel_alloc because this is not really a real channel */
if (!(tchan = ast_dummy_channel_alloc())) {
@@ -461,9 +471,41 @@
AST_LIST_INSERT_HEAD(headp, newvariable, entries);
}
- tchan->appl = ast_strdup(record.application_name);
- tchan->data = ast_strdup(record.application_data);
tchan->amaflags = record.amaflag;
+
+ /* We need to store an 'application name' and 'application
+ * data' on the channel for logging purposes, but the channel
+ * structure only provides a place to store pointers, and it
+ * expects these pointers to be pointing to data that does not
+ * need to be freed. This means that the channel's destructor
+ * does not attempt to free any storage that these pointers
+ * point to. However, we can't provide data in that form directly for
+ * these structure members. In order to ensure that these data
+ * elements have a lifetime that matches the channel's
+ * lifetime, we'll put them in a datastore attached to the
+ * channel, and set's the channel's pointers to point into the
+ * datastore. The datastore will then be automatically destroyed
+ * when the channel is destroyed.
+ */
+
+ if (!(datastore = ast_datastore_alloc(&fabricated_channel_datastore, NULL))) {
+ ast_channel_unref(tchan);
+ return NULL;
+ }
+
+ if (!(app_data = ast_malloc(strlen(record.application_name) + strlen(record.application_data) + 2))) {
+ ast_datastore_free(datastore);
+ ast_channel_unref(tchan);
+ return NULL;
+ }
+
+ tchan->appl = app_data;
+ tchan->data = app_data + strlen(record.application_name) + 1;
+
+ strcpy((char *) tchan->appl, record.application_name);
+ strcpy((char *) tchan->data, record.application_data);
+ datastore->data = app_data;
+ ast_channel_datastore_add(tchan, datastore);
return tchan;
}
Modified: certified/branches/1.8.15/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/certified/branches/1.8.15/main/channel.c?view=diff&rev=375587&r1=375586&r2=375587
==============================================================================
--- certified/branches/1.8.15/main/channel.c (original)
+++ certified/branches/1.8.15/main/channel.c Fri Nov 2 10:23:12 2012
@@ -2499,6 +2499,13 @@
struct ast_channel *chan = obj;
struct ast_var_t *vardata;
struct varshead *headp;
+ struct ast_datastore *datastore;
+
+ /* Get rid of each of the data stores on the channel */
+ while ((datastore = AST_LIST_REMOVE_HEAD(&chan->datastores, entry))) {
+ /* Free the data store */
+ ast_datastore_free(datastore);
+ }
headp = &chan->varshead;
More information about the asterisk-commits
mailing list