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

git-svn-id: https://origsvn.digium.com/svn/asterisk/certified/branches/1.8.15@375587 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/main/cel.c b/main/cel.c
index fa69b025135eeb9ce66c597518b16dc8cf6d5966..e9d05901a5a923f15632b3df3adf4fae4475041b 100644 (file)
--- a/main/cel.c
+++ b/main/cel.c
@@ -390,6 +390,14 @@ void ast_cel_check_retire_linkedid(struct ast_channel *chan)
        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_channel *ast_cel_fabricate_channel_from_event(const struct ast_event
        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,10 +471,42 @@ struct ast_channel *ast_cel_fabricate_channel_from_event(const struct ast_event
                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;
 }
 

diff --git a/main/channel.c b/main/channel.c
index c3d2bf8ae58f9724e067839120dc3b6d542582c5..7be65607031ba9bfc44e469795b0b8c5bc8d73b7 100644 (file)
--- a/main/channel.c
+++ b/main/channel.c
@@ -2499,6 +2499,13 @@ static void ast_dummy_channel_destructor(void *obj)
        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;