Fix OSError occurs when workers > 1 #1317

[smitchell6879]

Should fix OSError

[smitchell6879]

I am using python 3.8 with 3 workers on windows 10. Is there any more info on why it failed the test?

[Kludex]

linter https://github.com/encode/uvicorn/runs/5849365812?check_suite_focus=true

[smitchell6879]

linter https://github.com/encode/uvicorn/runs/5849365812?check_suite_focus=true

No log available. So whats my next step?

[Kludex]

Logs are available...

[euri10]

linter https://github.com/encode/uvicorn/runs/5849365812?check_suite_focus=true

No log available. So whats my next step?

you can run the lint script in the scripts folder

[smitchell6879]

linter https://github.com/encode/uvicorn/runs/5849365812?check_suite_focus=true

No log available. So whats my next step?

you can run the lint script in the scripts folder

So I finally figured out how to run the lint script, so it should be good now. Sorry for the my ignorance.

[Kludex]

Suggested change

	"The --reload flag should not be \
	used in production on Windows."
	"The --reload flag should not be "
	"used in production on Windows."

[smitchell6879]

Is there something else I need to fix or does this look good?

[Kludex]

I'm going to check this tonight.

[Kludex]

This is basically a revert of #1257, which was in place to solve other issues...

[euri10]

ok this one I dont get it, what is the purpose of the PR, please describe the issue

[smitchell6879]

ok this one I dont get it, what is the purpose of the PR, please describe the issue

The issue is #1317. On windows when running multiple workers without reload true you would get a oserror. By writing it this way you still provide the warning if reload is true but you no longer get the oserror.

[Kludex]

This is basically a revert of #1257, which was in place to solve other issues...

Is my statement false?

[smitchell6879]

I am assuming no your statement is not false but I am unsure of what issues the change fixed when it also created a new issue. I am currently home sick when I feel better I will dig into the code more to try to figure out the issue that this change fixed and see if I can recreate it.

[Kludex]

Hope you get better soon. 😟

[smitchell6879]

So I have linked the log to show the errors I am getting without the above change. I am using a custom logger so [Backend] is actually the uvicorn channel. https://pastebin.com/cXQuG3SH
Reviewing the code it seems to me without this change you are not setting the asyncio.set_event_loop_policy at all unless you have the reload parameter set to True in which case it shouldn't matter as much because you go back you only 1 worker. As you see in the paste bin I have reload set to False and workers set to 5 I forgot to add the python version which is 3.8 .
I am not going to act like I full understand asyncio.set_event_loop_policy But after digging around the net I did find "tornadoweb/tornado#2608" so I have updated to their teams fix. This does not give me any errors.

[euri10]

I think #1454 is a better alternative, ymmv

[smitchell6879]

Thats cool too as long as it fixes the issue.

[smitchell6879]

Has anyone had a chance to review either of these and decide which fix you prefer?

[smitchell6879]

Any chance for a update on this issue?

[Kludex]

This solution reverts previous changes. As mentioned on #1220 (comment), we should use the ProactorEventLoop whenever is possible. The case on which we don't want to use it is when we have either reload being True or the value of workers being higher than 1.

The ideal solution here would be to just make that check. I think we can even check if we're running on a child process instead of passing the reload/workers around.

As a note, the warning saying that reload should not be used in production is misplaced. It's not because we got to that point that we are in production. The warning should be removed.

In any case, I'll be closing this because of what I said on the first paragraph. Let's continue this on #1454 . Thanks for the efforts here @smitchell6879 ! 🙇

References:

• uvicorn reload fails on Windows and Python 3.8 #529
• Is there a reason for overriding the default asyncio policy on Windows? #1220
• OSError occurs when workers > 1 #1317
• Fix OSError: [WinError 87] The parameter is incorrect #1454
• Iocp drop #535