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

Is there a reason for overriding the default asyncio policy on Windows? #1220

Closed
2 tasks
polys opened this issue Oct 18, 2021 · 4 comments
Closed
2 tasks

Is there a reason for overriding the default asyncio policy on Windows? #1220

polys opened this issue Oct 18, 2021 · 4 comments

Comments

@polys
Copy link

polys commented Oct 18, 2021

Checklist

  • There are no similar issues or pull requests for this yet.
  • I discussed this idea on the community chat and feedback is positive.

Is your feature related to a problem? Please describe.

While developing on Windows, my app being served by uvicorn keeps raising ValueError: too many file descriptors in select(), which I understand is related to the suboptimal asyncio.WindowsSelectorEventLoopPolicy.

Python 3.8 changed the default to ProactorEventLoop on Windows but your code seems to override this.

Is there a reason for this?

Describe the solution you would like.

Ideally, all the too many file descriptors in select() exceptions go away.

Describe alternatives you considered

Additional context

This isn't an issue at all when running my app in a Linux container.

@polys polys changed the title Is there a reason you're overriding the default asyncio policy? Is there a reason for overriding the default asyncio policy on Windows? Oct 18, 2021
@sekrause
Copy link

Looking at the commit history the ProactorEventLoop was disabled to fix issue #529.

It probably wasn't understood that using SelectorEventLoop basically makes uvicorn completely useless for scalable applications on Windows because you will be limited to only 512 socket connections: https://docs.python.org/3.10/library/asyncio-platforms.html#windows

So simply disabling the ProactorEventLoop (the only scalable event loop on Windows) is generally not a good idea.

@sekrause
Copy link

@polys For now you can work around this issue by starting uvicorn programmatically in your own loop, for example this will give you a ProactorEventLoop in Python >= 3.8:

from asyncio import get_event_loop, new_event_loop
from fastapi import FastAPI
from uvicorn import Config, Server

app = FastAPI()

@app.get("/")
async def read_main():
    return f'Running loop: {get_event_loop()}'

loop = new_event_loop()
config = Config(app=app)
server = Server(config)
loop.run_until_complete(server.serve())

However I still think that uvicorn should use the ProactorEventLoop whenever possible. For example if the --reload option is the problem (issue #529), only switch to the SelectorEventLoop when --reload is enabled on Windows, and issue a warning in the logs that --reload should never be used in production on Windows.

@polys
Copy link
Author

polys commented Oct 20, 2021

Thanks for the suggestion 😀

In my case, I'm only using Windows for development, so --reload is actually quite useful. The app runs in a Linux container in production.

My workaround while on Windows is to use Hypercorn, which seems to be happier. I've only tried this for a couple of days now but haven't seen any too many file descriptors in select() exceptions in situations where uvicorn was consistently blowing up.

I'm still very happy to use uvicorn in production — just wanted to understand why there's this asyncio override on Windows.

only switch to the SelectorEventLoop when --reload is enabled on Windows, and issue a warning in the logs that --reload should never be used in production on Windows.

That's a nice suggestion — I hadn't realised that that's the only reason for not using ProactorEventLoop.

@euri10
Copy link
Member

euri10 commented Nov 19, 2021

However I still think that uvicorn should use the ProactorEventLoop whenever possible. For example if the --reload option is the problem (issue #529), only switch to the SelectorEventLoop when --reload is enabled on Windows, and issue a warning in the logs that --reload should never be used in production on Windows.

happy to review a PR for this, should you want to tackle it open a new issue
your explanation as to why we changed the windows default is I think correct even if it was a long time ago >_)

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

No branches or pull requests

3 participants