Skip to content

_SelectorSocketTransport.writelines is missing a flow control check allowing writes to fill memory until exhausted #127655

Closed

Description

@bdraco

Activity

  1. added
    type-bugAn unexpected behavior, bug, or error
    on Dec 5, 2024
  2. added
    stdlibStandard Library Python modules in the Lib/ directory
    3.11only security fixes
    3.12only security fixes
    3.13only security fixes
    3.14bugs and security fixes
    on Dec 6, 2024
  3. picnixz commented on Dec 6, 2024

    @picnixz
    Member

    @bdraco I put all security labels but does it affect 3.9/3.10/3.11? (can't check for now)

  4. bdraco commented on Dec 6, 2024

    @bdraco
    ContributorAuthor

    Its 3.12+ only since writelines was implemented in asyncio selector transport in 3.12

  5. 5 remaining items

  6. kumaraditya303 commented on Dec 6, 2024

    @kumaraditya303
    Contributor

    Thanks @bdraco

    I assume aiohttp would be affected by this too if it uses writelines. If possible do share any performance numbers you have of how much impact the zero copy writes have made to aiohttp.

  7. bdraco commented on Dec 6, 2024

    @bdraco
    ContributorAuthor

    Thanks @bdraco

    I assume aiohttp would be affected by this too if it uses writelines. If possible do share any performance numbers you have of how much impact the zero copy writes have made to aiohttp.

    @kumaraditya303

    aio-libs/aiohttp#10126 is the PR from a few hours ago that turned off zero copy writes which was released as part of aiohttp 3.11.10

    The codspeed report shows the degraded performances however its not so great because we were pushing the CI pretty hard at the time and it wasn't keeping up. codspeed is set to only show changes >8% on GH so you'd have to look at the report directly to get a better idea.

    This run https://codspeed.io/aio-libs/aiohttp/runs/675241d947498039ef29339c is probably the best one to look at for aiohttp 3.11.x. For aiohttp master branch https://codspeed.io/aio-libs/aiohttp/branches/disable_zero_copy as its going to show the difference the memcpy makes.

    The relevant benchmarks that show a real difference are (keep in mind we are benchmarking disabling zero copy writes):

    test_one_hundred_get_requests_with_1024_chunked_payload +3% (writelines is a bit worse for tiny payloads)
    test_one_hundred_get_requests_with_512kib_chunked_payload -21% (writelines is a lot better for large payloads)
    test_one_hundred_get_requests_with_30000_chunked_payload -4% (writelines is a slightly better for medium size payloads)

    Note that the margin of error is ~1-3% with the benchmarking (but if you have experience and a bit of wetware memory of previous runs, you can look at the flame-graphs you can usually tell if its error or actual change). Based on that, I'd say the above seems accurate and the error rate here is likely <1% between runs.

    Also benchmarks above were run with 3.12. We switched to benchmarking with 3.13 now that the first patch release, 3.13.1 is out a few hours after these runs. https://codspeed.io/aio-libs/aiohttp/runs/67524ceb47498039ef2933c9 is the first run with Python 3.13 for aiohttp 3.11 and shows a few unrelated (to this issue) improvements. More details in aio-libs/aiohttp#10131

  8. added 2 commits that reference this issue on Dec 6, 2024
  9. bdraco commented on Dec 6, 2024

    @bdraco
    ContributorAuthor

    Closing issue as linked PRs are merged

  10. moved this from Todo to Done in asyncioon Dec 6, 2024
  11. added a commit that references this issue on Dec 6, 2024
  12. added a commit that references this issue on Dec 9, 2024
  13. added a commit that references this issue on Dec 9, 2024
  14. added a commit that references this issue on Dec 9, 2024
  15. added a commit that references this issue on Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    3.12only security fixes3.13only security fixes3.14bugs and security fixesstdlibStandard Library Python modules in the Lib/ directorytopic-asynciotype-bugAn unexpected behavior, bug, or errortype-securityA security issue

    Projects

    • Status
      Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions