[Home]

Summary:ASTERISK-09825: When removing a class from musiconhold.conf and reloading the module, the class is not removed from memory
Reporter:Leif Madsen (lmadsen)Labels:
Date Opened:2007-07-06 14:16:52Date Closed:2007-08-16 20:05:55
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Resources/res_musiconhold
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) asterisk-10139.diff.txt
( 1) asterisk-10139-advanced.diff.txt
Description:This happens when loading from extconfig.conf as well, but since it happens in the flatfile, I'll assume it's the same problem in both cases.

If I add a new class to musiconhold.conf, then run 'module reload res_musiconhold.so' from the *CLI, then the class is added.

If I remove the class from the musiconhold.conf file, then reload res_musiconhold.so, then the class is not removed from memory (only a restart will remove it).
Comments:By: Jason Parker (jparker) 2007-07-06 15:30:03

It looks like it purposely doesn't remove classes.  It also doesn't appear to modify existing ones.
By: Russell Bryant (russell) 2007-07-06 17:35:26

I don't doubt it, but it does seem like the right thing to do is to remove ones that are no longer configured.  However, the code would also have to take care not to remove it while it is still in use by any channels.
By: Joshua C. Colp (jcolp) 2007-07-08 20:23:55

There could also be an additional problem because musiconhold classes that utilize the files method allow channels to return to the same class at the point at which they left it. We would have to make sure that this did not go kaboom in the process.
By: Digium Subversion (svnbot) 2007-07-13 15:47:48

Repository: asterisk
Revision: 75110

------------------------------------------------------------------------
r75110 | russell | 2007-07-13 15:47:47 -0500 (Fri, 13 Jul 2007) | 2 lines

create a branch for issue ASTERISK-9825 (svn/asterisk/team/russell/issue_10139)

------------------------------------------------------------------------
By: James Golovich (jamesgolovich) 2007-08-16 16:17:47

So heres the deal. asterisk-10139.diff.txt is the simple patch that just makes "module reload app_musiconhold.so" behave just like "moh reload".  There are 2 downsides to it

#1 it exposed the bug that when you use one of the apps to start MOH if you reload in the middle it reverts your MOH back to 'default' even if there were no changes.
#2 every moh in progress is stopped and restarted when the reload happens

The upside is that its extremely simple and doesn't cause any extra locking of the big list.

The asterisk-10139-advanced.diff.txt is the more complicated version that doesn't have those two issues.  It basically flags channels to be deleted and if they aren't in use deletes them right away, or if they are in use they will be deleted when the last channel that is using them exits.  The downside is that its more complicated and has a few more locks happening.

Edit: These are patches against trunk.  The first one should easily apply against 1.4.x, the second one I'm not sure about since there are a few differences between 1.4.x and trunk.


By: Digium Subversion (svnbot) 2007-08-16 17:06:37

Repository: asterisk
Revision: 79778

------------------------------------------------------------------------
r79778 | russell | 2007-08-16 17:06:35 -0500 (Thu, 16 Aug 2007) | 14 lines

This patch fixes a bug where reloading the module with "module reload" did not
delete classes from memory that were no longer in the config.  This patch fixes
that problem as well as another one.  Previously, if you reloaded MOH using the
"moh reload" CLI command, which behaved differently than "module reload ...",
MOH had to be stopped on every channel and started again immediately.  However,
there was no way to tell what class was being used, so they would all fall back
to the default class.

(closes issue ASTERISK-9825)
Reported by: blitzrage
Patches:
     asterisk-10139-advanced.diff.txt uploaded by jamesgolovich (license 176)
Tested by: jamesgolovich

------------------------------------------------------------------------
By: Digium Subversion (svnbot) 2007-08-16 17:12:50

Repository: asterisk
Revision: 79788

------------------------------------------------------------------------
r79788 | russell | 2007-08-16 17:12:49 -0500 (Thu, 16 Aug 2007) | 22 lines

Merged revisions 79778 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r79778 | russell | 2007-08-16 17:24:25 -0500 (Thu, 16 Aug 2007) | 14 lines

This patch fixes a bug where reloading the module with "module reload" did not
delete classes from memory that were no longer in the config.  This patch fixes
that problem as well as another one.  Previously, if you reloaded MOH using the
"moh reload" CLI command, which behaved differently than "module reload ...",
MOH had to be stopped on every channel and started again immediately.  However,
there was no way to tell what class was being used, so they would all fall back
to the default class.

(closes issue ASTERISK-9825)
Reported by: blitzrage
Patches:
     asterisk-10139-advanced.diff.txt uploaded by jamesgolovich (license 176)
Tested by: jamesgolovich

........

------------------------------------------------------------------------
By: Digium Subversion (svnbot) 2007-08-16 20:05:55

Repository: asterisk
Revision: 79825

------------------------------------------------------------------------
r79825 | file | 2007-08-16 20:05:53 -0500 (Thu, 16 Aug 2007) | 106 lines

Merged revisions 79747,79749,79755,79764,79788,79794,79813,79824 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r79747 | tilghman | 2007-08-16 18:09:46 -0300 (Thu, 16 Aug 2007) | 2 lines

Don't reload a configuration file if nothing has changed.

................
r79749 | mmichelson | 2007-08-16 18:21:35 -0300 (Thu, 16 Aug 2007) | 16 lines

Merged revisions 79748 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r79748 | mmichelson | 2007-08-16 16:16:40 -0500 (Thu, 16 Aug 2007) | 8 lines

Fixes a problem where agents would get stuck busy due to their wrapuptime being longer than the queue's wrapuptime and
ringinuse=no for the queue.

(closes issue ASTERISK-9889, reported by Doug, repaired by me)

Special thanks to fkasumovic for pointing out the source of the problem and to bweschke for helping to come up with a solution!


........

................
r79755 | file | 2007-08-16 18:28:50 -0300 (Thu, 16 Aug 2007) | 2 lines

Fix properties on trunk again.

................
r79764 | russell | 2007-08-16 18:33:38 -0300 (Thu, 16 Aug 2007) | 19 lines

Merged revisions 79756 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r79756 | russell | 2007-08-16 16:29:24 -0500 (Thu, 16 Aug 2007) | 11 lines

Fix more deadlocks in chan_iax2 that were introduced by making frame handling
and scheduling multi-threaded.  Unfortunately, we have to do some expensive
deadlock avoidance when queueing frames on to the ast_channel owner of the IAX2
pvt struct.  This was already handled for regular frames, but ast_queue_hangup
and ast_queue_control were still used directly.  Making these changes introduced
even more places where the IAX2 pvt struct can disappear in the context of a
function holding its lock due to calling a function that has to unlock/lock it
to avoid deadlocks.  I went through and fixed all of these places to account for
this possibility.
(issue ASTERISK-10007, patch by me)

........

................
r79788 | russell | 2007-08-16 19:30:39 -0300 (Thu, 16 Aug 2007) | 22 lines

Merged revisions 79778 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r79778 | russell | 2007-08-16 17:24:25 -0500 (Thu, 16 Aug 2007) | 14 lines

This patch fixes a bug where reloading the module with "module reload" did not
delete classes from memory that were no longer in the config.  This patch fixes
that problem as well as another one.  Previously, if you reloaded MOH using the
"moh reload" CLI command, which behaved differently than "module reload ...",
MOH had to be stopped on every channel and started again immediately.  However,
there was no way to tell what class was being used, so they would all fall back
to the default class.

(closes issue ASTERISK-9825)
Reported by: blitzrage
Patches:
     asterisk-10139-advanced.diff.txt uploaded by jamesgolovich (license 176)
Tested by: jamesgolovich

........

................
r79794 | russell | 2007-08-16 19:33:02 -0300 (Thu, 16 Aug 2007) | 12 lines

Merged revisions 79792 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r79792 | russell | 2007-08-16 17:32:33 -0500 (Thu, 16 Aug 2007) | 4 lines

Fix a little race condition that could cause a crash if two channels had MOH
stopped at the same time that were using a class that had been marked for
deletion when its use count hits zero.

........

................
r79813 | tilghman | 2007-08-16 20:31:14 -0300 (Thu, 16 Aug 2007) | 2 lines

Revise dialplan locks to permit multiple locks per channel, but with deadlock avoidance

................
r79824 | file | 2007-08-16 22:19:04 -0300 (Thu, 16 Aug 2007) | 2 lines

Fix building of chan_zap under development mode without libpri and libss7 installed.

................

------------------------------------------------------------------------