web.run_app(): Terminate properly with signal TERM and INT

[cecton]

Long story short

This is more or less related to #63. My issue is that web.run_app() gracefully exit when SIGINT is sent to the process but not when SIGTERM is sent.

This is because web.run_app() catches SIGINT (with KeyboardInterrupt exception) but does not override SIGTERM handling (there is no exception raised for that, one can only use signal.signal). Most of the time it is SIGTERM that is used to gracefully shutdown a process but that will actually kill the application instead of letting the requests finish properly and response. kill, Docker and system.d use SIGTERM by default.

Expected behaviour

I suppose the default expected behavior of web.run_app() is to allow the requests to be finished before terminating the process. That's what most people would think it does. The feeling is also harder since it does it already for SIGINT.

Actual behaviour

Only SIGINT triggers a graceful exit. SIGTERM does not let the running requests finish.

Steps to reproduce

1. run this script

import asyncio
from aiohttp import web

async def handle(request):
    await asyncio.sleep(60)
    text = "Hello"
    return web.Response(text=text)

app = web.Application()
app.router.add_get('/', handle)

web.run_app(app)

2. do a GET request on /
3. send SIGTERM to the process
4. you will notice the process ends before the 60 seconds and your request has been canceled

Your environment

Python 3.6, Arch Linux

[hubo1016]

Maybe raising a customized exception in a signal handler (instead of SystemExit) is enough to solve this?

[asvetlov]

Ok, run_app should accept a flag for enabling SIGTERM handling (switched on by default).
Implementation could raise a custom exception as @hubo1016 suggested.
Raising an exception derived from SystemExit is totally fine.

@cecton would you create a Pull Request?

[cecton]

Yeay 😄 thx @asvetlov

[cecton]

Solved by #1946

[lock]

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.