apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@covalent.net>
Subject Re: cvs commit: apr/file_io/unix filedup.c
Date Tue, 08 Jan 2002 17:31:40 GMT

----- Original Message ----- 
From: "David Reid" <dreid@jetnet.co.uk>
To: <dev@apr.apache.org>
Sent: Tuesday, January 08, 2002 10:01 AM
Subject: Re: cvs commit: apr/file_io/unix filedup.c


> Sorry, but I really don't like this patch.
> 
> The problem that we had wouldn't have been addressed by this patch anyways,
> and we are a library - so why are we trying to tell users what they can and
> can't do?
> 
> I'd like to revert this patch.
> 
> I think we should also have a _dup2 version of this function, but I think we
> shoudl basically do just what dup/dup2 do and not try to get overly fancy.
> We're a library, not a babysitter!

I agree with dup2.  It's easy for the user to forget to null out a ** target.

But as far as this patch is concerned [the final version, where we never close
fd's 0, 1, or 2 on cleanup], I disagree.  It's too trivial for a race condition
to intervene, between the user clearing a pool, and then dup'ing the replacement
handle, in which time another thread could call fork.

Httpd is too simple an example of how bad this could be.  Debugging such 
a problem is a real PITA (as you discovered :)  We should never expose fork to
this risk again, IMHO.

Bill



> > wrowe       02/01/07 19:45:09
> >
> >   Modified:    file_io/unix filedup.c
> >   Log:
> >     We cannot close-on-fork any fd 0 through 2 (stdin, stdout, stderr)!!!
> >     This patch is possibly still borked, we probably should remove any
> >     existing cleanup registered again (*new_file) if it was given.
> >
> >     This api really is dirty, should really have an apr_file_dup2() with
> >     different conventions (passing apr_file_t* for both left and right
> args.)
> >     I can see users 'forgetting' to null the target apr_file_t**
> destination.
> >
> 
> 
> 


Mime
View raw message