Hi! Anatol Belski wrote:
> regarding the mentioned tickets I've prepared a patch > https://github.com/php/php-src/compare/master...weltling:pipe_blocking_win . Great. > 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 It appears to be possible to set the buffer size by passing a nSize argument > 0 to CreatePipe[1][2]. Maybe it's reasonable to use 16 or 64 kB by default? > 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. I did some quick tests and the patch works fine. However, proc_open_bug69900.phpt fails on my machine (Win 7), because 0ms are reported for all but the first read (microtime issue on older Windows'). Maybe it's better to EXPECT something like "took less than 1 ms"? I have noticed that you've changed the bitfields of struct php_stdio_stream_data (in plain_wrapper.c). Wouldn't it be better to do: - unsigned is_pipe_blocking; + unsigned is_pipe_blocking:1; + unsigned _reserved:28; [1] <https://msdn.microsoft.com/en-us/library/windows/desktop/aa365152(v=vs.85).aspx> [2] <https://github.com/php/php-src/blob/php-5.6.10/ext/standard/proc_open.c#L414> -- Christoph M. Becker -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
