From 05f5367c2b0a1d5ee1a823b75e3b7df8f390b432 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 1 Jul 2016 13:42:50 +0200 Subject: [PATCH 1/3] Enforce using non-blocking I/O Reading from a stream should *never* block the loop. This is known to be an issue for process pipes on Windows. --- src/Stream.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Stream.php b/src/Stream.php index a23a4a8..f6df26b 100644 --- a/src/Stream.php +++ b/src/Stream.php @@ -40,7 +40,11 @@ public function __construct($stream, LoopInterface $loop, WritableStreamInterfac throw new InvalidArgumentException('First parameter must be a valid stream resource'); } - stream_set_blocking($this->stream, 0); + // this class relies on non-blocking I/O in order to not interrupt the event loop + // e.g. pipes on Windows do not support this: https://bugs.php.net/bug.php?id=47918 + if (stream_set_blocking($this->stream, 0) !== true) { + throw new \RuntimeException('Unable to set stream resource to non-blocking mode'); + } // Use unbuffered read operations on the underlying stream resource. // Reading chunks from the stream may otherwise leave unread bytes in From da790b85745681636b0d15dae1e8788925932fc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Wed, 8 Mar 2017 11:13:21 +0100 Subject: [PATCH 2/3] Test rejecting streams that do not support non-blocking mode --- src/Buffer.php | 6 ++++++ src/Stream.php | 8 ++++---- tests/BufferTest.php | 18 +++++++++++++++++- tests/EnforceBlockingWrapper.php | 20 ++++++++++++++++++++ tests/StreamTest.php | 19 ++++++++++++++++++- 5 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 tests/EnforceBlockingWrapper.php diff --git a/src/Buffer.php b/src/Buffer.php index 614edb3..af4b84e 100644 --- a/src/Buffer.php +++ b/src/Buffer.php @@ -21,6 +21,12 @@ public function __construct($stream, LoopInterface $loop) throw new \InvalidArgumentException('First parameter must be a valid stream resource'); } + // this class relies on non-blocking I/O in order to not interrupt the event loop + // e.g. pipes on Windows do not support this: https://bugs.php.net/bug.php?id=47918 + if (stream_set_blocking($stream, 0) !== true) { + throw new \RuntimeException('Unable to set stream resource to non-blocking mode'); + } + $this->stream = $stream; $this->loop = $loop; } diff --git a/src/Stream.php b/src/Stream.php index f6df26b..02ded13 100644 --- a/src/Stream.php +++ b/src/Stream.php @@ -35,14 +35,13 @@ class Stream extends EventEmitter implements DuplexStreamInterface public function __construct($stream, LoopInterface $loop, WritableStreamInterface $buffer = null) { - $this->stream = $stream; - if (!is_resource($this->stream) || get_resource_type($this->stream) !== "stream") { + if (!is_resource($stream) || get_resource_type($stream) !== "stream") { throw new InvalidArgumentException('First parameter must be a valid stream resource'); } // this class relies on non-blocking I/O in order to not interrupt the event loop // e.g. pipes on Windows do not support this: https://bugs.php.net/bug.php?id=47918 - if (stream_set_blocking($this->stream, 0) !== true) { + if (stream_set_blocking($stream, 0) !== true) { throw new \RuntimeException('Unable to set stream resource to non-blocking mode'); } @@ -53,13 +52,14 @@ public function __construct($stream, LoopInterface $loop, WritableStreamInterfac // This does not affect the default event loop implementation (level // triggered), so we can ignore platforms not supporting this (HHVM). if (function_exists('stream_set_read_buffer')) { - stream_set_read_buffer($this->stream, 0); + stream_set_read_buffer($stream, 0); } if ($buffer === null) { $buffer = new Buffer($stream, $loop); } + $this->stream = $stream; $this->loop = $loop; $this->buffer = $buffer; diff --git a/tests/BufferTest.php b/tests/BufferTest.php index 284a842..5c19669 100644 --- a/tests/BufferTest.php +++ b/tests/BufferTest.php @@ -20,13 +20,29 @@ public function testConstructor() /** * @covers React\Stream\Buffer::__construct - * @expectedException InvalidArgumentException */ public function testConstructorThrowsIfNotAValidStreamResource() { $stream = null; $loop = $this->createLoopMock(); + $this->setExpectedException('InvalidArgumentException'); + new Buffer($stream, $loop); + } + + /** + * @covers React\Stream\Buffer::__construct + */ + public function testConstructorThrowsExceptionIfStreamDoesNotSupportNonBlocking() + { + if (!in_array('blocking', stream_get_wrappers())) { + stream_wrapper_register('blocking', 'React\Tests\Stream\EnforceBlockingWrapper'); + } + + $stream = fopen('blocking://test', 'r+'); + $loop = $this->createLoopMock(); + + $this->setExpectedException('RuntimeException'); new Buffer($stream, $loop); } diff --git a/tests/EnforceBlockingWrapper.php b/tests/EnforceBlockingWrapper.php new file mode 100644 index 0000000..91a89fb --- /dev/null +++ b/tests/EnforceBlockingWrapper.php @@ -0,0 +1,20 @@ +createLoopMock(); + $this->setExpectedException('InvalidArgumentException'); + new Stream('breakme', $loop); + } + + /** + * @covers React\Stream\Stream::__construct + */ + public function testConstructorThrowsExceptionIfStreamDoesNotSupportNonBlocking() + { + if (!in_array('blocking', stream_get_wrappers())) { + stream_wrapper_register('blocking', 'React\Tests\Stream\EnforceBlockingWrapper'); + } + + $stream = fopen('blocking://test', 'r+'); $loop = $this->createLoopMock(); - $conn = new Stream('breakme', $loop); + + $this->setExpectedException('RuntimeException'); + new Stream($stream, $loop); } /** From efdecb9638bace336bea7c811662c6072038307a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Thu, 9 Mar 2017 07:52:27 +0100 Subject: [PATCH 3/3] Mock stream resource casting in order to support HHVM --- tests/EnforceBlockingWrapper.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/EnforceBlockingWrapper.php b/tests/EnforceBlockingWrapper.php index 91a89fb..1cc15ab 100644 --- a/tests/EnforceBlockingWrapper.php +++ b/tests/EnforceBlockingWrapper.php @@ -2,6 +2,11 @@ namespace React\Tests\Stream; +/** + * Used to test dummy stream resources that do not support setting non-blocking mode + * + * @link http://php.net/manual/de/class.streamwrapper.php + */ class EnforceBlockingWrapper { public function stream_open($path, $mode, $options, &$opened_path) @@ -9,6 +14,11 @@ public function stream_open($path, $mode, $options, &$opened_path) return true; } + public function stream_cast($cast_as) + { + return false; + } + public function stream_set_option($option, $arg1, $arg2) { if ($option === STREAM_OPTION_BLOCKING) {