Re: Passing process handles to proc_open and co

hi, On Thu, Dec 6, 2012 at 12:40 PM, Ángel González <keisial@gmail.com> wrote: > On 06/12/12 09:49, Pierre Joye wrote: >> hi! >> >> While looking at the bug #63073, I was wondering if we could simply do >> not pass open handles to the newly created child process. > > Looking at proc_open, where there is an explicit CreateProcess (we seem > to be using the library popen for the exec()...) We use CreateProcess too for our windows popen implementation, see: http://lxr.php.net/xref/PHP_5_4/TSRM/tsrm_win32.c#450 > If $pipes is empty and $descriptorspec doesn't require more than 0,1,2 > then it should be safe not to set bInheritHandles. > > However, in other cases inheriting handles *will* be needed. hm, Why? >> The main issue in this bug is the session related handles. They are >> passed to the parent process, which hangs until their are closed. It >> indeed does not happen (usually) until the end of the request. It >> makes these functions almost unusable as soon as sessions are used. >> >> These functions are about calling external commands and I fail to see >> which usage may require to access PHP related handles. Does anyone see >> any issue by not passing them anymore? And fixing this bug (and a >> couple of other I think)? >> >> It is windows only. Not sure if we do the same on other platforms. > The issue was only detected with session files. Windows is probably the > only mainstream OS without F_SETFD. When it's available, mod_files.c > will set the FD_CLOEXEC, which should remove the issue from the session. > > The same issue could still arise from other files, though. For instance > bug 44994 reported that it was happening with error.log. > > > I don't know *why* session files are an issue. It would make sense if they > were pipes and the reading was missed with the inherited child unkowingly > holding a write descriptor, but these are plain files. > Maybe related to the fact that they are locked, if the locks are also > inherited, > plus the way Windows may delay its automatic UnlockFile() after the process > closes. > > -- Pierre @pierrejoye