Repository navigation
Calling os.stat() on a named pipe used by asyncio.ProactorEventLoop.start_serving_pipe() will raise OSError #100573
Description
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
Activity
eryksun commentedon Dec 28, 2022
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, theConnectNamedPipe()call fails withERROR_NO_DATA(232), for whichBrokenPipeErroris raised1. Anos.stat()client disconnects immediately as soon as it determines that it connected to a pipe file or directory in the named-pipe filesystem2.The
BrokenPipeErrorcan be handled inloop_accept_pipe(). Simply close the pipe, and then continue viaself.call_soon(loop_accept_pipe).Footnotes
For some reason that I can't explain, the NT status code
STATUS_PIPE_CLOSINGis mapped to the Windows error codeERROR_NO_DATAinstead ofERROR_BROKEN_PIPE. Python thus maps bothERROR_BROKEN_PIPEandERROR_NO_DATAto POSIXEPIPE, for whichBrokenPipeErroris raised. ↩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 theFileBasicInfo. 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 becauseos.stat()is the only user of the pipe instance. ↩gvanrossum commentedon Dec 28, 2022
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 commentedon Dec 28, 2022
This issue can be resolved by handling
BrokenPipeErrorwhen it's raised by theaccept_pipe()call inloop_accept_pipe(). For example:gvanrossum commentedon Dec 28, 2022
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
OSErrorhere -- the code seems to assume that's pretty bad, because it stops looping (when all goes well,loop_accept_pipeseems to loop viaf.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 thatpipe.fileno() != -1before callingpipe.close()-- that seems an oversight too.PS2. Maybe this could be rewritten without the callback function, using a
Taskinstead??gvanrossum commentedon Jan 11, 2023
OK.
loop_accept_pipe()is a callback, not a coroutine, I don't see how it could even receive aCancelledError, but that code block is as old as asyncio, so I'm not touching that.Task.OSErrorwe should insertself.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").kumaraditya303 commentedon Jan 12, 2023
I ran the reproducer and verified that PR fixes this. I am OK with adding
self.call_soon(loop_accept_pipe)toOSErrorclause, trying again is better than just hanging the server.GH-100573: Fix server hang caused by os.stat() on named pipe (Windows) (
pythonGH-100573: Fix server hang caused by os.stat() on named pipe (W…
pythonGH-100573: Fix server hang caused by os.stat() on named pipe (W…
[3.10] GH-100573: Fix server hang caused by os.stat() on named pipe (…
[3.11] GH-100573: Fix server hang caused by os.stat() on named pipe (…
teppey commentedon Jan 18, 2023
Thank you for investigation and correction. I have confirmed that the change in #100959 resolves the issue.
capture_opts: Don't call stat on Windows to see if a filename is a Na…