[BUG] Received duplicated signal from Gunicorn

[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

Currently, when we send SIGINT to the gunicorn process that is running with uvicorn workers, the SIGINT is not propagated to the workers. On its turn, a SIGQUIT is sent to them. By the time of this issue, the reason for that decision is unclear, but you can follow this gunicorn issue to understand more.

The mentioned behavior has an effect on uvicorn, that I'm unsure how we should act. The effect is that when running gunicorn with uvicorn workers, the shutdown event doesn't happen. Which, in theory, is fine. As SIGINT and SIGQUIT, should be terminating the worker immediately, but that is not what is in place for standalone uvicorn i.e. SIGNIT triggers the shutdown event.

Well, all of what I mentioned above should actually be a separated issue, but I was not able to solve it without workarounds, that I'm unsure if we want them. The reason for that is the uvicorn worker receiving two signals when we send SIGINT to the gunicorn process, one is SIGINT, which is sent directly to the uvicorn worker and the other is the SIGQUIT which is sent by the gunicorn process.

As you see, even if we fix SIGQUIT by sending SIGINT, the issue is not solved i.e. uvicorn will receive double SIGINT, and we will force exit (via force_exit attribute).

To reproduce

Just create any ASGI app, with a shutdown event:

# test.py
from fastapi import FastAPI

app = FastAPI()

@app.on_event("shutdown")
async def shutdown():
    print("This will not be triggered")

Then run gunicorn with uvicorn workers:

gunicorn -k uvicorn.workers.UvicornWorker test:app

Feel free to press CTRL + C and you'll see this log:

❯ gunicorn -k uvicorn.workers.UvicornWorker test:app
[2021-07-10 20:08:01 +0200] [43323] [INFO] Starting gunicorn 20.1.0
[2021-07-10 20:08:01 +0200] [43323] [INFO] Listening at: http://127.0.0.1:8000 (43323)
[2021-07-10 20:08:01 +0200] [43323] [INFO] Using worker: uvicorn.workers.UvicornWorker
[2021-07-10 20:08:01 +0200] [43325] [INFO] Booting worker with pid: 43325
[2021-07-10 20:08:01 +0200] [43325] [INFO] Started server process [43325]
[2021-07-10 20:08:01 +0200] [43325] [INFO] Waiting for application startup.
[2021-07-10 20:08:01 +0200] [43325] [INFO] Application startup complete.
^C[2021-07-10 20:08:01 +0200] [43323] [INFO] Handling signal: int
[2021-07-10 20:08:01 +0200] [43325] [INFO] Shutting down
[2021-07-10 20:08:01 +0200] [43325] [INFO] Error while closing socket [Errno 9] Bad file descriptor
[2021-07-10 20:08:01 +0200] [43325] [INFO] Finished server process [43325]
[2021-07-10 20:08:01 +0200] [43325] [INFO] Worker exiting (pid: 43325)
[2021-07-10 20:08:01 +0200] [43323] [INFO] Shutting down: Master

As you see, the message on the shutdown event doesn't appear.

Expected behavior

If we want to have consistency with the standalone uvicorn, which I'm really unsure (as it doesn't feel right to wait the process to finish gracefully with SIGINT), then we should match this behavior and gunicorn with uvicorn workers should trigger the shutdown event as well.

In case we decide that the shutdown event should actually not be triggered by standalone uvicorn, then the issue here is no more an issue, and we should stop handling the SIGINT on the server.py. I believe this is unlikely to happen, because of practicality when developing.

Actual behavior

The shutdown event is not triggered running gunicorn with uvicorn workers when SIGINT is sent.

Environment

• OS / Python / Uvicorn version: Running uvicorn 0.14.0 with CPython 3.8.10 on Linux
• gunicorn (version 20.1.0)

Additional context

More context can be found on Gitter. Here are some messages, but you can follow the discussion there:
https://gitter.im/encode/community?at=60e3592b457e19611a37a5d6
https://gitter.im/encode/community?at=60e35dd8f862a72a30eeb8dc
https://gitter.im/encode/community?at=60e5966e24f0ae2a244a5e0d

\cc @tomchristie @florimondmanca

[sandys]

this issue is affecting us as well. Is there a fix that will be done by uvicorn ?

[Kludex]

I'm still waiting for a reply on benoitc/gunicorn#2604.

Do you have any suggestion? We could add a handler for SIGQUIT on the gunicorn worker...

[sandys]

@Kludex gunicorn is maintaining parity with uwsgi. Both kill off workers + shutdown when SIGINT or SIGQUIT is received. https://uwsgi-docs.readthedocs.io/en/latest/Management.html . in fact in uwsgi 2.1, SIGTERM also shuts down uwsgi. ( https://uwsgi-docs.readthedocs.io/en/latest/ThingsToKnow.html#things-to-know-best-practices-and-issues-read-it)

i think (speculating) that the way the internal code is written in both uwsgi and gunicorn is to kill everything in case of these three signals ( https://github.com/unbit/uwsgi/blob/d58a832c81c2c96ae0f6e72614e1cc47f4b5d332/core/emperor.c#L2000-L2002)

so ultimately i think that uvicorn will need to honor the gunicorn signal behavior (when running as a worker) and not interpret signals directly.

[sapjunior]

Is there any workaround for this problem? This issue block our project deployment timeline.

[vlad-pro]

Experiencing the same issue with gunicorn and uvicorn workers on GCP cloudrun platform.

[dmrz]

Here is a workaround I am using (is not battle-tested much yet though):

@app.on_event("shutdown")
async def shutdown():
    await database.disconnect()


if "gunicorn" in os.environ.get("SERVER_SOFTWARE", ""):
    loop = asyncio.get_event_loop()
    loop.add_signal_handler(signal.SIGQUIT, lambda _: asyncio.create_task(shutdown()))

[Feijo]

Any news about it?

[Kludex]

PR is welcome to modify the UvicornWorker.

[adnaanbheda]

@Kludex I see that you had created a PR (#1424), Is it fine if I polish that up and create a PR ?

[Kludex]

Yes. 🙏

[adnaanbheda]

@Kludex I've tested that PR, it works fine, i.e. the shutdown event is triggered, but shouldn't we use the server's install_signal_handlers to do this instead ?

Adding SIGQUIT as a handled signal in server just works fine as well, what do you think?

server.py

HANDLED_SIGNALS = (
    signal.SIGINT,  # Unix signal 2. Sent by Ctrl+C.
    signal.SIGTERM,  # Unix signal 15. Sent by `kill <pid>`.
    signal.SIGQUIT,
)

EDIT:

#1424 is triggering the shutdown event just fine for SIGINT and shutting down the application, but if we send a SIGQUIT it doesn't handle it. I've created a PR #1709, that does both.

[Kludex]

On my side, I'm not willing to change uvicorn for this, only the UvicornWorker.

[Kludex]

• Closed by Add SIGQUIT handler to UvicornWorker #1710.

It will be available on Uvicorn 0.20.0.