From 4e746cb9553b33fbcd6f61b51a9cd2f7e96d6248 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Mon, 27 Sep 2021 10:22:30 +0300 Subject: [PATCH] Close HTTP loop when connection task cancelled (#2245) * Terminate loop when no transport exists * Add log when closing HTTP loop because of shutdown * Add unit test --- sanic/http.py | 6 ++++ sanic/server/protocols/http_protocol.py | 2 +- tests/test_graceful_shutdown.py | 46 +++++++++++++++++++++++++ tests/test_signal_handlers.py | 7 +--- 4 files changed, 54 insertions(+), 7 deletions(-) create mode 100644 tests/test_graceful_shutdown.py diff --git a/sanic/http.py b/sanic/http.py index 7bdca6a296..3fafd83a9f 100644 --- a/sanic/http.py +++ b/sanic/http.py @@ -148,6 +148,12 @@ async def http1(self): await self.response.send(end_stream=True) except CancelledError: # Write an appropriate response before exiting + if not self.protocol.transport: + logger.info( + f"Request: {self.request.method} {self.request.url} " + "stopped. Transport is closed." + ) + return e = self.exception or ServiceUnavailable("Cancelled") self.exception = None self.keep_alive = False diff --git a/sanic/server/protocols/http_protocol.py b/sanic/server/protocols/http_protocol.py index b898d3b1bc..c609694eea 100644 --- a/sanic/server/protocols/http_protocol.py +++ b/sanic/server/protocols/http_protocol.py @@ -109,7 +109,7 @@ async def connection_task(self): # no cov except Exception: error_logger.exception("protocol.connection_task uncaught") finally: - if self.app.debug and self._http: + if self.app.debug and self._http and self.transport: ip = self.transport.get_extra_info("peername") error_logger.error( "Connection lost before response written" diff --git a/tests/test_graceful_shutdown.py b/tests/test_graceful_shutdown.py new file mode 100644 index 0000000000..8380ed50d2 --- /dev/null +++ b/tests/test_graceful_shutdown.py @@ -0,0 +1,46 @@ +import asyncio +import logging +import time + +from collections import Counter +from multiprocessing import Process + +import httpx + + +PORT = 42101 + + +def test_no_exceptions_when_cancel_pending_request(app, caplog): + app.config.GRACEFUL_SHUTDOWN_TIMEOUT = 1 + + @app.get("/") + async def handler(request): + await asyncio.sleep(5) + + @app.after_server_start + def shutdown(app, _): + time.sleep(0.2) + app.stop() + + def ping(): + time.sleep(0.1) + response = httpx.get("http://127.0.0.1:8000") + print(response.status_code) + + p = Process(target=ping) + p.start() + + with caplog.at_level(logging.INFO): + app.run() + + p.kill() + + counter = Counter([r[1] for r in caplog.record_tuples]) + + assert counter[logging.INFO] == 5 + assert logging.ERROR not in counter + assert ( + caplog.record_tuples[3][2] + == "Request: GET http://127.0.0.1:8000/ stopped. Transport is closed." + ) diff --git a/tests/test_signal_handlers.py b/tests/test_signal_handlers.py index c51cdd219f..f7657ad64b 100644 --- a/tests/test_signal_handlers.py +++ b/tests/test_signal_handlers.py @@ -7,7 +7,6 @@ import pytest -from sanic_testing.reusable import ReusableClient from sanic_testing.testing import HOST, PORT from sanic.compat import ctrlc_workaround_for_windows @@ -29,13 +28,9 @@ def set_loop(app, loop): signal.signal = mock else: loop.add_signal_handler = mock - print(">>>>>>>>>>>>>>>1", id(loop)) - print(">>>>>>>>>>>>>>>1", loop.add_signal_handler) def after(app, loop): - print(">>>>>>>>>>>>>>>2", id(loop)) - print(">>>>>>>>>>>>>>>2", loop.add_signal_handler) calledq.put(mock.called) @@ -100,7 +95,7 @@ async def atest(stop_first): os.kill(os.getpid(), signal.SIGINT) await asyncio.sleep(0.2) assert app.is_stopping - assert app.stay_active_task.result() == None + assert app.stay_active_task.result() is None # Second Ctrl+C should raise with pytest.raises(KeyboardInterrupt): os.kill(os.getpid(), signal.SIGINT)