Calling os.stat() on a named pipe used by asyncio.ProactorEventLoop.start_serving_pipe() will raise OSError

[teppey]

Bug report

On Windows, several calls to os.stat() on the named pipe used by asyncio.ProactorEventLoop.start_serving_pipe() will result in an OSError. At this time, the first call to os.stat() will cause a BrokenPipeError on the server side.

Steps to reproduce

Run the pipe server in Command Prompt:

>type pipe_server.py
import asyncio

async def start_server():
    pipe = r'\\.\pipe\_test_pipe_server'
    loop = asyncio.get_event_loop()
    await loop.start_serving_pipe(asyncio.Protocol, pipe)
    await asyncio.sleep(3600)

asyncio.run(start_server())

>python pipe_server.py

Run the following script as client on another Command Prompt:

>type pipe_stat.py
import os

pipe = r'\\.\pipe\_test_pipe_server'
os.stat(pipe)

>python pipe_stat.py

Display the following messages on the server side:

Pipe accept failed
pipe: <PipeHandle handle=368>
Traceback (most recent call last):
  File "d:\python_build\Python-3.9.13\lib\asyncio\windows_events.py", line 368, in loop_accept_pipe
    f = self._proactor.accept_pipe(pipe)
  File "d:\python_build\Python-3.9.13\lib\asyncio\windows_events.py", line 636, in accept_pipe
    connected = ov.ConnectNamedPipe(pipe.fileno())
BrokenPipeError: [WinError 232] The pipe is being closed

On client side, run pipe_stat.py repeat several times, then OSError occured:

Traceback (most recent call last):
  File "C:\Users\<user>\Desktop\pipe_stat.py", line 4, in <module>
    os.stat(pipe)
OSError: [WinError 231] All pipe instances are busy: '\\\\.\\pipe\\_test_pipe_server'

This problem seems to stem from commit 9eb3d54 between 3.8.0b3 and 3.8.0b4.

Your environment

• CPython versions tested on: 3.8.0b3+(9eb3d54), 3.8.0, 3.9.13, 3.10.9, 3.11.1
• Operating system and architecture: Windows 10 x64

Linked PRs

• GH-100573: Fix server hang caused by os.stat() on named pipe (Windows) #100959
• [3.11] GH-100573: Fix server hang caused by os.stat() on named pipe (Windows) (GH-100959) #101019
• [3.10] GH-100573: Fix server hang caused by os.stat() on named pipe (Windows) (GH-100959) #101020

[eryksun]

CreateNamedPipeW() creates a pipe instance in a state that allows a client to connect immediately. If the client has already closed the connection when the server tries to manually connect, the ConnectNamedPipe() call fails with ERROR_NO_DATA (232), for which BrokenPipeError is raised¹. An os.stat() client disconnects immediately as soon as it determines that it connected to a pipe file or directory in the named-pipe filesystem².

The BrokenPipeError can be handled in loop_accept_pipe(). Simply close the pipe, and then continue via self.call_soon(loop_accept_pipe).

Footnotes

1. For some reason that I can't explain, the NT status code STATUS_PIPE_CLOSING is mapped to the Windows error code ERROR_NO_DATA instead of ERROR_BROKEN_PIPE. Python thus maps both ERROR_BROKEN_PIPE and ERROR_NO_DATA to POSIX EPIPE, for which BrokenPipeError is raised. ↩

2. GetFileAttributesW() is called to determine whether the path name is a directory. This briefly opens another instance of the pipe. Instead, GetFileInformationByHandleEx() could be called to get the FileBasicInfo. We have to be careful with synchronous operations on a pipe, such as querying file information, but there's no risk of deadlock in this case because os.stat() is the only user of the pipe instance. ↩

[gvanrossum]

No matter what the client does, the server shouldn’t crash. So this is primarily an asyncio bug, right? Changing os.stat() to avoid this wouldn’t be a full fix.

[eryksun]

No matter what the client does, the server shouldn’t crash. So this is primarily an asyncio bug, right?

This issue can be resolved by handling BrokenPipeError when it's raised by the accept_pipe() call in loop_accept_pipe(). For example:

                f = self._proactor.accept_pipe(pipe)
            except BrokenPipeError:
                if pipe and pipe.fileno() != -1:
                    pipe.close()
                self.call_soon(loop_accept_pipe)

[gvanrossum]

I'll have access to a Windows machine next week and I'll try to look into this then. The code looks tricky, and it's old.

I wonder how many other things could cause OSError here -- the code seems to assume that's pretty bad, because it stops looping (when all goes well, loop_accept_pipe seems to loop via f.add_done_callback()).

PS. The code swallows CancelledError, which I think should never happen -- but that's not what this issue is about. Also, in that case it doesn't check that pipe.fileno() != -1 before calling pipe.close() -- that seems an oversight too.

PS2. Maybe this could be rewritten without the callback function, using a Task instead??

[gvanrossum]

OK.

• I confirmed the issue.
• I confirmed that the proposed fix works.
• I looked at the code and since loop_accept_pipe() is a callback, not a coroutine, I don't see how it could even receive a CancelledError, but that code block is as old as asyncio, so I'm not touching that.
• I'm also not going to rewrite this as a Task.
• However, I wonder if at the end of the block handling OSError we should insert self.call_soon(loop_accept_pipe), since that's the reason the bug is so annoying (if the error happens, the server is not closed or stopped, but it is no longer functional, causing the client to report "All pipe instances are busy").
• It needs a test. I can make one based on the OP's repro.

[kumaraditya303]

I ran the reproducer and verified that PR fixes this. I am OK with adding self.call_soon(loop_accept_pipe) to OSError clause, trying again is better than just hanging the server.

[teppey]

Thank you for investigation and correction. I have confirmed that the change in #100959 resolves the issue.