Hi Anatol,

I don't see any problems committing this.

Thanks. Dmitry.


On Tue, Jun 30, 2015 at 9:41 PM, Anatol Belski <anatol....@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
>
>

Reply via email to