From reviewboard:
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.
(closes issue #17081)
Reported by: wliegel
Patches:
17081v2.patch uploaded by mmichelson (license 60)
Tested by: wliegel, mmichelson
Review: https://reviewboard.asterisk.org/r/651/
git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.2@265172 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/trunk
................
r265090 | mmichelson | 2010-05-21 16:08:51 -0500 (Fri, 21 May 2010) | 15 lines
Merged revisions 265089 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r265089 | mmichelson | 2010-05-21 15:59:14 -0500 (Fri, 21 May 2010) | 8 lines
Don't hang up on a queue caller if the file we attempt to play does not exist.
This also fixes a documentation mistake in file.h that made my original attempt
to correct this problem not work correctly.
(closes issue #17061)
Reported by: RoadKill
........
................
git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.2@265091 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/trunk
........
r237920 | dvossel | 2010-01-05 17:08:50 -0600 (Tue, 05 Jan 2010) | 16 lines
fixes holdtime playback issue in app_queue
When reporting hold time, the number of seconds should be mod 60.
Otherwise audio playback could be something like "2 minutes 123 seconds"
rather than "2 minutes 3 seconds".
Also, the "minute" sound file is missing, so for the moment until
that file can be created the "minutes" file is used instead.
(closes issue #16168)
Reported by: nickilo
Patches:
patch-unified-trunk-rev-222176 uploaded by nickilo (license )
Tested by: nickilo, wonderg
........
git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.2@237921 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/trunk
........
r231556 | dvossel | 2009-11-30 12:55:07 -0600 (Mon, 30 Nov 2009) | 11 lines
app_queue crashes randomly, often during call-transfers
This patch adds a ref to the queue_ent object's parent call_queue
in queue_exec() so the call_queue won't be destroyed
while the the queue_ent still holds a pointer to it.
(closes issue 0015686)
Tested by: dvossel, aragon
........
git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.2@231558 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/trunk
................
r222176 | kpfleming | 2009-10-05 20:24:24 -0500 (Mon, 05 Oct 2009) | 27 lines
Recorded merge of revisions 222152 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r222152 | kpfleming | 2009-10-05 20:16:36 -0500 (Mon, 05 Oct 2009) | 20 lines
Fix ao2_iterator API to hold references to containers being iterated.
See Mantis issue for details of what prompted this change.
Additional notes:
This patch changes the ao2_iterator API in two ways: F_AO2I_DONTLOCK
has become an enum instead of a macro, with a name that fits our
naming policy; also, it is now necessary to call
ao2_iterator_destroy() on any iterator that has been
created. Currently this only releases the reference to the container
being iterated, but in the future this could also release other
resources used by the iterator, if the iterator implementation changes
to use additional resources.
(closes issue #15987)
Reported by: kpfleming
Review: https://reviewboard.asterisk.org/r/383/
........
................
git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.2@222187 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/trunk
................
r211040 | tilghman | 2009-08-07 13:17:41 -0500 (Fri, 07 Aug 2009) | 21 lines
Merged revisions 211038 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r211038 | tilghman | 2009-08-07 13:16:28 -0500 (Fri, 07 Aug 2009) | 14 lines
QUEUE_MEMBER_LIST _really_ wants the interface name, not the membername.
This is a partial revert of revision 82590, which was an attempted cleanup,
but in reality, it broke QUEUE_MEMBER_LIST, which has always been intended
as a method by which component interfaces could be queried from the queue.
Membername isn't useful here, because that field cannot be used to obtain
further information about the member. See the documentation on
QUEUE_MEMBER_LIST, RemoveQueueMember, QUEUE_MEMBER_PENALTY, and the various
AMI commands which take a member argument for further justification.
(closes issue #15664)
Reported by: rain
Patches:
app_queue-queue_member_list.diff uploaded by rain (license 327)
........
................
git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.2@211051 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/trunk
................
r205350 | mmichelson | 2009-07-08 14:26:55 -0500 (Wed, 08 Jul 2009) | 20 lines
Merged revisions 205349 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r205349 | mmichelson | 2009-07-08 14:26:13 -0500 (Wed, 08 Jul 2009) | 14 lines
Prevent phantom calls to queue members.
If a caller were to hang up while a periodic announcement or position
were being said, the return value for those functions would incorrectly
indicate that the caller was still in the queue. With these changes,
the problem does not occur.
(closes issue #14631)
Reported by: latinsud
Patches:
queue_announce_ghost_call2.diff uploaded by latinsud (license 745)
(with small modification from me)
........
................
git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.2@205353 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/trunk
........
r193349 | mmichelson | 2009-05-08 14:50:44 -0500 (Fri, 08 May 2009) | 12 lines
Reset the members' call counts when resetting queue statistics.
This helps to prevent odd scenarios where a queue will claim to have
taken 0 calls, but the members appear to have taken a non-zero amount.
(closes issue #15068)
Reported by: sum
Patches:
patchreset.patch uploaded by sum (license 766)
Tested by: sum
........
git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.2@193350 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/trunk
........
r188032 | mmichelson | 2009-04-13 09:17:56 -0500 (Mon, 13 Apr 2009) | 6 lines
Set all queue variables on both the caller and member channels.
This allows for the variables to be accessed if a member macro is run.
Thanks to Grigoriy Puzankin for bringing this up on the -dev list.
........
git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.2@188039 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/trunk
................
r185072 | mmichelson | 2009-03-30 11:26:48 -0500 (Mon, 30 Mar 2009) | 45 lines
Merged revisions 185031 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r185031 | mmichelson | 2009-03-30 11:17:35 -0500 (Mon, 30 Mar 2009) | 39 lines
Fix queue weight behavior so that calls in low-weight queues are not inappropriately blocked.
(This is copied and pasted from the review request I made for this patch)
Asterisk has some odd behavior when queue weights are used. The current logic used when
potentially calling a queue member is:
If the member we are going to call is part of another queue and _that other queue has any
callers in it_ and has a higher weight than the queue we are calling from, then don't try
to contact that member. The issue here is what I have marked with underscores. If the
higher-weighted queue has any callers in it at all, then the queue member will be unreachable
from the lower-weighted queue. This has the potential to be really really bad if using a
queue strategy, such as leastrecent or fewestcalls, with the potential to call the same
member repeatedly.
The fix proposed by garychen on issue 13220 is very simple and, as far as I can see, works
well for this situation. With this set of changes, the logic used becomes:
If the member we are going to call is part of another queue, the other queue has a higher
weight than the queue we are calling from, and the higher weight queue has at least as many
callers as available members, then do not try to contact the queue member. If the higher
weighted queue has fewer callers than available members, then there is no reason to deny
the call to this member since the other queue can afford to spare a member.
Since the fix involved writing a generic function for determining the number of available
members in the queue, I also modified the is_our_turn function to make use of the new
num_available_members function to determine if it is our turn to try calling a member. There
is one small behavior change. Before writing this patch, if you had autofill disabled, then
if you were the head caller in a queue, you would automatically be told that it was your
turn to try calling a member. This did not take into account whether there were actually any
queue members available to take the call. Now we actually make sure there is at least one
member available to take the call if autofill is disabled.
(closes issue #13220)
Reported by: garychen
Review: http://reviewboard.digium.com/r/202/
........
................
git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.2@185089 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/trunk
........
r183244 | mmichelson | 2009-03-19 13:10:34 -0500 (Thu, 19 Mar 2009) | 16 lines
Fix a memory leak associated with queues.
For every attempt that app_queue made to place an outbound call to a queue member,
we would allocate a queue_end_bridge structure. When the bridge for the call had
completed, we would free the structure. Unfortunately not all call attempts actually
end up bridged to a member, so we need to be more selective of when to allocate
the structure. With this change, the allocation occurs in an area where we can
guarantee that the call will be bridged.
(closes issue #14680)
Reported by: caspy
Patches:
14680.patch uploaded by mmichelson (license 60)
Tested by: caspy
........
git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.2@183247 65c4cc65-6c06-0410-ace0-fbb531ad65f3
If trying to dial a non-existent queue, there would
be a segfault when attempting to access q->weight, even
though q was NULL. This problem was introduced during
the queue-reset merge and thus only affects trunk.
(closes issue #14643)
Reported by: alecdavis
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@181244 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r180006 | mmichelson | 2009-03-03 16:48:18 -0600 (Tue, 03 Mar 2009) | 17 lines
Clarify some documentation of queues.conf.sample
It had always been possible to explicitly specify a "blank"
value for a sound file in queues.conf and have no sound played
back. The problem with this is that it would result in some ugly
CLI warnings from file.c.
This commit introduces a check when playing a file in app_queue
to see if the name of the file is zero-length and return early if
that is the case. Also, the ability to specify the blank sound
files in queues.conf is now mentioned more clearly in queues.conf.sample
(closes issue #14227)
Reported by: caspy
........
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@180007 65c4cc65-6c06-0410-ace0-fbb531ad65f3
The problem here is that the hint processing code was subscribed to the wrong
event type. So, it started processing state for a hint too soon, before the
device state cache had been updated.
Also, fix a similar bug in app_queue, as it was also subscribed to the wrong
event type.
(closes issue #14461)
Reported by: alecdavis
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@176557 65c4cc65-6c06-0410-ace0-fbb531ad65f3
From a user point-of-view, this adds new CLI commands and Manager Actions to
better facilitate the reloading of queues and the resetting of their statistics.
The new CLI commands are the "queue reload" and "queue reset stats" commands.
The new manager actions are the QueueReload and QueueReset commands.
Review: http://reviewboard.digium.com/r/115
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@175663 65c4cc65-6c06-0410-ace0-fbb531ad65f3
If someone has configured the queue to play an position or holdtime
announcement, then it is odd and potentially unexpected to hear a
"Thank you for your patience" sound when no position or holdtime
was actually announced.
This fixes the announcement so that the "thanks" sound is only played
in the case that a position or holdtime was actually announced.
There is a way that the "thank you" sound can be played without a
position or holdtime, and that is to set announce-frequency to a value
but keep announce-position and announce-holdtime both turned off.
(closes issue #14227)
Reported by: caspy
Patches:
14227_v3.patch uploaded by putnopvut (license 60)
Tested by: caspy
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@174948 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r173692 | mmichelson | 2009-02-05 14:29:09 -0600 (Thu, 05 Feb 2009) | 12 lines
Fix situations where queue members could be autopaused unexpectedly
Specifically, this patch prevents us from autopausing members when
we receive a busy or congestion frame from them.
(closes issue #14376)
Reported by: fiddur
Patches:
14376.patch uploaded by putnopvut (license 60)
Tested by: fiddur
........
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@173693 65c4cc65-6c06-0410-ace0-fbb531ad65f3
Also, implement a private cause code (as suggested by Tilghman). This works with
chan_sip, but doesn't propagate through chan_local.
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@172318 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This is to avoid having the list and counter of missed calls being touched by queue calls. Add the C option to queue() and nothing
will be logged on phones that support the Reason: header on SIP cancel, like the SNOM phones.
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@171924 65c4cc65-6c06-0410-ace0-fbb531ad65f3
The data passed to the end_bridge_callback was assumed to be data which was
still stack'd. The problem was that with some call features, attended transfers
in particular, a new bridge thread is started once the feature completes, meaning
that when the end_bridge_callback is called, the end_bridge_callback_data was
invalid.
To fix this problem, there are two measures taken
1. Instead of pointing to stacked data, we now used heap-allocated data for
passing to the end_bridge_callback in app_queue
2. Since bridges can end multiple times on a single logical call, we wait until
the final bridge is broken to actually set any queue variables. This is accomplished
through reference-counting and the use of an end_bridge_callback_data_fixup function
in app_queue.c
(closes issue #14260)
Reported by: ccesario
Patches:
14260.patch uploaded by putnopvut (license 60)
Tested by: ccesario
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@171618 65c4cc65-6c06-0410-ace0-fbb531ad65f3
The fix being applied is a bit different for trunk and the 1.6.X branches.
For trunk, we only wish to strip off the characters beyond the second slash
if the channel is a Local channel (i.e. we are removing the /n from the device
name). Other channel technologies with multiple slashes (e.g. DAHDI) need the
information after the second slash in order to get the proper device state
information.
In addition to this fix, the 1.6.X branches are receiving a much more important
fix as well. The problem in 1.6.X is that the member's device name was being directly
changed instead of having a copy changed. This meant that we would strip off the
second slash and trailing characters and then leave the member's device name like
that permanently thereafter.
(closes issue #14014)
Reported by: kebl0155
Patches:
14014_number2.patch uploaded by putnopvut (license 60)
Tested by: kebl0155
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@169611 65c4cc65-6c06-0410-ace0-fbb531ad65f3
