uvicorn
uvicorn copied to clipboard
Fix shutdown event on Windows in reloader
On Windows, using Process.terminate()
will immediately kill the child process so that the asgi event shutdown cannot be triggered in reloader. This PR is going to implement a graceful exit of child process.
For reference, what the description says is written here: https://docs.python.org/3/library/multiprocessing.html#multiprocessing.Process.terminate
Without the reload flag, does it run the shutdown event?
Without the reload flag, does it run the shutdown event?
In my tests, It seems the shutdown event is set without the reload flag, since there is no SpawnProcess.terminate()
in the exit signal handler of main process
Can we do something like https://stackoverflow.com/a/47314406/13087720 here?
Can we do something like https://stackoverflow.com/a/47314406/13087720 here?
After digging into multiprocessing
module, I found that it passes a 0 to _winapi.CreateProcess
as creationflags value and can't be modified outside.
https://github.com/python/cpython/blob/3.10/Lib/multiprocessing/popen_spawn_win32.py#L70-L79
with open(wfd, 'wb', closefd=True) as to_child:
# start process
try:
hp, ht, pid, tid = _winapi.CreateProcess(
python_exe, cmd,
None, None, False, 0, env, None, None)
_winapi.CloseHandle(ht)
except:
_winapi.CloseHandle(rhandle)
raise
While subprocess
seems only have support for command executions (https://stackoverflow.com/questions/2046603/is-it-possible-to-run-function-in-a-subprocess-without-threading-or-writing-a-se), it's quite hard to use it instead.
@StarHeartHunt If you have time, it would be cool to add a test for the changes, and fix the pipeline. :eyes: :pray:
@Kludex i have windows! :)
Ok... And this PR works there? :eyes:
A test is still needed for Windows (
pytest.mark.skipif
should help), and I need some people with Windows to help me here. π
Tried getting a subprocess in pytest but got an EOFError: run out of input
. I need some help from people familiar with writing test scripts on cross-module cases like this.π
A test is still needed for Windows (
pytest.mark.skipif
should help), and I need some people with Windows to help me here. eyesTried getting a subprocess in pytest but got an
EOFError: run out of input
. I need some help from people familiar with writing test scripts on cross-module cases like this.open_hands
We actually already have a test for it, it's the one you've modified :sweat_smile:
So... Someone can confirm this works? Someone from the team has a way to test this locally? @encode/maintainers
pytest
works on my Windows 11 machine
EDIT: I meant all tests pass
pytest
works on my Windows 11 machineEDIT: I meant all tests pass
By "testing" I mean manually checking that it works on Windows. We have windows on our pipeline, so I know the tests are passing on Windows :eyes:
Works on my machine
app.py
from contextlib import asynccontextmanager
from typing import AsyncGenerator
from fastapi import FastAPI
from starlette.applications import Starlette
@asynccontextmanager
async def lifespan(app: Starlette) -> AsyncGenerator[None, None]:
print("START")
yield None
print("SHUTDOWN")
app = FastAPI(lifespan=lifespan)
Install deps
$ pip install fastapi
$ pip install git+https://github.com/StarHeartHunt/uvicorn@bug/fix-shutdown-event-on-windows-in-hot-reloader --force-reinstall
Run
$ uvicorn app:app --reload
INFO: Will watch for changes in these directories: ['C:\\Users\\aceus\\perso\\tmp']
INFO: Uvicorn running on http://127.0.0.1:8000 (Press CTRL+C to quit)
INFO: Started reloader process [2108] using StatReload
INFO: Started server process [19776]
INFO: Waiting for application startup.
START
INFO: Application startup complete.
WARNING: StatReload detected changes in 'app.py'. Reloading...
INFO: Shutting down
INFO: Waiting for application shutdown.
SHUTDOWN
INFO: Application shutdown complete.
INFO: Finished server process [19776]
INFO: Started server process [8672]
INFO: Waiting for application startup.
START
INFO: Application startup complete.
INFO: Shutting down
INFO: Waiting for application shutdown.
SHUTDOWN
INFO: Application shutdown complete.
INFO: Finished server process [8672]
INFO: Stopping reloader process [2108]
Hi, this pr works with our application and the log is here: https://github.com/nonebot/nonebot2/issues/1654#issuecomment-1482589615
Do we need to do the same for the Multiprocess
class?
Do we need to do the same for the
Multiprocess
class?
I think there is no need to apply this patch to Multiprocess
class. Maybe #1872 is relevant to #1909, we can have a further triage after this PR is mergedπ
I'm not sure if this PR will meet the issues in #684. I can't test because I'm using a MacBook right now, it would be nice if someone could try it.
I'm not sure if this PR will meet the issues in #684. I can't test because I'm using a MacBook right now, it would be nice if someone could try it.
Using the repro by @abersheeran in it, I tried, and it works well.
β uvicorn app:app --reload
INFO: Will watch for changes in these directories: ['C:\\Users\\ava\\Projects\\uvicorn-test']
INFO: Uvicorn running on http://127.0.0.1:8000 (Press CTRL+C to quit)
INFO: Started reloader process [2704] using StatReload
INFO: Started server process [12708]
INFO: Waiting for application startup.
START
ERROR: Traceback (most recent call last):
File "C:\Users\ava\AppData\Local\Programs\Python\Python312\Lib\site-packages\starlette\routing.py", line 677, in lifespan
async with self.lifespan_context(app) as maybe_state:
File "C:\Users\ava\AppData\Local\Programs\Python\Python312\Lib\contextlib.py", line 204, in __aenter__
return await anext(self.gen)
^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\ava\Projects\uvicorn-test\app.py", line 10, in lifespan
afhajdhfa
NameError: name 'afhajdhfa' is not defined
ERROR: Application startup failed. Exiting.
WARNING: StatReload detected changes in 'app.py'. Reloading...
INFO: Started server process [11988]
INFO: Waiting for application startup.
START
INFO: Application startup complete.
I'm not sure if this PR will meet the issues in #684. I can't test because I'm using a MacBook right now, it would be nice if someone could try it.
Using the repro by @abersheeran in it, I tried, and it works well.
β uvicorn app:app --reload INFO: Will watch for changes in these directories: ['C:\\Users\\ava\\Projects\\uvicorn-test'] INFO: Uvicorn running on http://127.0.0.1:8000 (Press CTRL+C to quit) INFO: Started reloader process [2704] using StatReload INFO: Started server process [12708] INFO: Waiting for application startup. START ERROR: Traceback (most recent call last): File "C:\Users\ava\AppData\Local\Programs\Python\Python312\Lib\site-packages\starlette\routing.py", line 677, in lifespan async with self.lifespan_context(app) as maybe_state: File "C:\Users\ava\AppData\Local\Programs\Python\Python312\Lib\contextlib.py", line 204, in __aenter__ return await anext(self.gen) ^^^^^^^^^^^^^^^^^^^^^ File "C:\Users\ava\Projects\uvicorn-test\app.py", line 10, in lifespan afhajdhfa NameError: name 'afhajdhfa' is not defined ERROR: Application startup failed. Exiting. WARNING: StatReload detected changes in 'app.py'. Reloading... INFO: Started server process [11988] INFO: Waiting for application startup. START INFO: Application startup complete.
Thanks.
Once rebased, we can merge it. :pray:
Once again, thanks @abersheeran :pray:
Did this caused https://github.com/encode/uvicorn/discussions/1977 ?
Did this caused #1977 ?
It seems couldn't be reproduced on my Windows 10 laptop. I think it's not relevant to #1909 or #1584
Use case and test script, by randomly adding and removing lines to trigger reload and see
from contextlib import asynccontextmanager
from typing import Union
from fastapi import FastAPI
@asynccontextmanager
async def lifespan(app: FastAPI):
print("startup")
yield
print("shutdown")
app = FastAPI(lifespan=lifespan)
@app.get("/")
def read_root():
return {"Hello": "World"}
@app.get("/items/{item_id}")
def read_item(item_id: int, q: Union[str, None] = None):
return {"item_id": item_id, "q": q}
Start uvicorn by the following command
uvicorn main:app --reload
And the environment
PS D:\PythonWorkspace\reloadtest> pip list
Package Version Editable project location
------------------ --------- ----------------------------------
1223 0.0.1 D:\PythonWorkspace\reloadtest
anyio 3.6.2
arrow 1.2.3
asds 0.1.0 D:\PythonWorkspace\reloadtest\asds
binaryornot 0.4.4
cashews 5.3.1
certifi 2022.12.7
chardet 5.1.0
charset-normalizer 3.1.0
click 8.1.3
colorama 0.4.6
cookiecutter 2.1.1
distlib 0.3.6
fastapi 0.95.0
filelock 3.10.3
h11 0.14.0
httpcore 0.16.3
httpx 0.23.3
idna 3.4
importlib-metadata 6.1.0
Jinja2 3.1.2
jinja2-time 0.2.0
loguru 0.6.0
MarkupSafe 2.1.2
multidict 6.0.4
nb-cli 1.0.5
nonebot2 2.0.0rc3
noneprompt 0.1.9
pip 23.0.1
platformdirs 2.6.2
prompt-toolkit 3.0.38
pydantic 1.10.7
pyfiglet 0.8.post1
pygtrie 2.5.0
python-dateutil 2.8.2
python-dotenv 1.0.0
python-slugify 8.0.1
PyYAML 6.0
requests 2.28.2
rfc3986 1.5.0
setuptools 67.6.0
six 1.16.0
sniffio 1.3.0
starlette 0.26.1
text-unidecode 1.3
typing_extensions 4.5.0
urllib3 1.26.15
uvicorn 0.22.0
virtualenv 20.17.1
watchfiles 0.18.1
wcwidth 0.2.6
win32-setctime 1.1.0
yarl 1.8.2
zipp 3.15.0