All streams are duplex (r/w) by default and can thus keep the loop running

[clue]

The Stream class assumes all stream resources are duplex (r/w) streams, while some may only be readable (e.g. STDIN) or only be writable (e.g. STDOUT).

I'm not considering this to be a bug because this happens to be documented behavior, albeit probably unexpected for many newcomers.

The following gist highlights this problem:

$loop = \React\EventLoop\Factory::create();

// by default, this will open a duplex stream (r/w)
$out = new \React\Stream\Stream(STDOUT, $loop);

// this line is completely optional - only here to make this problem more obvious
// the same issue happens without writing any data
$out->write("hello\n");

$loop->run();

The average user would probably assume this to write out some data (if at all) and then exit.

However, this example does not terminate ever. The Stream class assumes the STDOUT stream is a duplex stream resource and thus keeps waiting for incoming data infinitely.

In the above example, this problem can be worked around by pausing the stream anywhere like this:

$out->pause();

I'm going to argue that this is counter-intuitive and confusing (what exactly is being paused here?).

Note that a similar problem happens with stream resources that are read-only (e.g. STDIN) though it's less apparent.

This ticket aims to serve as a reminder for this hardly documented behavior, for the reference and in order to discuss possible alternatives.

[clue]

Possible solution A: Clear, explicit documentation.

This is probably the least work. Some documentation could certainly help here – though TBH I'm not considering this a solution.

[clue]

Possible solution B: Examine stream resource meta data and automatically pause write-only streams.

This would involve some sorcery with stream_get_meta_data() and fopen() flags.

This would probably solve the 80% case where we can automatically stop reading from write-only streams.

This shouldn't be too much work and would probably resolve this for most cases. I'm not sure how reliable this is going to be, so we would likely need some tests to back this up.

However, the user could potentially still issue some fancy things like:

// oh really?!
assert($out->isReadable() === true);

$out->on('data', function () {
    // will never be called anyway
});

// what is this supposed to do in the first place?
$out->pipe($another);

// back to the initial issue…
$out->resume();

As such, while this may be considered a good default for now, I'm tempted to consider this a temporary workaround only.

[clue]

Possible solution C: Add DuplexStreamResource (currently Stream), ReadableStreamResource and WritableStreamResource classes and make this an explicit choice.

The user would then have to use code like:

$in = new ReadableStreamResource(STDIN);
$out = new WritableStreamResource(STDOUT);

assert($in->isReadable() === true);
assert($out->isWritable() === true);

$in->pipe($out);

And

$stream = new DuplexStreamResource(fopen('php://memory', 'r+'));
// or possibly some future factory method (to be discussed in a separate ticket):
$stream = DuplexStreamResource::fopen('php://memory', 'r+'));

This would align with our interfaces: DuplexStreamInterface, ReadableStreamInterface and WritableStreamInterface.

Due to the available interface, this would also prevent illogical method calls like these:

// neither of these methods are defined
$in->isWritable();
$in->write();
$in->end();
$in->pipe($stream);
$in->on('data', function () { });

$out->isReadable();

This change would (obviously) involve some more work and a BC break (which I'm okay with, personally).

[semmel]

Why isn't calling $in->close() enough?
This seems to work just fine:

$loop = React\EventLoop\Factory::create();
$in = new \React\Stream\Stream(STDIN, $loop);

// program exit
$heartBeatTimer = $loop->addPeriodicTimer(0.01, function() {
    pcntl_signal_dispatch(); 
});
pcntl_signal(SIGTERM, function() use ($in, $heartBeatTimer){
    $heartBeatTimer->cancel();
    // Remove the stdin stream from the run loop as well
    $in->close();
});
// use the stdin stream
$in->on('data', function(string $newLine){
    // handle the input line
});

$loop->run();

[clue]

@semmel Not sure I get what you're suggesting here, care to elaborate? :)

The loop does (correctly) terminate if it has nothing to do. It's just that an (unmindful) call to new Stream(STDOUT, $loop) keeps the loop busy forever.

[semmel]

@clue Yeah I probably misunderstood completely this issue 😳

My understanding is that it is just natural to this library to keep the loop running while there are streams open and connected to the loop.
E.g. when a stream via TCP connection is dropped by the remote end of course I expect $loop->run() to cease blocking the script and continue, but on the other hand a STDIN or STDOUT stream is never dropped by to other end, is it? So unless I do that myself I think it's ok for both of them to keep the loop running.

What I probably got wrong is

The Stream class assumes the STDOUT stream is a duplex stream resource and thus keeps waiting for incoming data infinitely.
... this problem can be worked around by pausing the stream anywhere like this: $out->pause();

thinking that in order to drop out of the loop I had to call $in->pause(), but I found that $in->close(); is just ok.

So nevermind, I just didn't get what the bug is about while I was just looking for an example to manage a STDIN stream with React.

[codedokode]

You could also implement both solutions, so the user can either choose stream type explicitly (new ReadableStream) or use a factory ($stream = SomeStreamFactory::createFromResource($fileHandle)).

Having all streams duplex by default is confusing.

@semmel

but on the other hand a STDIN or STDOUT stream is never dropped by to other end, is it? So unless I do that myself I think it's ok for both of them to keep the loop running.

It would be more convenient if when there is no more data to write to STDOUT (for example, a stream has empty buffer and no data were added on drain event) it would remove itself from the loop and put back when write() is called again.

[codedokode]

but on the other hand a STDIN or STDOUT stream is never dropped by to other end, is it?

Actually there can be EOF on STDIN when it is redirected to a fileand file has been read completely or when a user presses Ctrl + D in console.