But the patch didn't actually do anything to prevent the problem we saw as it was in NO way related to a fork(). The issue is that if we call a pool_clear we'll run the cleanup's anyway, as we should.
I think I'd be happier just letting the user decide and not getting all motherly and trying to keep stdin/out/err open at all costs. It could be that the user wants them closed! It's perfectly OK to try and close stderr, open a pipe and get stderr as one end, if that what you *want* to happen. (can't think why, but it should be possible). Httpd was a simple case, but once again the correct solution was concerned with pool lifetimes, not the opening/closing of fd's. In fact my tree hasn't got your commit in it, and it runs like a dream once we use plog :) We've said it time and time again, we're only a library and if people want to use us to do stupid stuff, that's not our concern. I have a patch that adds dup2, just need some test's for it. david > > 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
