[Home]

Summary:ASTERISK-04416: [patch] FreeBSD portability and mpg123 termination bugs
Reporter:Luigi Rizzo (rizzo)Labels:
Date Opened:2005-06-15 07:44:46Date Closed:2011-06-07 14:10:49
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Resources/res_musiconhold
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) moh.diff
Description:This patch fixes two issues, largely documented in the patch itself too.

First, FreeBSD systems with userland thread library (any of 4.x,5.x,6.x)
have some issues in the handling of blocking file descriptors which require
the workaround described here and implemented in the first two chunks
of the patch. I believe the additional fcntl() does not harm on Linux, so
if someone can test it there i think this should be committed in the interest
of portability.

Second, mpg123 forks a process to decouple input and output.
The original method of killing only the parent is not guaranteed to
work, and it may well leave a stray mpg123 process around.
This patch fixes it by setting the process group id of the child to a unique
value before the exec(), and then calling killpg() instead of kill(),
to deliver the signal to all mpg123 processes.
I believe this bug was present also on Linux systems.

****** ADDITIONAL INFORMATION ******

The first issue may affect other BSD systems as well.

Also, a similar set of fixes should be applied in all places where asterisk
fork()s a new process.
Comments:By: Kevin P. Fleming (kpfleming) 2005-07-11 17:34:36

Committed to CVS HEAD, thanks! (and sorry for the delay)
By: Kevin P. Fleming (kpfleming) 2005-07-12 14:51:40

This patch is causing 100% CPU consumption running Asterisk on Linux systems, with no active channels (but with default mpg123 moh classes operating). I've pulled it from CVS HEAD for now, until the cause can be found.
By: Luigi Rizzo (rizzo) 2005-07-12 16:40:17

annoying :)
Which version of mpg123, and is it asterisk or mpg123 that eats the CPU ?

Could you try to put an
#ifdef __FreeBSD__
#endif
arount the line
fcntl(i, F_SETFL, O_NONBLOCK | fl);
to see if the problem disappears ?
The only differences at this point should be that the new code
closes stdin and stderr, which mpg123 should not need
(it better not, because asterisk wants to use them!)
So if the problem remains it we can try and leave them open,
or redirect to /dev/null.
By: Mark Spencer (markster) 2005-08-02 17:14:43

Okay, rizzo, go ahead and submit a patch that has this all #ifdef *BSD or and we can reapply, but please be sure it doesn't affect the linux side at all.
By: Michael Jerris (mikej) 2005-08-09 12:45:04

suspended due to no response.  Please re-open when an updated patch is ready.
By: Digium Subversion (svnbot) 2008-01-15 15:40:46.000-0600

Repository: asterisk
Revision: 6086

U   trunk/res/res_musiconhold.c

------------------------------------------------------------------------
r6086 | kpfleming | 2008-01-15 15:40:45 -0600 (Tue, 15 Jan 2008) | 3 lines

fix threading portability problem with FreeBSD (bug ASTERISK-4416)
ensure that all mpg123 child processes get killed when the parent is killed (bug ASTERISK-4416)

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

http://svn.digium.com/view/asterisk?view=rev&revision=6086
By: Digium Subversion (svnbot) 2008-01-15 15:41:09.000-0600

Repository: asterisk
Revision: 6111

U   trunk/res/res_musiconhold.c

------------------------------------------------------------------------
r6111 | kpfleming | 2008-01-15 15:41:08 -0600 (Tue, 15 Jan 2008) | 2 lines

revert patch from bug ASTERISK-4416 until CPU consumption problem can be resolved

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

http://svn.digium.com/view/asterisk?view=rev&revision=6111