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

Fix system routes polluting the middleware cache #9852

Merged
merged 1 commit into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/9852.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed system routes polluting the middleware cache -- by :user:`bdraco`.
15 changes: 12 additions & 3 deletions aiohttp/web_app.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import asyncio
import logging
import warnings
from functools import cache, partial, update_wrapper
from functools import lru_cache, partial, update_wrapper
from typing import (
TYPE_CHECKING,
Any,
Expand Down Expand Up @@ -44,6 +44,7 @@
MaskDomain,
MatchedSubAppResource,
PrefixedSubAppResource,
SystemRoute,
UrlDispatcher,
)

Expand All @@ -70,7 +71,6 @@
_Resource = TypeVar("_Resource", bound=AbstractResource)


@cache
def _build_middlewares(
handler: Handler, apps: Tuple["Application", ...]
) -> Callable[[Request], Awaitable[StreamResponse]]:
Expand All @@ -84,6 +84,9 @@ def _build_middlewares(
return handler


_cached_build_middleware = lru_cache(maxsize=1024)(_build_middlewares)


@final
class Application(MutableMapping[Union[str, AppKey[Any]], Any]):
__slots__ = (
Expand Down Expand Up @@ -397,7 +400,13 @@ async def _handle(self, request: Request) -> StreamResponse:
handler = match_info.handler

if self._run_middlewares:
handler = _build_middlewares(handler, match_info.apps)
# If its a SystemRoute, don't cache building the middlewares since
# they are constructed for every MatchInfoError as a new handler
# is made each time.
if isinstance(match_info.route, SystemRoute):
handler = _build_middlewares(handler, match_info.apps)
else:
handler = _cached_build_middleware(handler, match_info.apps)

return await handler(request)

Expand Down
25 changes: 24 additions & 1 deletion tests/test_web_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import pytest
from yarl import URL

from aiohttp import web
from aiohttp import web, web_app
from aiohttp.pytest_plugin import AiohttpClient
from aiohttp.test_utils import TestClient
from aiohttp.typedefs import Handler, Middleware
Expand Down Expand Up @@ -522,3 +522,26 @@ async def call(self, request: web.Request, handler: Handler) -> web.Response:
assert 201 == resp.status
txt = await resp.text()
assert "OK[new style middleware]" == txt


async def test_middleware_does_not_leak(aiohttp_client: AiohttpClient) -> None:
async def any_handler(request: web.Request) -> NoReturn:
assert False

class Middleware:
async def call(
self, request: web.Request, handler: Handler
) -> web.StreamResponse:
return await handler(request)

app = web.Application()
app.router.add_route("POST", "/any", any_handler)
app.middlewares.append(Middleware().call)

client = await aiohttp_client(app)

web_app._cached_build_middleware.cache_clear()
for _ in range(10):
resp = await client.get("/any")
assert resp.status == 405
assert web_app._cached_build_middleware.cache_info().currsize < 10
Loading