Repository navigation
Writing to an invalid stream creates an infinite loop of errors #2
Description
Activity
Part of this issue was on a write error Ratchet tried to send another message (lol) as pointed out in ratchetphp/Ratchet#137. That has been resolved in ratchetphp/Ratchet#200.
I still have a concern that if a write error occurs (attempting to write to a closed stream) and the developer does not take the appropriate action the resources will not be cleaned up...If
!is_resource()orfeof()are detected that will not change, I wonder if in addition to the error emission React should clean up the resources at that point.Would it make sense in this case for the stream to keep track of it's closed state? For instance:
class Stream extends EventEmitter implements ReadableStreamInterface, WritableStreamInterface { public $bufferSize = 4096; public $stream; protected $readable = true; protected $writable = true; protected $closing = false; protected $loop; protected $buffer; protected $closed = false; //<--- NEW public function __construct($stream, LoopInterface $loop) { $this->buffer->on('error', function ($error) { if($this->closed) { return; } $this->emit('error', array($error, $this)); $this->close(); }); } public function handleClose() { if (is_resource($this->stream)) { fclose($this->stream); } $this->closed = true; } }
In the context of React v0.4.1 in the Ratchet Websocket library (0.3.1) I encountered the following situation. When the buffer code somehow tries to write to an invalid stream (Buffer.php L83) it will emit the error "tried to write to invalid stream".
Buffer.php
The stream will receive this error & emits the error also. Somehow this triggers writing to the buffer again, which then emits the "tried to write to invalid stream" error again, thus creating an infinite loop.
Stream.php
Removing the " $this->emit('error', array($error, $this));" from the Stream __construct fixes the infinite loop, but it seems to be that not the real problem is fixed in this way. I have constructed a piece of code that enables me to reproduce this problem (via websockets). I will try to create a unit test that fails for this piece of code. But I hope this report helps as a start.