From 3d21f066bfc3a49d68062b951d8e524c19c73d26 Mon Sep 17 00:00:00 2001 From: Max Fischer Date: Sun, 10 Mar 2024 17:03:06 +0100 Subject: [PATCH 01/12] test desired signal behaviour --- tests/test_server.py | 60 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 tests/test_server.py diff --git a/tests/test_server.py b/tests/test_server.py new file mode 100644 index 000000000..5df10d64b --- /dev/null +++ b/tests/test_server.py @@ -0,0 +1,60 @@ +from typing import Generator +import asyncio +import os +import signal +import sys + +import pytest + +from uvicorn.config import Config +from uvicorn.server import Server + + +class CaughtSignal(Exception): + pass + + +@pytest.fixture(params=[signal.SIGINT, signal.SIGTERM]) +def exception_signal(request: "type[pytest.FixtureRequest]") -> Generator[signal.Signals, None, None]: # pragma: py-win32 + """Fixture that replaces SIGINT/SIGTERM handling with a normal exception""" + + def raise_handler(*_: object) -> None: + raise CaughtSignal + + original_handler = signal.signal(request.param, raise_handler) + yield request.param + signal.signal(request.param, original_handler) + + +@pytest.fixture(params=[signal.SIGINT, signal.SIGTERM]) +def async_exception_signal(request: "type[pytest.FixtureRequest]") -> Generator[signal.Signals, None, None]: # pragma: py-win32 + """Fixture that replaces SIGINT/SIGTERM handling with a normal exception""" + + def raise_handler(*_: object) -> None: + raise CaughtSignal + + original_handler = signal.signal(request.param, raise_handler) + yield request.param + signal.signal(request.param, original_handler) + + +async def dummy_app(scope, receive, send): # pragma: py-win32 + pass + + +@pytest.mark.anyio +@pytest.mark.skipif(sys.platform == "win32", reason="require unix-like signal handling") +async def test_server_interrupt(exception_signal: signal.Signals): # pragma: py-win32 + """Test interrupting a Server that is run explicitly inside asyncio""" + + async def interrupt_running(srv: Server): + while not srv.started: + await asyncio.sleep(0.01) + os.kill(os.getpid(), exception_signal) + + server = Server(Config(app=dummy_app, loop="asyncio")) + asyncio.create_task(interrupt_running(server)) + with pytest.raises(CaughtSignal): + await server.serve() + # set by the server's graceful exit handler + assert server.should_exit From 5c9bd41d0f8305da6089cae6ef8ce81708cc9093 Mon Sep 17 00:00:00 2001 From: Max Fischer Date: Sun, 10 Mar 2024 17:27:12 +0100 Subject: [PATCH 02/12] capture and restore signal handlers --- uvicorn/server.py | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/uvicorn/server.py b/uvicorn/server.py index c7645f3ce..7752917ed 100644 --- a/uvicorn/server.py +++ b/uvicorn/server.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +import contextlib import logging import os import platform @@ -57,11 +58,17 @@ def __init__(self, config: Config) -> None: self.force_exit = False self.last_notified = 0.0 + self._captured_signals: list[int] = [] + def run(self, sockets: list[socket.socket] | None = None) -> None: self.config.setup_event_loop() return asyncio.run(self.serve(sockets=sockets)) async def serve(self, sockets: list[socket.socket] | None = None) -> None: + with self.capture_signals(): + await self._serve(sockets) + + async def _serve(self, sockets: list[socket.socket] | None = None) -> None: process_id = os.getpid() config = self.config @@ -70,8 +77,6 @@ async def serve(self, sockets: list[socket.socket] | None = None) -> None: self.lifespan = config.lifespan_class(config) - self.install_signal_handlers() - message = "Started server process [%d]" color_message = "Started server process [" + click.style("%d", fg="cyan") + "]" logger.info(message, process_id, extra={"color_message": color_message}) @@ -302,22 +307,30 @@ async def _wait_tasks_to_complete(self) -> None: for server in self.servers: await server.wait_closed() - def install_signal_handlers(self) -> None: + @contextlib.contextmanager + def capture_signals(self): + # Signals can only be listened to from the main thread. if threading.current_thread() is not threading.main_thread(): - # Signals can only be listened to from the main thread. + yield return - - loop = asyncio.get_event_loop() - + # always use signal.signal, even if loop.add_signal_handler is available + # this allows to restore previous signal handlers later on + original_handlers = { + sig: signal.signal(sig, self.handle_exit) for sig in HANDLED_SIGNALS + } try: - for sig in HANDLED_SIGNALS: - loop.add_signal_handler(sig, self.handle_exit, sig, None) - except NotImplementedError: # pragma: no cover - # Windows - for sig in HANDLED_SIGNALS: - signal.signal(sig, self.handle_exit) + yield + finally: + for sig, handler in original_handlers.items(): + signal.signal(sig, handler) + # If we did gracefully shut down due to a signal, try to + # trigger the expected behaviour now; multiple signals would be + # done LIFO, see https://stackoverflow.com/questions/48434964 + for captured_signal in reversed(self._captured_signals): # pragma: py-win32 + signal.raise_signal(captured_signal) def handle_exit(self, sig: int, frame: FrameType | None) -> None: + self._captured_signals.append(sig) if self.should_exit and sig == signal.SIGINT: self.force_exit = True else: From c4566596c264d63f2e990cc56de702a0c61a50b3 Mon Sep 17 00:00:00 2001 From: Max Fischer Date: Sun, 10 Mar 2024 17:38:11 +0100 Subject: [PATCH 03/12] ruff --- tests/test_server.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/test_server.py b/tests/test_server.py index 5df10d64b..2a6c9fceb 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -1,8 +1,10 @@ -from typing import Generator +from __future__ import annotations + import asyncio import os import signal import sys +from typing import Generator import pytest @@ -15,7 +17,7 @@ class CaughtSignal(Exception): @pytest.fixture(params=[signal.SIGINT, signal.SIGTERM]) -def exception_signal(request: "type[pytest.FixtureRequest]") -> Generator[signal.Signals, None, None]: # pragma: py-win32 +def exception_signal(request: type[pytest.FixtureRequest]) -> Generator[signal.Signals, None, None]: # pragma: py-win32 """Fixture that replaces SIGINT/SIGTERM handling with a normal exception""" def raise_handler(*_: object) -> None: @@ -27,12 +29,15 @@ def raise_handler(*_: object) -> None: @pytest.fixture(params=[signal.SIGINT, signal.SIGTERM]) -def async_exception_signal(request: "type[pytest.FixtureRequest]") -> Generator[signal.Signals, None, None]: # pragma: py-win32 +def async_exception_signal( # pragma: py-win32 + request: type[pytest.FixtureRequest] +) -> Generator[signal.Signals, None, None]: """Fixture that replaces SIGINT/SIGTERM handling with a normal exception""" def raise_handler(*_: object) -> None: raise CaughtSignal + asyncio.get_running_loop().add_signal_handler original_handler = signal.signal(request.param, raise_handler) yield request.param signal.signal(request.param, original_handler) From ab22bc5ba9c35f443da917c6b91d2ac0827070fb Mon Sep 17 00:00:00 2001 From: Max Fischer Date: Sun, 10 Mar 2024 17:41:52 +0100 Subject: [PATCH 04/12] checks --- tests/test_server.py | 2 +- uvicorn/server.py | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/test_server.py b/tests/test_server.py index 2a6c9fceb..05d7b9c83 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -30,7 +30,7 @@ def raise_handler(*_: object) -> None: @pytest.fixture(params=[signal.SIGINT, signal.SIGTERM]) def async_exception_signal( # pragma: py-win32 - request: type[pytest.FixtureRequest] + request: type[pytest.FixtureRequest], ) -> Generator[signal.Signals, None, None]: """Fixture that replaces SIGINT/SIGTERM handling with a normal exception""" diff --git a/uvicorn/server.py b/uvicorn/server.py index 7752917ed..a285876c3 100644 --- a/uvicorn/server.py +++ b/uvicorn/server.py @@ -12,7 +12,7 @@ import time from email.utils import formatdate from types import FrameType -from typing import TYPE_CHECKING, Sequence, Union +from typing import TYPE_CHECKING, Generator, Sequence, Union import click @@ -308,16 +308,14 @@ async def _wait_tasks_to_complete(self) -> None: await server.wait_closed() @contextlib.contextmanager - def capture_signals(self): + def capture_signals(self) -> Generator[None, None, None]: # Signals can only be listened to from the main thread. if threading.current_thread() is not threading.main_thread(): yield return # always use signal.signal, even if loop.add_signal_handler is available # this allows to restore previous signal handlers later on - original_handlers = { - sig: signal.signal(sig, self.handle_exit) for sig in HANDLED_SIGNALS - } + original_handlers = {sig: signal.signal(sig, self.handle_exit) for sig in HANDLED_SIGNALS} try: yield finally: From 89f6751f7bea982335924183316d4a9960365a3a Mon Sep 17 00:00:00 2001 From: Max Fischer Date: Sun, 10 Mar 2024 20:48:53 +0100 Subject: [PATCH 05/12] test asyncio handlers --- tests/test_server.py | 56 ++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/tests/test_server.py b/tests/test_server.py index 05d7b9c83..0512e084d 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -1,10 +1,11 @@ from __future__ import annotations import asyncio +import contextlib import os import signal import sys -from typing import Generator +from typing import Callable, ContextManager, Generator import pytest @@ -12,35 +13,25 @@ from uvicorn.server import Server -class CaughtSignal(Exception): - pass - - -@pytest.fixture(params=[signal.SIGINT, signal.SIGTERM]) -def exception_signal(request: type[pytest.FixtureRequest]) -> Generator[signal.Signals, None, None]: # pragma: py-win32 - """Fixture that replaces SIGINT/SIGTERM handling with a normal exception""" - - def raise_handler(*_: object) -> None: - raise CaughtSignal - - original_handler = signal.signal(request.param, raise_handler) - yield request.param - signal.signal(request.param, original_handler) - - -@pytest.fixture(params=[signal.SIGINT, signal.SIGTERM]) -def async_exception_signal( # pragma: py-win32 - request: type[pytest.FixtureRequest], -) -> Generator[signal.Signals, None, None]: - """Fixture that replaces SIGINT/SIGTERM handling with a normal exception""" +# asyncio does NOT allow raising in signal handlers, so to detect +# raised signals raised a mutable `witness` receives the signal +@contextlib.contextmanager +def capture_signal_sync(sig: signal.Signals) -> Generator[list[int], None, None]: # pragma: py-win32 + """Replace `sig` handling with a normal exception via `signal""" + witness: list[int] = [] + original_handler = signal.signal(sig, lambda signum, frame: witness.append(signum)) + yield witness + signal.signal(sig, original_handler) - def raise_handler(*_: object) -> None: - raise CaughtSignal - asyncio.get_running_loop().add_signal_handler - original_handler = signal.signal(request.param, raise_handler) - yield request.param - signal.signal(request.param, original_handler) +@contextlib.contextmanager +def capture_signal_async(sig: signal.Signals) -> Generator[list[int], None, None]: # pragma: py-win32 + """Replace `sig` handling with a normal exception via `asyncio""" + witness: list[int] = [] + original_handler = signal.getsignal(sig) + asyncio.get_running_loop().add_signal_handler(sig, witness.append, sig) + yield witness + signal.signal(sig, original_handler) async def dummy_app(scope, receive, send): # pragma: py-win32 @@ -49,7 +40,11 @@ async def dummy_app(scope, receive, send): # pragma: py-win32 @pytest.mark.anyio @pytest.mark.skipif(sys.platform == "win32", reason="require unix-like signal handling") -async def test_server_interrupt(exception_signal: signal.Signals): # pragma: py-win32 +@pytest.mark.parametrize("exception_signal", [signal.SIGTERM, signal.SIGINT]) +@pytest.mark.parametrize("capture_signal", [capture_signal_sync, capture_signal_async]) +async def test_server_interrupt( + exception_signal: signal.Signals, capture_signal: Callable[[signal.Signals], ContextManager[None]] +): # pragma: py-win32 """Test interrupting a Server that is run explicitly inside asyncio""" async def interrupt_running(srv: Server): @@ -59,7 +54,8 @@ async def interrupt_running(srv: Server): server = Server(Config(app=dummy_app, loop="asyncio")) asyncio.create_task(interrupt_running(server)) - with pytest.raises(CaughtSignal): + with capture_signal(exception_signal) as witness: await server.serve() + assert witness # set by the server's graceful exit handler assert server.should_exit From 6c53405debdc8e59a86407364a9015e7389f8fd8 Mon Sep 17 00:00:00 2001 From: Max Fischer Date: Sun, 10 Mar 2024 21:06:36 +0100 Subject: [PATCH 06/12] add note on signal handler handling --- docs/index.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/index.md b/docs/index.md index 6ce92feb8..30eb7d88b 100644 --- a/docs/index.md +++ b/docs/index.md @@ -260,6 +260,10 @@ if __name__ == "__main__": asyncio.run(main()) ``` +!!! note + Both `server.run` and `server.serve` install custom signal handlers while the server runs. + The original signal handlers are restored and, if appropriate, invoked once the server finishes. + ### Running with Gunicorn [Gunicorn][gunicorn] is a mature, fully featured server and process manager. From 8682a94ddc5a68a6f1dc3e303d3e77799ac718f6 Mon Sep 17 00:00:00 2001 From: Max Fischer Date: Sun, 10 Mar 2024 21:11:58 +0100 Subject: [PATCH 07/12] remove legacy signal raising --- tests/test_server.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_server.py b/tests/test_server.py index 0512e084d..0657be409 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -2,7 +2,6 @@ import asyncio import contextlib -import os import signal import sys from typing import Callable, ContextManager, Generator @@ -50,7 +49,7 @@ async def test_server_interrupt( async def interrupt_running(srv: Server): while not srv.started: await asyncio.sleep(0.01) - os.kill(os.getpid(), exception_signal) + signal.raise_signal(exception_signal) server = Server(Config(app=dummy_app, loop="asyncio")) asyncio.create_task(interrupt_running(server)) From 103e51bc5b68285effa2da474a24a51ed151775e Mon Sep 17 00:00:00 2001 From: Max Fischer Date: Tue, 12 Mar 2024 20:33:44 +0100 Subject: [PATCH 08/12] test SIGBREAK on windows --- tests/test_server.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/test_server.py b/tests/test_server.py index 0657be409..95a2afcf3 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -15,7 +15,7 @@ # asyncio does NOT allow raising in signal handlers, so to detect # raised signals raised a mutable `witness` receives the signal @contextlib.contextmanager -def capture_signal_sync(sig: signal.Signals) -> Generator[list[int], None, None]: # pragma: py-win32 +def capture_signal_sync(sig: signal.Signals) -> Generator[list[int], None, None]: """Replace `sig` handling with a normal exception via `signal""" witness: list[int] = [] original_handler = signal.signal(sig, lambda signum, frame: witness.append(signum)) @@ -37,10 +37,18 @@ async def dummy_app(scope, receive, send): # pragma: py-win32 pass +if sys.platform == "win32": + signals = [signal.SIGBREAK] + signal_captures = [capture_signal_sync] +else: + signals = [signal.SIGTERM, signal.SIGINT] + signal_captures = [capture_signal_sync, capture_signal_async] + + @pytest.mark.anyio @pytest.mark.skipif(sys.platform == "win32", reason="require unix-like signal handling") -@pytest.mark.parametrize("exception_signal", [signal.SIGTERM, signal.SIGINT]) -@pytest.mark.parametrize("capture_signal", [capture_signal_sync, capture_signal_async]) +@pytest.mark.parametrize("exception_signal", signals) +@pytest.mark.parametrize("capture_signal", signal_captures) async def test_server_interrupt( exception_signal: signal.Signals, capture_signal: Callable[[signal.Signals], ContextManager[None]] ): # pragma: py-win32 From c8018a6c338fca3ea49743f12439e958f6f5a827 Mon Sep 17 00:00:00 2001 From: Max Fischer Date: Tue, 12 Mar 2024 20:39:55 +0100 Subject: [PATCH 09/12] remove test guard --- tests/test_server.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_server.py b/tests/test_server.py index 95a2afcf3..dac8fb026 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -46,7 +46,6 @@ async def dummy_app(scope, receive, send): # pragma: py-win32 @pytest.mark.anyio -@pytest.mark.skipif(sys.platform == "win32", reason="require unix-like signal handling") @pytest.mark.parametrize("exception_signal", signals) @pytest.mark.parametrize("capture_signal", signal_captures) async def test_server_interrupt( From 4abad9415ebe5c335e491be2128b712c17582746 Mon Sep 17 00:00:00 2001 From: Max Fischer Date: Tue, 12 Mar 2024 20:47:32 +0100 Subject: [PATCH 10/12] include convered branch --- uvicorn/server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uvicorn/server.py b/uvicorn/server.py index a285876c3..bfce1b1b1 100644 --- a/uvicorn/server.py +++ b/uvicorn/server.py @@ -324,7 +324,7 @@ def capture_signals(self) -> Generator[None, None, None]: # If we did gracefully shut down due to a signal, try to # trigger the expected behaviour now; multiple signals would be # done LIFO, see https://stackoverflow.com/questions/48434964 - for captured_signal in reversed(self._captured_signals): # pragma: py-win32 + for captured_signal in reversed(self._captured_signals): signal.raise_signal(captured_signal) def handle_exit(self, sig: int, frame: FrameType | None) -> None: From 3dd7f8031c793dd34cc2f3c3fc92528bdb36bd36 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Tue, 19 Mar 2024 02:32:45 -0600 Subject: [PATCH 11/12] Update docs/index.md --- docs/index.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/docs/index.md b/docs/index.md index 30eb7d88b..4a554a397 100644 --- a/docs/index.md +++ b/docs/index.md @@ -260,9 +260,6 @@ if __name__ == "__main__": asyncio.run(main()) ``` -!!! note - Both `server.run` and `server.serve` install custom signal handlers while the server runs. - The original signal handlers are restored and, if appropriate, invoked once the server finishes. ### Running with Gunicorn From bb9997788e21d1a572a95196e5b493ff80e92bc3 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Tue, 19 Mar 2024 02:32:59 -0600 Subject: [PATCH 12/12] Update docs/index.md --- docs/index.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/index.md b/docs/index.md index 4a554a397..6ce92feb8 100644 --- a/docs/index.md +++ b/docs/index.md @@ -260,7 +260,6 @@ if __name__ == "__main__": asyncio.run(main()) ``` - ### Running with Gunicorn [Gunicorn][gunicorn] is a mature, fully featured server and process manager.