Fix connect_read_pipe/connect_write_pipe double-close of the underlying fd - #764
jaideeppyne wants to merge 1 commit into
Conversation
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.
|
Thanks for the PR! But I'm not sure @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 |
|
Thanks for the context. I agree that restoring |
Problem
connect_read_pipeandconnect_write_pipepassed the Python file object's fd touv_pipe_open. libuv takes ownership and closes that fd inuv_close. The attached file object later calledclose()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 beforeuv_pipe_openso 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.pyadds a probe (from the issue) that opens a freshos.pipe()inside the file object'sclose()and fails if those fds are stolen. Covered for both read and write pipes.Fixes #763