GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix connect_read_pipe/connect_write_pipe double-close of the underlying fd by jaideeppyne · Pull Request #764 · MagicStack/uvloop

Conversation

@jaideeppyne

@jaideeppyne jaideeppyne commented Sep 16, 2026

Copy link
Copy Markdown

Problem

connect_read_pipe and connect_write_pipe passed the Python file object's fd to uv_pipe_open. libuv takes ownership and closes that fd in uv_close. The attached file object later called close() on the same number. If the kernel had reused the fd, this closed an unrelated descriptor.

This is the pipe equivalent of the socket double-close fixed in d5195d7. The EBADF on non-socket close() is not always benign.

The same race exists if the transport is GC'd instead of explicitly closed.

Fix

os.dup() the fd before uv_pipe_open so libuv and the Python file object each own a distinct descriptor. uv_close() closes the dup; fileobj.close() (or GC) closes the original. Either order is safe.

Tests

tests/test_pipes.py adds a probe (from the issue) that opens a fresh os.pipe() inside the file object's close() and fails if those fds are stolen. Covered for both read and write pipes.

Fixes #763

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@jaideeppyne
Fix pipe transport double-close of the underlying fd
2918926 
connect_read_pipe and connect_write_pipe passed the Python file
object's fd to libuv, which takes ownership and closes it. Closing
the file object later (or GC) then closed the same fd again, which
can steal a recycled descriptor.

Dup the fd before uv_pipe_open so libuv and the file object each
own a distinct descriptor.
@fantix

fantix commented Oct 1, 2026

Copy link
Copy Markdown
Member

Thanks for the PR! But I'm not sure dup() is the right way here.

@1st1 previously explained that duplication can make uvloop exhaust FD limits sooner than standard asyncio. 318e593 subsequently removed duplication, including from these two APIs.

I'd like to preserve your regression tests and resolve the ownership conflict without reintroducing duplication. That means we shall either patch libuv to avoid closing the FD conditionally, or replicate uv_pipe_t in uvloop with polling. Neither sounds like a good approach for maintenance, though. @1st1 wdyt?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jaideeppyne

jaideeppyne commented Oct 3, 2026

Copy link
Copy Markdown
Author

Thanks for the context. I agree that restoring dup() globally is not a good tradeoff if it recreates the earlier FD-pressure problem. The regression tests are independent of that implementation choice, so I’m happy to keep them and adapt the fix once the preferred ownership model is settled. I’ll hold off on further code changes while @1st1 weighs in rather than committing the project to either a libuv patch or a custom polling implementation prematurely.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

connect_read_pipe double-closes the underlying fd, possibly closing an unrelated fd

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@jaideeppyne @fantix
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.