Skip to content

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

Merged
cboden merged 1 commit into
reactphp:masterfrom
astephens25:nonblocking-streamselectloop
Jul 21, 2013
Merged

[EventLoop] Changed StreamSelectLoop to use non-blocking behavior on tic...#204
cboden merged 1 commit into
reactphp:masterfrom
astephens25:nonblocking-streamselectloop

Conversation

@astephens25

Copy link
Copy Markdown

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

igorw commented Jun 14, 2013

Copy link
Copy Markdown
Contributor

Poke @nrk.

@nrk

nrk commented Jun 17, 2013

Copy link
Copy Markdown
Member

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
Copy Markdown
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

cboden commented Jul 8, 2013

Copy link
Copy Markdown
Member

Ran several tests, no issues.

Super Poke @nrk

@nrk

nrk commented Jul 15, 2013

Copy link
Copy Markdown
Member

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

cboden commented Jul 21, 2013

Copy link
Copy Markdown
Member

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