Consider not throwing exception if stream cannot be set to non-blocking mode

[ostrolucky]

This library currently cannot be used on Windows OS because of exceptions which are thrown, like here. However, if I comment out such throwns in your library, following code works, albeit extremly slowly

<?php

use Psr\Http\Message\ServerRequestInterface;
use React\EventLoop\Factory;
use React\Http\Response;
use React\Http\Server;
use React\Stream\ReadableResourceStream;
use React\Stream\WritableResourceStream;

require __DIR__ . '/../vendor/autoload.php';

$loop = Factory::create();

$server = new Server(function (ServerRequestInterface $request) {
    return new Response(
        200,
        array(
            'Content-Type' => 'text/plain'
        ),
        "Hello world\n"
    );
});

$t1 = time();

\React\Stream\Util::pipe(
    new ReadableResourceStream(fopen("C:\Downloads\Rogue One A Star Wars Story 2016 x264 HDTS AAC(Deflickered)-DDR\sample-Rogue One A Star Wars Story 2016 x264 HDTS AAC(Deflickered)-DDR.mkv", 'r'), $loop),
    new WritableResourceStream(fopen('file', 'w'), $loop)
)->on('end', function() use ($t1) {
    echo 'finished in '.(time() - $t1);
});

$socket = new \React\Socket\Server(isset($argv[1]) ? $argv[1] : '0.0.0.0:0', $loop);
$server->listen($socket);

echo 'Listening on ' . str_replace('tcp:', 'http:', $socket->getAddress()) . PHP_EOL;

$loop->run();

I think it's better to allow usage of this library on Windows OS, even if it means it will be very slow. For some use cases speed in kilobytes might be enough.

[clue]

@ostrolucky I understand where you're coming from and agree that it's frustrating that this doesn't work as one may expect.

I agree that there are a number of use cases that may cope with this just fine. However, there are also a large number of use cases that simply don't work at all on Windows. For instance, consider the following example:

$stream = new ReadableResourceStream(STDIN, $loop);
$stream->on('data', function ($chunk) {
    echo $chunk;
});

$loop->addPeriodicTimer(1.0, function () {
    echo 'hi';
});

We've seen similar bug reports in the past where people would assume that this is a bug in our library, while in fact PHP simply does not support this at all on Windows due to platform restrictions. As such, we've decided to entirely block this via #46 and have not seen any complaints ever since.

As an alternative, we try to direct people to possible work arounds. You may be able to use https://github.com/reactphp/filesystem for filesystem I/O and/or to eventually spawn a child process via reactphp/child-process#9.

I hope this helps 👍

[ostrolucky]

Why blocking it entirely though? It makes sense to block this by default if people report this blocking behaviour as a bug, but how about giving the option to NOT throw that exception? Above was just a simple example. In real program I work with STDIN so I cannot use filesystem or child processes, nor I need accurate periodic timer.

[clue]

I'm not sold on the idea if it makes sense to support blocking behavior in an async program, as this may come over as "endorsing" or recommending, but I'm open to discuss what others think about this.

In your above example, you'll very likely experience some significant performance degradations if you're piping to a slow target device. In other words, does it make sense that your HTTP server is blocked only because another async stream is accessing the filesystem? Of course, there may be use cases where this is acceptable.

In the meantime, I would suggest simply copying the class and applying your patch locally? If you feel this is something that many people could benefit from, I encourage you to file a PR with added tests and documentation and we can review the consequences of such a change in greater detail 👍 Thank you!

[ostrolucky]

I actually use amphp/byte-stream which doesn't throw such exception. I tried your library only to check if you solved performance issue. I found you not only did not solve the issue, but prevented usage of streams on windows systems completely. So nothing to do for me, I'm just going to continue using amphp.

Poor performance is unfortunate, but my http server is meant for single http client only, so it's still better to run poorly (if another stream is running) than not run at all.

[clue]

Happy to hear your problem is resolved then 👍

In case anybody still feels like picking this up, I'm open for discussions and I encourage you to file a PR with added tests and documentation and we can review the consequences of such a change in greater detail 👍

[WyriHaximus]

Just a side note here that when using WSL on Windows you don't have these issues.