[BUG] Exit at startup failure on multiple workers

[Kludex]

Checklist

• The bug is reproducible against the latest release and/or master.
• There are no similar issues or pull requests to fix it yet.

Describe the bug

When the startup fails, either on reload mode or multiple workers, the main process is not terminated.

To reproduce

Application:

# test.py
from fastapi import FastAPI

app = FastAPI()

@app.on_event("startup")
def startup():
    raise Exception("Hi")

uvicorn test:app --workers 2
# or
uvicorn test:app --reload

Expected behavior

All the processes (parent and children) should be terminated.

Actual behavior

❯ uvicorn test:app --reload
INFO:     Uvicorn running on http://127.0.0.1:8000 (Press CTRL+C to quit)
INFO:     Started reloader process [40026] using watchgod
INFO:     Started server process [40028]
INFO:     Waiting for application startup.
ERROR:    Traceback (most recent call last):
  File "/home/marcelo/anaconda3/envs/uvicorn/lib/python3.8/site-packages/starlette/routing.py", line 540, in lifespan
    async for item in self.lifespan_context(app):
  File "/home/marcelo/anaconda3/envs/uvicorn/lib/python3.8/site-packages/starlette/routing.py", line 481, in default_lifespan
    await self.startup()
  File "/home/marcelo/anaconda3/envs/uvicorn/lib/python3.8/site-packages/starlette/routing.py", line 518, in startup
    handler()
  File "/home/marcelo/Development/./test.py", line 7, in startup
    raise Exception("Hi")
Exception: Hi

ERROR:    Application startup failed. Exiting.

After this log, it hangs forever. On the reload, it doesn't prevent the failure from happening multiple times if you modify a watched file. See:

❯ uvicorn test:app --reload            
INFO:     Uvicorn running on http://127.0.0.1:8000 (Press CTRL+C to quit)
INFO:     Started reloader process [40674] using watchgod
INFO:     Started server process [40676]
INFO:     Waiting for application startup.
ERROR:    Traceback (most recent call last):
  File "/home/marcelo/anaconda3/envs/uvicorn/lib/python3.8/site-packages/starlette/routing.py", line 540, in lifespan
    async for item in self.lifespan_context(app):
  File "/home/marcelo/anaconda3/envs/uvicorn/lib/python3.8/site-packages/starlette/routing.py", line 481, in default_lifespan
    await self.startup()
  File "/home/marcelo/anaconda3/envs/uvicorn/lib/python3.8/site-packages/starlette/routing.py", line 518, in startup
    handler()
  File "/home/marcelo/Development/./test.py", line 7, in startup
    raise Exception("Hi")
Exception: Hi

ERROR:    Application startup failed. Exiting.
WARNING:  WatchGodReload detected file change in '['/home/marcelo/Development/test.py']'. Reloading...
INFO:     Started server process [40734]
INFO:     Waiting for application startup.
ERROR:    Traceback (most recent call last):
  File "/home/marcelo/anaconda3/envs/uvicorn/lib/python3.8/site-packages/starlette/routing.py", line 540, in lifespan
    async for item in self.lifespan_context(app):
  File "/home/marcelo/anaconda3/envs/uvicorn/lib/python3.8/site-packages/starlette/routing.py", line 481, in default_lifespan
    await self.startup()
  File "/home/marcelo/anaconda3/envs/uvicorn/lib/python3.8/site-packages/starlette/routing.py", line 518, in startup
    handler()
  File "/home/marcelo/Development/./test.py", line 7, in startup
    raise Exception("Hi")
Exception: Hi

ERROR:    Application startup failed. Exiting.

In the case of multiple workers, it just hangs forever.

Environment

• OS / Python / Uvicorn version: Running uvicorn 0.14.0 with CPython 3.8.10 on Linux

[humrochagf]

Oh, nice to know, I bumped into this issue while writing some tests to improve the coverage, I wasn't sure it was something expected or a bug, but I added a note to check it later when I have more time.

This change will probably require a minor bump instead of a patch, since someone can be relying on this hanging behaviour at their daemons. Even though the app isn't really up, careless configured daemons can peak processing by infinity loops of app reboots.

In my tests, I noticed it hangs even sooner at the startup process, for example, if you miss-configure the app.

It fails to load the app and hangs instead of exiting:

➜ uvicorn test --workers 2 
INFO:     Uvicorn running on http://127.0.0.1:8000 (Press CTRL+C to quit)
INFO:     Started parent process [15068]
ERROR:    Error loading ASGI app. Import string "test" must be in format "<module>:<attribute>".
ERROR:    Error loading ASGI app. Import string "test" must be in format "<module>:<attribute>".

The same for the reload case:

➜ uvicorn test --reload   
INFO:     Will watch for changes in these directories: ['/home/user/tests/server']
INFO:     Uvicorn running on http://127.0.0.1:8000 (Press CTRL+C to quit)
INFO:     Started reloader process [15234] using watchgod
ERROR:    Error loading ASGI app. Import string "test" must be in format "<module>:<attribute>".

[humrochagf]

More info on the case, on my tests passing the wrong name the workers dies at the following lines of the configuration loading:

https://github.com/encode/uvicorn/blob/master/uvicorn/config.py#L449-L451

The problem with crashes that happen inside the configuration loading is that the lifetime communication with the parent process isn't set up at this moment, so the parent never gets notified.

Trying to make some notifications after the setup of the lifetime doesn't seem to have much effect either:

https://github.com/encode/uvicorn/blob/master/uvicorn/server.py#L77-L79

Do you know the proper way to communicate a shutdown event to the parent process? I couldn't find a good IPC example at the codebase to try out in this situation.

[Kludex]

I would check how gunicorn and hypercorn are doing it.

[humrochagf]

For more info on the case, I checked both Hypercorn and Gunicorn. In Hypercorn it seems to have a similar issue to the one described here.

For Gunicorn it works perfectly, but there's a difference in the way they handle the workers.
Gunicorn has an Arbiter that is responsible to manage and keep the workers alive, and it maintains a tight control of the worker's state.

One thing that could be brought from there is to make sure all lifespan setup happens during the spawning process of the worker instead of having parts of its loading happening during the call to serve. That way you make sure you have a communication line with the worker set up to be able to close the parent if something happens during the startup.

[Kludex]

The reload part is fixed, but not the multiple workers one.

[danieldaeschle]

Is there any update on this? I have the same problem when the app crashes, Kubernetes does not recognize it and also does not restart it.

Maybe a workaround?

[Kludex]

The fix on the reload is on máster, we need to release it.

[danieldaeschle]

I need it for multiple workers. I don't use reload on production.