Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a configuration flag for enable request task handler cancelling when client connection closing. #7056

Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9772512
the prototype
mosquito Oct 28, 2022
489bd3f
add rests and docs
mosquito Oct 31, 2022
320ba35
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 31, 2022
4eef315
revew fix
mosquito Oct 31, 2022
608f877
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 31, 2022
55c631b
remove redundant test
mosquito Oct 31, 2022
a0f0305
add test_no_cancel_handler_on_connection_lost
mosquito Oct 31, 2022
0fd1d3a
dabatable warning
mosquito Oct 31, 2022
4c1bdbf
Merge branch 'master' into feature/configurable-cancel-task-when-clie…
mosquito Nov 1, 2022
906b643
Use most safe default
mosquito Nov 1, 2022
3d27e5f
Add explanation of selected default
mosquito Nov 1, 2022
0016d68
Add gunicorn control for cancel_handler_on_connection_lost
mosquito Nov 1, 2022
11ece9e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 1, 2022
d57683a
remove redundant brackets
mosquito Nov 1, 2022
db4badd
revert default to False
mosquito Nov 2, 2022
91a7fdd
Merge branch 'master' into feature/configurable-cancel-task-when-clie…
Dreamsorcerer Dec 11, 2022
e281c70
Update web.py
Dreamsorcerer Dec 11, 2022
6cee518
Update web_protocol.py
Dreamsorcerer Dec 11, 2022
f56cfa4
Update web_server.py
Dreamsorcerer Dec 11, 2022
08b3199
Update worker.py
Dreamsorcerer Dec 11, 2022
cd6c1db
Update deployment.rst
Dreamsorcerer Dec 11, 2022
13458df
Update web_reference.rst
Dreamsorcerer Dec 11, 2022
5c0e5d7
Update test_web_server.py
Dreamsorcerer Dec 11, 2022
8f3e433
Update test_web_server.py
Dreamsorcerer Dec 11, 2022
21ba63c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 11, 2022
8c41bde
Create 7056.feature
Dreamsorcerer Dec 11, 2022
4109e3c
Update test_web_server.py
Dreamsorcerer Dec 11, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions aiohttp/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ async def _run_app(
handle_signals: bool = True,
reuse_address: Optional[bool] = None,
reuse_port: Optional[bool] = None,
cancel_handler_on_connection_lost: bool = False,
) -> None:
# An internal function to actually do all dirty job for application running
if asyncio.iscoroutine(app):
Expand All @@ -320,6 +321,7 @@ async def _run_app(
access_log_format=access_log_format,
access_log=access_log,
keepalive_timeout=keepalive_timeout,
cancel_handler_on_connection_lost=cancel_handler_on_connection_lost,
)

await runner.setup()
Expand Down Expand Up @@ -480,6 +482,7 @@ def run_app(
handle_signals: bool = True,
reuse_address: Optional[bool] = None,
reuse_port: Optional[bool] = None,
cancel_handler_on_connection_lost: bool = False,
loop: Optional[asyncio.AbstractEventLoop] = None,
) -> None:
"""Run an app locally"""
Expand Down Expand Up @@ -512,6 +515,7 @@ def run_app(
handle_signals=handle_signals,
reuse_address=reuse_address,
reuse_port=reuse_port,
cancel_handler_on_connection_lost=cancel_handler_on_connection_lost,
)
)

Expand Down
5 changes: 5 additions & 0 deletions aiohttp/web_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,8 @@ def connection_lost(self, exc: Optional[BaseException]) -> None:

super().connection_lost(exc)

should_cancel_task_handler = self._manager.cancel_handler_on_connection_lost

self._manager = None
self._force_close = True
self._request_factory = None
Expand All @@ -330,6 +332,9 @@ def connection_lost(self, exc: Optional[BaseException]) -> None:
if self._waiter is not None:
self._waiter.cancel()

if should_cancel_task_handler and self._task_handler is not None:
self._task_handler.cancel()

self._task_handler = None

if self._payload_parser is not None:
Expand Down
2 changes: 2 additions & 0 deletions aiohttp/web_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def __init__(
*,
request_factory: Optional[_RequestFactory] = None,
debug: Optional[bool] = None,
cancel_handler_on_connection_lost: bool = False,
**kwargs: Any,
) -> None:
if debug is not None:
Expand All @@ -33,6 +34,7 @@ def __init__(
self.requests_count = 0
self.request_handler = handler
self.request_factory = request_factory or self._make_request
self.cancel_handler_on_connection_lost = cancel_handler_on_connection_lost

@property
def connections(self) -> List[RequestHandler]:
Expand Down
8 changes: 7 additions & 1 deletion docs/web_reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2809,7 +2809,8 @@ Utilities
access_log=aiohttp.log.access_logger, \
handle_signals=True, \
reuse_address=None, \
reuse_port=None)
reuse_port=None, \
cancel_handler_on_connection_lost=False)

A high-level function for running an application, serving it until
keyboard interrupt and performing a
Expand Down Expand Up @@ -2904,6 +2905,11 @@ Utilities
this flag when being created. This option is not
supported on Windows.

:param bool cancel_handler_on_connection_lost: tells the runner whether to
cancel the execution of the
handler task if the client
connection has been closed.
mosquito marked this conversation as resolved.
Show resolved Hide resolved

.. versionadded:: 3.0

Support *access_log_class* parameter.
Expand Down
52 changes: 51 additions & 1 deletion tests/test_web_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import pytest

from aiohttp import client, helpers, web
from aiohttp import ClientSession, client, helpers, web


async def test_simple_server(aiohttp_raw_server: Any, aiohttp_client: Any) -> None:
Expand Down Expand Up @@ -207,3 +207,53 @@ async def handler(request):
)

logger.exception.assert_called_with("Error handling request", exc_info=exc)


async def test_cancel_handler_on_connection_lost(aiohttp_unused_port) -> None:
event = asyncio.Event()
port = aiohttp_unused_port()

async def on_request(_: web.Request) -> web.Response:
nonlocal event
try:
await asyncio.sleep(10)
except asyncio.CancelledError:
event.set()
raise
else:
raise web.HTTPInternalServerError()

app = web.Application()
app.router.add_route("GET", "/", on_request)

runner = web.AppRunner(app, cancel_handler_on_connection_lost=True)
await runner.setup()

site = web.TCPSite(runner, host="localhost", port=port)

await site.start()

async def client_request_maker():
async with ClientSession(base_url=f"http://localhost:{port}") as session:
request = session.get("/")

with pytest.raises(asyncio.TimeoutError):
await asyncio.wait_for(request, timeout=0.1)

try:
assert (
runner.server.cancel_handler_on_connection_lost
), "Flag was not propagated"
await client_request_maker()

await asyncio.wait_for(event.wait(), timeout=1)
assert event.is_set(), "Request handler hasn't been cancelled"
mosquito marked this conversation as resolved.
Show resolved Hide resolved
finally:
await asyncio.gather(runner.shutdown(), site.stop())


async def test_cancel_handler_on_connection_lost_flag_on_runner() -> None:
runner = web.AppRunner(web.Application(), cancel_handler_on_connection_lost=True)
await runner.setup()
assert runner.server.cancel_handler_on_connection_lost, "Flag was not propagated"
await runner.shutdown()
Dreamsorcerer marked this conversation as resolved.
Show resolved Hide resolved