fix: resource warnings (#3838)

---------

Co-authored-by: Janek Nouvertné <25355197+provinzkraut@users.noreply.github.com>

Author: gsakkis

litestar/_kwargs/extractors.py

@@ -337,16 +337,16 @@ async def _extract_multipart(
         if body_kwarg_multipart_form_part_limit is not None
         else connection.app.multipart_form_part_limit
     )
-    connection.scope["_form"] = form_values = (  # type: ignore[typeddict-unknown-key]
-        connection.scope["_form"]  # type: ignore[typeddict-item]
-        if "_form" in connection.scope
-        else await parse_multipart_form(
+    scope_state = ScopeState.from_scope(connection.scope)
+    if scope_state.form is Empty:
+        scope_state.form = form_values = await parse_multipart_form(
             stream=connection.stream(),
             boundary=connection.content_type[-1].get("boundary", "").encode(),
             multipart_form_part_limit=multipart_form_part_limit,
             type_decoders=connection.route_handler.resolve_type_decoders(),
         )
-    )
+    else:
+        form_values = scope_state.form
 
     if field_definition.is_non_string_sequence:
         values = list(form_values.values())
@@ -377,7 +377,7 @@ async def _extract_multipart(
                 or (is_optional_union(tp) and is_non_string_sequence(make_non_optional_union(tp)))
             )
         ):
-            form_values[name] = [value]
+            form_values[name] = [value]  # pyright: ignore
 
     return form_values
 
@@ -426,11 +426,13 @@ def create_url_encoded_data_extractor(
     async def extract_url_encoded_extractor(
         connection: Request[Any, Any, Any],
     ) -> Any:
-        connection.scope["_form"] = form_values = (  # type: ignore[typeddict-unknown-key]
-            connection.scope["_form"]  # type: ignore[typeddict-item]
-            if "_form" in connection.scope
-            else parse_url_encoded_form_data(await connection.body())
-        )
+        scope_state = ScopeState.from_scope(connection.scope)
+        if scope_state.form is Empty:
+            scope_state.form = form_values = (  # type: ignore[assignment]
+                parse_url_encoded_form_data(await connection.body())
+            )
+        else:
+            form_values = scope_state.form  # type: ignore[assignment]
 
         if not form_values and is_data_optional:
             return None

litestar/_multipart.py

@@ -48,6 +48,13 @@ def parse_content_header(value: str) -> tuple[str, dict[str, str]]:
     return value.strip().lower(), options
 
 
+async def _close_upload_files(fields: dict[str, list[Any]]) -> None:
+    for values in fields.values():
+        for value in values:
+            if isinstance(value, UploadFile):
+                await value.close()
+
+
 async def parse_multipart_form(  # noqa: C901
     stream: AsyncGenerator[bytes, None],
     boundary: bytes,
@@ -72,10 +79,11 @@ async def parse_multipart_form(  # noqa: C901
     if not chunk:
         return fields
 
+    data: UploadFile | bytearray = bytearray()
+
     try:
         with PushMultipartParser(boundary, max_segment_count=multipart_form_part_limit) as parser:
             segment: MultipartSegment | None = None
-            data: UploadFile | bytearray = bytearray()
             while not parser.closed:
                 for form_part in parser.parse(chunk):
                     if isinstance(form_part, MultipartSegment):
@@ -113,8 +121,17 @@ async def parse_multipart_form(  # noqa: C901
                 chunk = await async_next(stream, b"")
 
     except ParserError as exc:
+        # if an exception is raised, make sure that all 'UploadFile's are closed
+        if isinstance(data, UploadFile):
+            await data.close()
+        await _close_upload_files(fields)
+
         raise ClientException("Invalid multipart/form-data") from exc
     except ParserLimitReached:
+        if isinstance(data, UploadFile):
+            await data.close()
+        await _close_upload_files(fields)
+
         # FIXME (3.0): This should raise a '413 - Request Entity Too Large', but for
         # backwards compatibility, we keep it as a 400 for now
         raise ClientException("Request Entity Too Large") from None

litestar/connection/request.py

@@ -270,25 +270,15 @@ async def form(self) -> FormMultiDict:
                         multipart_form_part_limit=self.app.multipart_form_part_limit,
                     )
                 elif content_type == RequestEncodingType.URL_ENCODED:
-                    form_data = parse_url_encoded_form_data(
+                    form_data = parse_url_encoded_form_data(  # type: ignore[assignment]
                         await self.body(),
                     )
                 else:
                     form_data = {}
 
-                self._connection_state.form = form_data
+                self._connection_state.form = form_data  # pyright: ignore
 
-            # form_data is a dict[str, list[str] | str | UploadFile]. Convert it to a
-            # list[tuple[str, str | UploadFile]] before passing it to FormMultiDict so
-            # multi-keys can be accessed properly
-            items = []
-            for k, v in form_data.items():
-                if isinstance(v, list):
-                    for sv in v:
-                        items.append((k, sv))
-                else:
-                    items.append((k, v))
-            self._form = FormMultiDict(items)
+            self._form = FormMultiDict.from_form_data(cast("dict[str, Any]", form_data))
 
         return self._form
 

litestar/datastructures/multi_dicts.py

@@ -95,6 +95,27 @@ def copy(self) -> Self:  # type: ignore[override]
 class FormMultiDict(ImmutableMultiDict[Any]):
     """MultiDict for form data."""
 
+    @classmethod
+    def from_form_data(cls, form_data: dict[str, list[str] | str | UploadFile]) -> FormMultiDict:
+        """Create a FormMultiDict from form data.
+
+        Args:
+            form_data: Form data to create the FormMultiDict from.
+
+        Returns:
+            A FormMultiDict instance
+        """
+        # Convert form_data to a list[tuple[str, str | UploadFile]] before passing it
+        # to FormMultiDict so multi-keys can be accessed properly
+        items = []
+        for k, v in form_data.items():
+            if not isinstance(v, list):
+                items.append((k, v))
+            else:
+                for sv in v:
+                    items.append((k, sv))
+        return cls(items)
+
     async def close(self) -> None:
         """Close all files in the multi-dict.
 

litestar/datastructures/upload_file.py

@@ -94,6 +94,8 @@ async def close(self) -> None:
         Returns:
             None.
         """
+        if self.file.closed:
+            return None
         if self.rolled_to_disk:
             return await sync_to_thread(self.file.close)
         return self.file.close()

litestar/routes/http.py

@@ -1,11 +1,11 @@
 from __future__ import annotations
 
 from itertools import chain
-from typing import TYPE_CHECKING, Any, cast
+from typing import TYPE_CHECKING, Any
 
 from msgspec.msgpack import decode as _decode_msgpack_plain
 
-from litestar.datastructures.upload_file import UploadFile
+from litestar.datastructures.multi_dicts import FormMultiDict
 from litestar.enums import HttpMethod, MediaType, ScopeType
 from litestar.exceptions import ClientException, ImproperlyConfiguredException, SerializationException
 from litestar.handlers.http_handlers import HTTPRouteHandler
@@ -77,17 +77,18 @@ async def handle(self, scope: HTTPScope, receive: Receive, send: Send) -> None:
         if route_handler.resolve_guards():
             await route_handler.authorize_connection(connection=request)
 
-        response = await self._get_response_for_request(
-            scope=scope, request=request, route_handler=route_handler, parameter_model=parameter_model
-        )
-
-        await response(scope, receive, send)
+        try:
+            response = await self._get_response_for_request(
+                scope=scope, request=request, route_handler=route_handler, parameter_model=parameter_model
+            )
 
-        if after_response_handler := route_handler.resolve_after_response():
-            await after_response_handler(request)
+            await response(scope, receive, send)
 
-        if form_data := scope.get("_form", {}):
-            await self._cleanup_temporary_files(form_data=cast("dict[str, Any]", form_data))
+            if after_response_handler := route_handler.resolve_after_response():
+                await after_response_handler(request)
+        finally:
+            if (form_data := ScopeState.from_scope(scope).form) is not Empty:
+                await FormMultiDict.from_form_data(form_data).close()
 
     def create_handler_map(self) -> None:
         """Parse the ``router_handlers`` of this route and return a mapping of
@@ -258,9 +259,3 @@ def options_handler(scope: Scope) -> Response:
             include_in_schema=False,
             sync_to_thread=False,
         )(options_handler)
-
-    @staticmethod
-    async def _cleanup_temporary_files(form_data: dict[str, Any]) -> None:
-        for v in form_data.values():
-            if isinstance(v, UploadFile) and not v.file.closed:
-                await v.close()

litestar/utils/scope/state.py

@@ -9,7 +9,7 @@
 if TYPE_CHECKING:
     from typing_extensions import Self
 
-    from litestar.datastructures import URL, Accept, Headers
+    from litestar.datastructures import URL, Accept, Headers, UploadFile
     from litestar.types.asgi_types import Scope
     from litestar.types.composite_types import ExceptionHandlersMap
 
@@ -83,7 +83,7 @@ def __init__(self) -> None:
     dependency_cache: dict[str, Any] | EmptyType
     do_cache: bool | EmptyType
     exception_handlers: ExceptionHandlersMap | EmptyType
-    form: dict[str, str | list[str]] | EmptyType
+    form: dict[str, str | list[str] | UploadFile] | EmptyType
     flash_messages: list[dict[str, str]]
     headers: Headers | EmptyType
     is_cached: bool | EmptyType

pyproject.toml

@@ -203,6 +203,9 @@ fail_under = 50
 addopts = "--strict-markers --strict-config --dist=loadgroup -m 'not server_integration'"
 asyncio_mode = "auto"
 filterwarnings = [
+  "error",
+  # https://github.com/pytest-dev/pytest-asyncio/issues/724
+  "default:.*socket.socket:pytest.PytestUnraisableExceptionWarning",
   "ignore::trio.TrioDeprecationWarning:anyio._backends._trio*:",
   "ignore::DeprecationWarning:pkg_resources.*",
   "ignore::DeprecationWarning:google.rpc",

tests/e2e/test_routing/conftest.py

@@ -1,4 +1,3 @@
-import subprocess
 import time
 from pathlib import Path
 from typing import Callable, List
@@ -16,16 +15,13 @@ def runner(app: str, server_command: List[str]) -> None:
         tmp_path.joinpath("app.py").write_text(app)
         monkeypatch.chdir(tmp_path)
 
-        proc = psutil.Popen(
-            server_command,
-            stderr=subprocess.PIPE,
-            stdout=subprocess.PIPE,
-        )
+        proc = psutil.Popen(server_command)
 
         def kill() -> None:
             for child in proc.children(recursive=True):
                 child.kill()
             proc.kill()
+            proc.wait()
 
         request.addfinalizer(kill)
 

tests/unit/test_datastructures/test_multi_dicts.py

@@ -1,7 +1,8 @@
 from __future__ import annotations
 
+from unittest.mock import patch
+
 import pytest
-from pytest_mock import MockerFixture
 
 from litestar.datastructures import UploadFile
 from litestar.datastructures.multi_dicts import FormMultiDict, ImmutableMultiDict, MultiDict
@@ -34,20 +35,19 @@ def test_immutable_multi_dict_as_mutable() -> None:
     assert multi.mutable_copy().dict() == MultiDict(data).dict()
 
 
-async def test_form_multi_dict_close(mocker: MockerFixture) -> None:
-    close = mocker.patch("litestar.datastructures.multi_dicts.UploadFile.close")
-
+async def test_form_multi_dict_close() -> None:
     multi = FormMultiDict(
         [
             ("foo", UploadFile(filename="foo", content_type="text/plain")),
             ("bar", UploadFile(filename="foo", content_type="text/plain")),
         ]
     )
-
+    with patch("litestar.datastructures.multi_dicts.UploadFile.close") as mock_close:
+        await multi.close()
+        assert mock_close.call_count == 2
+    # calls the real UploadFile.close method to clean up
     await multi.close()
 
-    assert close.call_count == 2
-
 
 @pytest.mark.parametrize("type_", [MultiDict, ImmutableMultiDict])
 def test_copy(type_: type[MultiDict | ImmutableMultiDict]) -> None:

tests/unit/test_dto/test_factory/test_integration.py

@@ -56,11 +56,13 @@ def handler(data: User = Body(media_type=RequestEncodingType.URL_ENCODED)) -> Us
 
 
 async def test_multipart_encoded_form_data(use_experimental_dto_backend: bool) -> None:
+    default_file = UploadFile(content_type="text/plain", filename="forbidden", file_data=b"forbidden")
+
     @dataclass
     class Payload:
         file: UploadFile
         forbidden: UploadFile = field(
-            default=UploadFile(content_type="text/plain", filename="forbidden", file_data=b"forbidden"),
+            default=default_file,
             metadata=dto_field("read-only"),
         )
 
@@ -78,6 +80,8 @@ async def handler(data: Payload = Body(media_type=RequestEncodingType.MULTI_PART
         )
         assert response.content == b"forbidden"
 
+    await default_file.close()
+
 
 def test_renamed_field(use_experimental_dto_backend: bool) -> None:
     @dataclass

tests/unit/test_plugins/test_pydantic/test_beanie_integration.py

@@ -1,15 +1,13 @@
-from typing import TYPE_CHECKING, Optional, Type
+from typing import Optional
 
 import beanie
-
-if TYPE_CHECKING:
-    from pydantic import BaseModel
+from pydantic import BaseModel
 
 from litestar.plugins.pydantic import PydanticDTO
 
 
-def test_generate_field_definitions_from_beanie_models(base_model: "Type[BaseModel]") -> None:
-    class Category(base_model):  # type: ignore[valid-type, misc]
+def test_generate_field_definitions_from_beanie_models() -> None:
+    class Category(BaseModel):
         name: str
         description: str