Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Received-SPF: pass (nike.apache.org: local policy)
From: Stefan Fritsch <sf@sfritsch.de>
To: dev@apr.apache.org
Subject: apr_file_dup and FD_CLOEXEC revisited
Date: Thu, 11 Jun 2009 18:10:04 +0200
User-Agent: KMail/1.11.4 (Linux/2.6.29-2-686; KDE/4.2.4; i686; ; )
MIME-Version: 1.0
Content-Type: Multipart/Mixed;
  boundary="Boundary-00=_dxSMKoQMnLb8Khi"
Message-Id: <200906111810.05014.sf@sfritsch.de>

--Boundary-00=_dxSMKoQMnLb8Khi
Content-Type: text/plain;
  charset="us-ascii"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

Hi,

I finally got around to test the stderr reopening behaviour with the 
various FD_CLOEXEC bits in trunk. I noticed that this still does not 
work, i.e. when mod_php exec()s another program, stderr gets closed.

The problem is in apr_file_dup2(). It should retain the 
INHERIT/NOCLEANUP flags of new_file, but currently it checks those 
flags in old_file. There was some discussion about this starting at
http://www.mail-archive.com/dev@apr.apache.org/msg21357.html

The attached patch fixes the behaviour. With it, a program exec()ed by 
mod_php has exactly stdin, stdout, and stderr open, and nothing else.

With the patch, I think the FD_CLOEXEC bits can be backported to 1.4. 
The relevant commits are
747990
748361
748371
748565
748988
749810

One remaining problem is that when apr is compiled with these changes 
on a recent linux kernel, it won't run on older kernels due to the 
various new syscalls missing. Therefore, maybe it is not a good idea 
to backport this to 1.3. Or one could disable the use of the new 
syscalls for 1.3.

Cheers,
Stefan


--Boundary-00=_dxSMKoQMnLb8Khi
Content-Type: text/x-patch;
  charset="ISO-8859-15";
  name="dup_cloexec.diff"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
	filename="dup_cloexec.diff"

diff --git a/file_io/unix/filedup.c b/file_io/unix/filedup.c
index d40004f..47ea4f3 100644
=2D-- a/file_io/unix/filedup.c
+++ b/file_io/unix/filedup.c
@@ -35,12 +35,12 @@ static apr_status_t file_dup(apr_file_t **new_file,
             return APR_EINVAL;
         }
 #ifdef HAVE_DUP3
=2D        if (!(old_file->flags & (APR_FILE_NOCLEANUP|APR_INHERIT)))
+        if (!((*new_file)->flags & (APR_FILE_NOCLEANUP|APR_INHERIT)))
             flags |=3D O_CLOEXEC;
         rv =3D dup3(old_file->filedes, (*new_file)->filedes, flags);
 #else
         rv =3D dup2(old_file->filedes, (*new_file)->filedes);
=2D        if (!(old_file->flags & (APR_FILE_NOCLEANUP|APR_INHERIT))) {
+        if (!((*new_file)->flags & (APR_FILE_NOCLEANUP|APR_INHERIT))) {
             int flags;
=20
             if (rv =3D=3D -1)

--Boundary-00=_dxSMKoQMnLb8Khi--