8000 [EventLoop] Changed StreamSelectLoop to use non-blocking behavior on tic... by astephens25 · Pull Request #204 · reactphp/reactphp · GitHub
[go: up one dir, main page]

Skip to content

[EventLoop] Changed StreamSelectLoop to use non-blocking behavior on tic... #204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 21, 2013
Merged

[EventLoop] Changed StreamSelectLoop to use non-blocking behavior on tic... #204

merged 1 commit into from
Jul 21, 2013

Conversation

astephens25
Copy link

I noticed the StreamSelectLoop does not exhibit the same non-blocking behavior as the LibEventLoop when using the tick() method. I modified it to never block on stream_select() nor sleep on pending timers when using the tick() method and provided a new protected method for the internal run() loop.

The existing test for the StreamSelectLoop is quite bare, so I did not modify it. If you would prefer, I will gladly add a test for this modification.

@igorw
Copy link
Contributor
igorw commented Jun 14, 2013

Poke @nrk.

@nrk
Copy link
Member
nrk commented Jun 17, 2013

Changes look good but I only did a code review without actually trying them (will do that later or tomorrow at worst).
It would be great to have a test checking for consistency of tick() among the various loop implementations.

@astephens25
Copy link
Author

I will see if I can get some time here in the next day or so to add a test for this particular scenario.

@cboden
Copy link
Member
cboden commented Jul 8, 2013

Ran several tests, no issues.

Super Poke @nrk

@nrk
Copy link
Member
nrk commented Jul 15, 2013

Here I am!
Finally had the chance to try things a bit with a couple of scripts and every seems indeed to be working fine.

@cboden cboden merged commit 5d53c66 into reactphp:master Jul 21, 2013
@cboden
Copy link
Member
cboden commented Jul 21, 2013

Thanks @astephens25 + @nrk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0