Re: Fixes for #69900 and #69963

Hi Anatol, I don't see any problems committing this. Thanks. Dmitry. On Tue, Jun 30, 2015 at 9:41 PM, Anatol Belski <anatol.php@belski.net> wrote: > Hi, > > regarding the mentioned tickets I've prepared a patch > > https://github.com/php/php-src/compare/master...weltling:pipe_blocking_win > . > Anonymous pipes on Windows > > - don't support asynchronous IO > - don't support select() > - have buffer size of 4096 bytes (or even less) which is much easier > overflown that a buffer of 64kb often to be seen on Unix > > These issues was addressed last year, but the tickets mentioned report > about > the issues still persisting in PHP5/7. While it's likely a no go for PHP5, > I > would like to push this patch into PHP7. Particularly what it does is > allowing the blocking read() again for the edge cases reported in the > listed > tickets. It is done adding a context option and a new win only option for > proc_open. Just to illustrate a few - > > ==== SNIPPET ==== > /* here proc_open() will do a blocking read on the child process*/ > $process = proc_open(PHP_BINARY.' -f ' . $fl, $descriptorspec, $pipes, > NULL, > NULL, array("blocking_pipes" => true)); > ==== END SNIPPET ==== > > ==== SNIPPET ==== > $in = fopen("php://stdin", "rb", false, > stream_context_create(array("pipe" > => array("blocking" => true)))); > +while(!feof($in)){ /* here feof() will do a blocking read on the pipe */ > ........................ > ==== END SNIPPET ==== > > The only drawback on this is that blocking reads on pipes are likely to > cause pipe buffer overflows and thus deadlock, like it was before. Though, > when working with them carefully, this could be avoided and would bring > improvements with several edge cases, almost when working on CLI. Anyway > this patch brings no difference to PHP5 while addressing those edge cases. > > I would like to ask for review here. I've also made a minimal build with > this patch http://windows.php.net/downloads/snaps/ostc/69900/ > for those > interested to test. If there are no strong objections, it should be merged > soon. > > Thanks > > Anatol > >