python / cpython

The Python programming language
https://www.python.org
Other
62.29k stars 29.93k forks source link

ProactorEventLoop doesn't support stdin/stdout nor files with connect_read_pipe/connect_write_pipe #71019

Open 33c119a3-a693-466b-94f3-780522e618f3 opened 8 years ago

33c119a3-a693-466b-94f3-780522e618f3 commented 8 years ago
BPO 26832
Nosy @terryjreedy, @pfmoore, @mjpieters, @tjguk, @zware, @1st1, @zooba, @minrk

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['3.8', '3.9', '3.7', 'expert-asyncio', 'type-bug', '3.10', '3.11', 'OS-windows'] title = "ProactorEventLoop doesn't support stdin/stdout nor files with connect_read_pipe/connect_write_pipe" updated_at = user = 'https://bugs.python.org/GabrielMesquitaCangussu' ``` bugs.python.org fields: ```python activity = actor = 'minrk' assignee = 'none' closed = False closed_date = None closer = None components = ['Windows', 'asyncio'] creation = creator = 'Gabriel Mesquita Cangussu' dependencies = [] files = [] hgrepos = [] issue_num = 26832 keywords = [] message_count = 7.0 messages = ['264043', '264044', '264516', '264523', '264599', '325001', '413781'] nosy_count = 9.0 nosy_names = ['terry.reedy', 'paul.moore', 'mjpieters', 'tim.golden', 'zach.ware', 'yselivanov', 'steve.dower', 'minrk', 'Gabriel Mesquita Cangussu'] pr_nums = [] priority = 'normal' resolution = None stage = None status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue26832' versions = ['Python 3.5', 'Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10', 'Python 3.11'] ```

33c119a3-a693-466b-94f3-780522e618f3 commented 8 years ago

The documentation of asyncio specifies that the methods connect_read_pipe and connect_write_pipe are available on Windows with the ProactorEventLoop. The documentation then says that those methods accept file-like objects on the pipe parameter. However, the methods doesn't seem to work with stdio or any disk file under Windows. The following example catches this problem:

import asyncio
import sys

class MyProtocol(asyncio.Protocol):
    def connection_made(self, transport):
        print('connection established')

    def data_received(self, data):
        print('received: {!r}'.format(data.decode()))

    def connection_lost(self, exc):
        print('lost connection')

if sys.platform.startswith('win32'):
    loop = asyncio.ProactorEventLoop()
else:
    loop = asyncio.SelectorEventLoop()
coro = loop.connect_read_pipe(MyProtocol, sys.stdin)
loop.run_until_complete(coro)
loop.run_forever()
loop.close()

This code when executed on Ubuntu have the desired behavior, but under Windows 10 it gives OSError: [WinError 6] The handle is invalid. The complete output is this:

c:\Users\Gabriel\Documents\Python Scripts>python async_pipe.py
connection established
Fatal read error on pipe transport
protocol: <__main__.MyProtocol object at 0x000001970EB2FAC8>
transport: <_ProactorReadPipeTransport fd=0>
Traceback (most recent call last):
  File "C:\Program Files\Python35\lib\asyncio\proactor_events.py", line 195, in _loop_reading
    self._read_fut = self._loop._proactor.recv(self._sock, 4096)
  File "C:\Program Files\Python35\lib\asyncio\windows_events.py", line 425, in recv
    self._register_with_iocp(conn)
  File "C:\Program Files\Python35\lib\asyncio\windows_events.py", line 606, in _register_with_iocp
    _overlapped.CreateIoCompletionPort(obj.fileno(), self._iocp, 0, 0)
OSError: [WinError 6] Identificador inválido
lost connection

I think that the documentation should state that there is no support for disk files and stdio with the methods in question and also state what exactly they support (an example would be nice). And, of course, better support for watching file descriptors on Windows on future Python releases would be nice.

Thank you, Gabriel

zooba commented 8 years ago

File "C:\Program Files\Python35\lib\asyncio\windows_events.py", line 606, in _register_with_iocp _overlapped.CreateIoCompletionPort(obj.fileno(), self._iocp, 0, 0)

I don't think there's any case where this is going to work on an actual file - CreateIoCompletionPort needs a handle and not a file descriptor.

Presumably this line of code normally gets used with something else that defines fileno()?

terryjreedy commented 8 years ago

The surprise to me, being on Windows, is that the pipe connection methods sometimes work with non-pipes. The limitations of Windows event loops are given in https://docs.python.org/3/library/asyncio-eventloops.html#windows. The pipe connection functions are discussed in https://docs.python.org/3/library/asyncio-eventloop.html#connect-pipes. Both say that the methods do not work with Windows' SelectorEventLoop.

My understanding is that this is because Windows' select() call does not work with pipes -- meaning honest-to-goodness OS pipes. So I understood "*pipe* is file-like object." more as a un-surprising statement of fact than as a permissive "'pipe' can be any file-like object and not necessarily a pipe".

If 'pipe' were intended to mean 'file-like', then why use 'pipe'? But I can see how a current unix user would understand that sentence the other way. Perhaps the sentence should read "For SelectorEventLoops (not on Windows), *pipe* can also be any file-like object with the appropriate methods." -- assuming that this is true on all non-Windows systems.

Isn't there some other way to asynchronously read/file files, as opposed to sockets and pipes, on Windows?

zooba commented 8 years ago

Pipes and file handles are equivalent in Windows, but socket handles are their own namespace and have their own functions. Some Python code will switch between them automatically to make socket functions work with file descriptors, but generally I'd expect stream pipes (as opposed to, IIRC, datagram pipes) to be compatible with files but not sockets.

Not entirely sure how that plays into this issue, but it's a bit more background.

33c119a3-a693-466b-94f3-780522e618f3 commented 8 years ago

Terry,

About your question, "Isn't there some other way to asynchronously read/file files, as opposed to sockets and pipes, on Windows?", that is exactly the problem I was trying to overcome, specifically for reading the stdin.

On my research I found both this stackoverflow questions with no valid answers:

http://stackoverflow.com/questions/31510190/aysncio-cannot-read-stdin-on-windows

http://stackoverflow.com/questions/31511563/file-to-socket-adapter-in-python

And I understood that 'pipe' wouldn't mean a 'file', but why can't I use 'sys.stdin' with 'ProactorEventLoop.connect_read_pipe()', can't 'sys.stdin' be a pipe on Windows like it can on Posix?

652782da-b5f3-46b2-ae1b-1f73662bb759 commented 6 years ago

I'm trying to figure out why Windows won't let us do this. I think the reason is that sys.std(in|out) filehandles are not opened as pipes, and do not have the required OVERLAPPED flag set (see the CreateIoCompletionPort documentation at https://docs.microsoft.com/en-us/windows/desktop/fileio/createiocompletionport; it's that function that is used to handle pipes (via IocpProactor.recv -> IocpProactor._register_with_iocp -> overlapped.CreateIoCompletionPort).

The solution then would be to create a pipe for a stdio filehandle with the flag set.

And that's where my Windows-fu ends, and where I lack the VM and motivation to go try that out.

97d49212-90f1-472f-a5f7-34cb9d6fce6c commented 2 years ago

It appears that connect_read_pipe also doesn't accept pipes returned by os.pipe. If that's the case, what does ProactorEventLoop.connect_read_pipe accept? I haven't been able to find any examples of connect_read_pipe that work on Windows, and every connect_read_pipe call in the cpython test suite appears to be skipped on win32. Should it still be raising NotImplementedError on ProactorEventLoop?

I think the error handling could be better (I only get logged errors, nothing I can catch/handle). It seems like connect_read_pipe itself should raise when it fails to register the pipe with IOCP. If that's not feasible, connection_lost/transport.close should probably be triggered, but it isn't with Python 3.9, at least.

Example that works on posix, but seems to fail with non-catchable errors with ProactorEventLoop:

import asyncio
import os
import sys

class PipeProtocol(asyncio.Protocol):
    def __init__(self):
        self.finished = asyncio.Future()

    def connection_made(self, transport):
        print("connection made", file=sys.stderr)
        self.transport = transport

    def connection_lost(self, exc):
        print("connection lost", exc, file=sys.stderr)
        self.finished.set_result(None)

    def data_received(self, data):
        print("data received", data, file=sys.stderr)
        self.handler(data)

    def eof_received(self):
        print("eof received", file=sys.stderr)
        self.finished.set_result(None)

async def test():
    r, w = os.pipe()
    rf = os.fdopen(r, 'r')
    x, p = await asyncio.get_running_loop().connect_read_pipe(PipeProtocol, rf)
    await asyncio.sleep(1)
    print("writing")
    os.write(w, b'asdf')
    await asyncio.sleep(2)
    print("closing")
    os.close(w)
    await asyncio.wait([p.finished], timeout=3)
    x.close()

if __name__ == "__main__":
    asyncio.run(test())
kumaraditya303 commented 1 year ago

FWIW this is an OS limitation and I doubt that anything can be done to get this working. It is documented that file io cannot be monitored with asyncio. I'll close this unless anyone has a concrete implementation to make this work.

gvanrossum commented 1 year ago

I think the OP nailed it with "I think that the documentation should state that there is no support for disk files and stdio with the methods in question and also state what exactly they support (an example would be nice)." Currently the docs are just no help at all.

kumaraditya303 commented 1 year ago

On Windows, sockets and named pipes are supported, on Linux fifo, sockets, pipes and character devices are supported, no idea about macOS. Someone would have to research more.

TBBle commented 1 year ago

ProactorEventLoop on Windows puts such things through the CreateIoCompletionPort API, which supports:

a handle opened by the CreateFile function using the FILE_FLAG_OVERLAPPED flag (for example, files, mail slots, and pipes). Objects created by other functions such as socket can also be associated with an I/O completion port

In general files could be opened such that they would work with connect_read_pipe on Windows, although you need to poke at the Win32 API directly to get an appropriate handle to wrap in a file-like object for these APIs.

You can see what this looks like in asyncio.windows_utils's pipe function, and its file-like wrapper in PipeHandle, used here to implement subprocess.PIPE via a random named pipe, and which is then used with connect_read_pipe/connect_write_pipe via BaseSubprocessTransport. In fact, nothing in PipeHandle jumps out at me as pipe-specific so it could probably be used as-is to wrap a handle created like h2 is in pipe there.

So maybe it'd be a useful feature to have an open in asyncio.windows_utils to provide overlapped-enabled file-handles, like pipe there does, to wrap up all that _winapi magic. Although these don't appear to be public APIs, just implementation details of asyncio, so maybe it's better to expect such sophisticated users to reimplement these details themselves? (I see that at some point, there was a socketpair function in asyncio.windows_utils too so clearly this module is not a particularly-stable API.)

As mentioned earlier, stdin/stdout are not opened in a manner that would work for this. There might be a way to change this, but success isn't assured and doing that would probably mess with Python's invariants around stdin/stdout anyway if you didn't also ensure Python wasn't trying to operate on those FDs as well.

gvanrossum commented 1 year ago

@kumaraditya303

On Windows, sockets and named pipes are supported, on Linux fifo, sockets, pipes and character devices are supported, no idea about macOS. Someone would have to research more.

Did you test this or did you just read so in the docs? If you tested it, email me your test program and I'll run it on macOS.

We should probably be careful to distinguish between OS-level abstractions and the Python objects used to wrap these -- e.g.on UNIX, to me "socket" paints a picture in my mind of a file descriptor created with socket(2), whereas these functions probably require socket objects created by Python's socket module.

Thanks @TBBle for the clear description of how this works on Windows!

Once we have the inventory for macOS I think we can make progress with writing the proper docs.

gvanrossum commented 1 year ago

It's in the source:

        if not (stat.S_ISFIFO(mode) or
                stat.S_ISSOCK(mode) or
                stat.S_ISCHR(mode)):
            self._pipe = None
            self._fileno = None
            self._protocol = None
            raise ValueError("Pipe transport is for pipes/sockets only.")

It looks like this covers the same categories as on Linux (I'm guessing a pipe is really a fifo).

EDIT: And it's the same for connect_write_pipe().

So with that, I think I'd be happy to entertain a PR that updates the docs.

kumaraditya303 commented 1 year ago

Did you test this or did you just read so in the docs? If you tested it, email me your test program and I'll run it on macOS.

It is what I have used in the past with asyncio, I didn't know that there was check for it but the check is only for selector not proactor event loop.

xintenseapple commented 3 weeks ago

How is this issue still open? Currently, the ProactorEventLoop docs very clearly (and incorrectly) state that this should work on Windows (in contrast to SelectorEventLoop). At the very least, the asyncio docs need to be updated to clearly indicate that stdin/stdout are not supported on Windows. Frankly, it's probably more appropriate to mark these functions as entirely unsupported on Windows.

An actual solution can be worked towards in the meantime, but the current behavior is wildly confusing.

gvanrossum commented 3 weeks ago

How is this issue still open?

That sounds a bit passive-aggressive. You could be more constructive and submit a PR that updates the docs according to the discussion in this issue.