[asterisk-commits] kpfleming: trunk r370211 - in /trunk: ./ main/cel.c

[SVN commits to the Asterisk project]

Author: kpfleming
Date: Wed Jul 18 14:18:40 2012
New Revision: 370211

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=370211
Log:
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)
Review: https://reviewboard.asterisk.org/r/2053/
........

Merged revisions 370205 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 370206 from http://svn.asterisk.org/svn/asterisk/branches/10

Modified:
    trunk/   (props changed)
    trunk/main/cel.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-10-merged' - no diff available.

Modified: trunk/main/cel.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/cel.c?view=diff&rev=370211&r1=370210&r2=370211
==============================================================================
--- trunk/main/cel.c (original)
+++ trunk/main/cel.c Wed Jul 18 14:18:40 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,
+};
+
 struct ast_channel *ast_cel_fabricate_channel_from_event(const struct ast_event *event)
 {
 	struct varshead *headp;
@@ -400,6 +408,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())) {
@@ -467,9 +477,41 @@
 		AST_LIST_INSERT_HEAD(headp, newvariable, entries);
 	}
 
-	ast_channel_appl_set(tchan, ast_strdup(record.application_name));
-	ast_channel_data_set(tchan, ast_strdup(record.application_data));
 	ast_channel_amaflags_set(tchan, 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;
+	}
+
+	ast_channel_appl_set(tchan, app_data);
+	ast_channel_data_set(tchan, app_data + strlen(record.application_name) + 1);
+
+	strcpy((char *) ast_channel_appl(tchan), record.application_name);
+	strcpy((char *) ast_channel_data(tchan), record.application_data);
+	datastore->data = app_data;
+	ast_channel_datastore_add(tchan, datastore);
 
 	return tchan;
 }