[asterisk-dev] [Code Review] Adjust shitty usage of memory in app_queue

[Mark Michelson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/650/
-----------------------------------------------------------

Review request for Asterisk Developers.


Summary
-------

This review request is for the patch on issue 17081.

A user reported that he saw increasing numbers of allocations stemming
from app_queue.c when he would run the "queue show" CLI command. The
user reported that he was using approximately 40 realtime queues and
as he ran the CLI command more and more, the memory usage would shoot up.
As it turns out, there was a memory leak and a separate usage of memory
that, while not really a leak, was very irresponsible.

Both memory problems can be attributed to the function init_queue(). When
the "queue show" command is run, all realtime queues have the init_queue()
function called on the in-memory queue. The idea is to place the queue in
its default state and then overwrite options specified in the realtime backend
as we read them.

The first problem, the memory leak, had to do with the fact that the string
field for the name of the first periodic announcement file was being re-created
every time init_queue was called. This patch corrects the behavior by only
calling ast_str_create if the memory has not already been allocated. 

The other problem is a bit more complicated. The majority of the strings
in the call_queue structure were changed to use the ast_string_fields API
for 1.6.0 and beyond. init_queue resets all string fields on the queue to
their default values. Then, later in the realtime queue loading process,
these string fields are set to their configured values.

For those unfamiliar with string fields, frequent resizing of a string like
this is not what the string fields API is designed for. The result of this
constant resizing is that as the queue gets loaded, eventually space for
the string runs out and so a new memory pool, at twice the size of the
previously allocated one, is created for the string fields. The reporter
of issue 17081 wrote a script that ran the "queue show" CLI command 2100
times. By the end, each of his 40 queues was taking about a megabyte of
memory apiece just for their string fields.

My fix for this problem is to revert the call_queue structure from using
string fields. In my patch here, I have moved the queue back to using
fixed-sized buffers. I ran the script provided by the reporter of 17081
and determined that I no longer saw the steadily-increasing memory usage
that I had seen before applying the patch.

This patch is applicable to the 1.6.0, 1.6.1, and 1.6.2 branches of
Asterisk. Asterisk trunk (to be 1.8 eventually) does not have this problem
because the string fields API has some extra checks in place to prevent it
from being as much of a memory hog in a situation like this.

For this review, I'm mainly asking that you make sure that I did not make
any silly mistakes, like using the wrong variable name somewhere. Also,
I'm open for other suggestions for solving this.


This addresses bug 17081.
    https://issues.asterisk.org/view.php?id=17081


Diffs
-----

  /branches/1.6.0/apps/app_queue.c 260567 

Diff: https://reviewboard.asterisk.org/r/650/diff


Testing
-------

Testing is described in the description above.

The reporter of issue 17081 shared a script that would run the "queue show" CLI command 2100 times. After each, it would check to see if the number of allocations from app_queue.c had increased. With this, it is plain to see that this patch fixes the memory usage issues.


Thanks,

Mark