Skip to content
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

Don't try and select until after ready events have been processed #159

Merged
merged 4 commits into from
Jan 28, 2017

Conversation

swfiua
Copy link

@swfiua swfiua commented Jan 20, 2017

Previous attempts to solve #75 included moving _init_loopback_task outside the main run() loop.

This fails to solve the problem because new tasks are simply added to the ready queue and that is not processed until the selector is called to look for new events.

This fixes #75 by moving the selector code and subsequent processing of events to the end of the run loop.

It does indeed solve the problem on windows, so far without an increase in stinging bat density.

It should be noted I'm quite out of my depth at this point, so proceed with caution!

@dabeaz
Copy link
Owner

dabeaz commented Jan 20, 2017

Honestly not sure how I feel about moving that code to the end. Almost wondering if we can fix this with some kind of simple check. Or maybe even just an exception handler (catch the offending exception when it's empty and move on).

@swfiua
Copy link
Author

swfiua commented Jan 20, 2017

The way I look at this, it is an infinite loop, so the fix only really has an effect on the first time round the loop -- it is an infinite cycle of ".. events, sleepers, ready ... ".

So the only effect of moving the code is that sleepers and ready stuff now get processed the first time round the loop, before the selector gets called, whereas in the past selectors were done first.

Now, because of the way curio works, the first time round the loop (on unix systems) an empty list of events will be returned, so the first iteration of the loop no events are processed anyway.

So the fix doesn't really change anything except avoiding calling selector_select when the selector is empty.

Plan B would be to do something like this at the top of the loop:

   if selector._readers or selector._writers:
        events = selector_select(timeout)
  else:
        events = []

Note that this has the same (practical) result as moving the selector code to the end of the loop, since the first time round the loop the selector is always empty, so no event code gets executed anyway until the sleepers and ready stuff have been processed the first time round.

@swfiua
Copy link
Author

swfiua commented Jan 26, 2017

This version just catches the OSError exception from selector_select call with nothing to select.

@dabeaz
Copy link
Owner

dabeaz commented Jan 28, 2017

Going to merge this. I had a thought though. Instead of logging an error message, what if the exception handler merely slept for the specified timeout period? With that, maybe the loopback task wouldn't be necessary.

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.

Windows 10, OSError: [WinError 10022] An invalid argument was supplied
2 participants