PollIOLoop.stop() aborts the current event loop iteration. #672

Closed
jordens opened this Issue Jan 24, 2013 · 6 comments

2 participants

@jordens

THe docstring to stop() says "Stop the loop after the current event loop iteration is complete." But PollIOLoop.stop() aborts the eventloop mid flight before any polling happens. This makes it impossible to do the following to have the loop do a single iteration as no IO is done:

loop.add_timeout(0, loop.stop)
loop.start()

I submitted this for pyzmq which copied the eventloop and was asked to submit it here:.

zeromq/pyzmq#297

If this is not the right solution, how can I run a single iteration of the loop?

@jordens

The following works but is ugly:

loop.add_timeout(0, lambda: loop.add_callback(loop.stop))
@bdarnell
tornadoweb member

The "current iteration" is not well-defined and probably isn't the best way to express what IOLoop.stop does. The idea is that stop() causes the loop to stop as soon as practical; the "after the current iteration" language is there to point out that some stuff may still happen before the loop is stopped, not to guarantee that something (i.e. another poll, or the results of that poll) will happen.

We don't have a way to run for exactly one iteration, and even if we did I'm not sure you'd want to - lots of basic operations are spread out over multiple "iterations" for various reasons (reentrancy concerns, starvation of IO vs CPU). Your best bet is probably to use add_timeout to schedule a stop a small but non-zero time in the future.

@jordens

Single iterations of the loop are useful if you want to integrate this loop into another eventloop and iterate it in a timed fashion. IPython is a user and does tries to do this (via pyzmq).
It appears to be an easily added feature (or easily fixed bug depending on how you see it) and would also implement the behavior as described in the docstring. If you want to exit the loop as soon as possible, you can add a cancel() that exits the loop earlier.
add_timeout with a finite delay is not reliable and blocks unneccessarily.

@bdarnell
tornadoweb member

If you run single iterations on a timer Tornado will not be very responsive. For example, when an HTTP request comes in it takes at least two iterations before any RequestHandler code is run. The second iteration will not block, so if you run the loop for a few milliseconds instead of one iteration it will be given a chance to run immediately.

In general, when I have multiple event loops in a process I prefer to just give each one its own thread (unless deep (fully-responsive) integration is possible, like what we now have between tornado and twisted). If you interleave two event loops in the same thread responsiveness will suffer no matter what you do, so I'd recommend just giving the secondary event loop a time budget every invocation rather than trying to second-guess its iteration policy.

@jordens

Sure. There are many ways to get this done. However, I submitted this issue not about how to best run two eventloops but about the docstring and the code contradicting each other and the inability of running a single iteration of the loop. Even in the case you mention, a very helpful feature would be to run the eventloop until there are no callbacks and the fds are empty.

@bdarnell
tornadoweb member

For stop(), I think the answer is to fix the docstring to be less specific. I might be able to be convinced to add some sort of non-blocking run method (which would be defined loosely enough that it wouldn't commit to exactly one iteration. For the current implementations it should probably loop until the poller returned no events, although hypothetical implementations integrated with other event loops may not be able to support this concept without falling back to a timer-based approach)

@bdarnell bdarnell added a commit that closed this issue Feb 9, 2013
@bdarnell bdarnell Clarify docstring of IOLoop.stop.
The mention of "after the current iteration" was meant to emphasize that the
stop was not instantaneous, but it could have also been read as a guarantee
that one more poll call would take place.

Closes #672.
280c286
@bdarnell bdarnell closed this in 280c286 Feb 9, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment