Req #38915 [Com]: Apache: system() (and similar) don't cleanup opened handles of Apache

[oliver at realtsp dot com]

Edit report at https://bugs.php.net/bug.php?id=38915&edit=1

 ID: 38915
 Comment by: oliver at realtsp dot com
 Reported by:dimmoborgir at gmail dot com
 Summary:Apache: system() (and similar) don't cleanup opened
 handles of Apache
 Status: Analyzed
 Type:   Feature/Change Request
 Package:Program Execution
 Operating System:   UNIX
 PHP Version:5.2.2, 4.4.7
 Block user comment: N
 Private report: N

 New Comment:

we solved by passing the forked/exec'd process through a bash shell and closing 
all te file 
descriptors: eg: (note this is for FreeBSD using daemon, but "nohup" should 
work 
on linux)

daemon /usr/bin/env bash -c 'exec 0<&-; exec 1> /path/to/error/log; exec 2> 
/path/to/stdout/log; 
eval exec {3..255}\>\&-; /usr/bin/env php /path/to/script args...'

Note we find it crucial to redirect and NOT CLOSE STDOUT and STDERR because 
otherwise you will 
never find out if sth is wrong with forked process. You should ensure that they 
exist and are 
writable before forking. 

The trick with eval exec {3..255}\>\&-;  is from here:
http://blog.n01se.net/blog-n01se-net-p-145.html

This works for us in a php-fastcgi situation. the fastcgi-socket and the mysql 
socket are both 
closed successfully. the new process opens its own mysql socket just fine...

I suspect this is similar to what Jeroen's closedexec.c does, but no need for a 
c program. 
Everyone should have bash. 

If you redirect the stdout of above fork command to a file and check the 
contents of that daemon 
gives you nice messages, just append 

 2> /path/to/temp/stderr/file/for/daemon/messages

to the above command.

We have the construction of the fork command wrapped in a simple function, like 
so:

$exec_cmd = ((php_uname('s') == 'FreeBSD') ? 'daemon' : 'nohup') .  

// try to 
be OS agnostic, daemon = fork, setguid etc, but don't close stderr with -f  
   
  ' /usr/bin/env bash -c ' .

// wrap 
actual call to new php process in a shell (use env!), so
 
  // must escape here in case the already escaped args contain  

 
  // specials chars like single quotes (which the will!)

 
  escapeshellarg(
'exec 0<&-; ' . 

// close 
STDIN   

'exec 1> ' . escapeshellarg($app_log) . '; ' .  

// STDOUT 
> app_log   
>
'exec 2> ' . escapeshellarg($error_log) . '; ' .

// STDOUT 
> error_log 
>
'eval exec {3..255}\>\&-; ' .   

// we can 
close all other fds (fastcgi, mysql, etc)..eval trick!  
   
'/usr/bin/env php ' . BASE . $cmd . ' ' .   

// call 
php (with env!) don't rely on shebang or exec perms 
 
join(' ',   

// add 
args separated by spaces
  
 array_map(function ($arg) { return escapeshellarg($arg); }, 
$args))
// after 
escaping them   

);


Sorry about the formatting...


Previous Comments:

[2010-04-16 01:31:48] crrodriguez at opensuse dot org

In linux, this should fix the issue for mail()

diff --git a/ext/standard/mail.c b/ext/standard/mail.c
index ab65f16..a8b3bf5 100644
--- a/ext/standard/mail.c
+++ b/ext/standard/mail.c
@@ -288,8 +288,12 @@ PHPAPI int php_mail(char *to, char *subject, char 
*message, 
char *headers, char
 * (e.g. the shell can't be executed) we explicitely set it to 0 to be
 * sure we don't catch any older errno value. */
errno = 0;
+#if defined(__linux__) && defined(__GLIBC__) &&  __GLIBC_PREREQ(2, 9)
+   sendmail = popen(sendmail_cmd, "we");
+#else 
sendmail = popen(sendmail_cmd, "w");
 #endif
+#endif
if (extra_cmd != NULL) {
efree (sendmail_cmd);
}


Note that you need glibc 2.9 though.


[2010-02-22 19:16:37] ionut dot dumitru at webland dot ro

the problem is still there in 5.2 just with php not

Req #38915 [Com]: Apache: system() (and similar) don't cleanup opened handles of Apache

[crrodriguez at opensuse dot org]

Edit report at http://bugs.php.net/bug.php?id=38915&edit=1

 ID:   38915
 Comment by:   crrodriguez at opensuse dot org
 Reported by:  dimmoborgir at gmail dot com
 Summary:  Apache: system() (and similar) don't cleanup opened
   handles of Apache
 Status:   Analyzed
 Type: Feature/Change Request
 Package:  Feature/Change Request
 Operating System: UNIX
 PHP Version:  5.2.2, 4.4.7

 New Comment:

In linux, this should fix the issue for mail()



diff --git a/ext/standard/mail.c b/ext/standard/mail.c

index ab65f16..a8b3bf5 100644

--- a/ext/standard/mail.c

+++ b/ext/standard/mail.c

@@ -288,8 +288,12 @@ PHPAPI int php_mail(char *to, char *subject, char
*message, 

char *headers, char

 * (e.g. the shell can't be executed) we explicitely set it to 0
to be

 * sure we don't catch any older errno value. */

errno = 0;

+#if defined(__linux__) && defined(__GLIBC__) &&  __GLIBC_PREREQ(2, 9)

+   sendmail = popen(sendmail_cmd, "we");

+#else 

sendmail = popen(sendmail_cmd, "w");

 #endif

+#endif

if (extra_cmd != NULL) {

efree (sendmail_cmd);

}





Note that you need glibc 2.9 though.


Previous Comments:

[2010-02-22 19:16:37] ionut dot dumitru at webland dot ro

the problem is still there in 5.2 just with php not involving apache.

so i write a cli daemon A which uses a listener socket , at some point
it starts another daemon B with any of exec/system/popen etc. 'A' works
as a sort of supervisor for B so i can't shut it down. but at some point
i need to restart A, well I can't cause it won't bind to the same
listener address anymore because B is keeping the handles open. spent a
lot of time but i guess i have to go the file/cron way since php can't
clean itself.


[2009-12-23 18:45:09] devrim at kodingen dot com

Apparently Jeroen's closedexec.c code is not available from his
website,



here is my copy: http://pastie.org/754717



It is very useful to us, and that fix is still not there.


[2009-07-03 00:38:14] ras...@php.net

This is finally fixed in Apache now.



https://issues.apache.org/bugzilla/show_bug.cgi?id=46425



It really is the responsibility of initiator of an fd to set the
O_CLOEXEC flag on it.  Going back and trying to do it after the fact is
really messy because we don't have direct access to these fds.  Since I
can't think of a portable way to get a list of all open fds, we would
have to pick some arbitrary number and loop through and set FD_CLOEXEC
on the first N fds.



Something like this:



Index: ext/standard/exec.c

===

RCS file: /repository/php-src/ext/standard/exec.c,v

retrieving revision 1.113.2.3.2.13

diff -u -r1.113.2.3.2.13 exec.c

--- ext/standard/exec.c 30 Apr 2009 15:25:05 -  1.113.2.3.2.13

+++ ext/standard/exec.c 3 Jul 2009 00:35:25 -

@@ -101,6 +101,17 @@

sig_handler = signal (SIGCHLD, SIG_DFL);

 #endif

 

+#if defined(F_SETFD) && defined(FD_CLOEXEC)

+   {

+   int i, oldflags;

+   for(i=0; i<10; i++) {

+   oldflags = fcntl(i, F_GETFD, 0);

+   oldflags |= FD_CLOEXEC;

+   fcntl(i, F_SETFD, oldflags);

+   }

+   }

+#endif

+

 #ifdef PHP_WIN32

fp = VCWD_POPEN(cmd_p, "rb");

 #else



and something similar would have to be done in the other places we fork,
like in mail().  This is extremely ugly, as far as I am concerned, and
likely doesn't catch everything anyway.



If someone sees a clean way of fixing this that I have missed, please
let us know.  Otherwise I would encourage you to upgrade your Apache or
lean on whatever web server you are using that is passing dirty fds to
us.




[2009-06-25 16:07:50] virus at tgu dot ru

OMG...

It's still didn't fixed... :(


[2008-08-26 13:08:53] anomie at users dot sf dot net

It seems that it could easily be fixed by having mod_php use the
Apache-provided functions which are intended to solve exactly this
problem, but the PHP developers seem to have decided to ignore this bug
instead.



Rather than a cron script, use a trampoline program like the one posted
by jeroen at unfix dot org; we have something similar here, although
ours doesn't impose an arbitrary run time limit.




The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

http://bugs.php.net/bug.php?id=38915


-- 
E