----- Original Message ----- From: "David Reid" <[EMAIL PROTECTED]> To: <[email protected]> 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. > > > > >
