Skip to content

Conversation

@iwai
Copy link
Contributor

@iwai iwai commented Jan 30, 2015

Fixed PHP Warning: fread(): 294 is not a valid stream resource in .../react/stream/src/Stream.php on line 127

Call Stream::resume in https://github.com/reactphp/stream/blob/master/src/Stream.php#L40

Perhaps this problem, Read Stream Listener is not successfully deleted when you close the connection, warning came out after a while.
Code to strictly reproduce is difficult, but it was caused by my production.

This pull request, because the first time that for me, there may be a problem with the manners, but please forgive me. and i'm afraid my expressions may be rude or hard to read, because I'm not so good at English.

@clue
Copy link
Owner

clue commented Feb 23, 2015

Thanks for providing this PR, very much appreciated! 👍

Could you please describe the necessary steps to reproduce this?

Given the current code flow, there's little chance that this method call will actually be the cause of any error message. The Client ctor will only be called by the Factory after creating a Stream via the Connector. Each Stream instance already invokes resume() during its ctor, and calling it immediately after in the Client ctor is essentially a NOOP. As such, I'm struggling to see how this could cause your error message.

That being said, there's current no test case that shows that this method is required in first place, so I'm okay with removing it altogether as per your PR :)

@iwai
Copy link
Contributor Author

iwai commented Feb 24, 2015

Thank you reply.

My working this repository, https://github.com/iwai/react-resque
Still, code but there are certain things and the difference in local, now, I am working in this repository.
Because while working, if you come to work, do you may be described again?

@clue
Copy link
Owner

clue commented Feb 24, 2015

https://github.com/iwai/react-resque

Nice, I'll certainly keep an eye on this :) I've also started working on a similar project (php-resque-react) a few months ago.

Code to strictly reproduce is difficult, but it was caused by my production.

Please let me know if you manage to reproduce this. If you need any help, post in this issue or check out #reactphp on irc.freenode.org, thanks!

@clue
Copy link
Owner

clue commented Mar 5, 2016

PHP Warning: fread(): 294 is not a valid stream resource in .../react/stream/src/Stream.php on line 127

See also reactphp/stream#18 which will be addressed via reactphp/stream#40.

@clue clue changed the title Bugfix: duplicate resume call. Remove unneeded resume() call May 16, 2016
@clue
Copy link
Owner

clue commented May 16, 2016

Alright, let's get this in :shipit:

To be clear: I do not consider this a bug fix, however I see no need to keep this method call because it should be a NOOP anyway.

@clue clue merged commit d00794d into clue:master May 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants