Skip to content

Commit 17fbcbc

Browse files
committed
fix: prevent closed pipe errors on closing asyncio transport resources
The proactor pipe transports for subprocess won't be automatically closed, so "closed pipe" errors (pytest warnings) occur during garbage collection (upon `__del__`). This results in a bunch of pytest warnings whenever closing and freeing up fixture Nvim sessions. A solution is to close all the internal `_ProactorBasePipeTransport` objects later when closing the asyncio event loop. Also, `_ProactorBasePipeTransport.close()` does not close immediately, but rather works asynchronously; therefore the `__del__` finalizer still can throw if called by GC after the event loop is closed. One solution for properly closing the pipe transports is to await the graceful shutdown of these transports. Example CI output (the pytest warnings that are going to be fixed): ``` Exception ignored in: <function _ProactorBasePipeTransport.__del__ at 0x00000205288AD1C0> Traceback (most recent call last): File "C:\hostedtoolcache\windows\Python\3.11.5\x64\Lib\asyncio\proactor_events.py", line 116, in __del__ _warn(f"unclosed transport {self!r}", ResourceWarning, source=self) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\hostedtoolcache\windows\Python\3.11.5\x64\Lib\asyncio\proactor_events.py", line 80, in __repr__ info.append(f'fd={self._sock.fileno()}') ^^^^^^^^^^^^^^^^^^^ File "C:\hostedtoolcache\windows\Python\3.11.5\x64\Lib\asyncio\windows_utils.py", line 102, in fileno raise ValueError("I/O operation on closed pipe") ValueError: I/O operation on closed pipe Exception ignored in: <function _ProactorBasePipeTransport.__del__ at 0x00000205288AD1C0> Traceback (most recent call last): File "C:\hostedtoolcache\windows\Python\3.11.5\x64\Lib\asyncio\proactor_events.py", line 116, in __del__ File "C:\hostedtoolcache\windows\Python\3.11.5\x64\Lib\asyncio\proactor_events.py", line 80, in __repr__ File "C:\hostedtoolcache\windows\Python\3.11.5\x64\Lib\asyncio\windows_utils.py", line 102, in fileno ValueError: I/O operation on closed pipe ```
1 parent 7f60f72 commit 17fbcbc

File tree

1 file changed

+36
-6
lines changed

1 file changed

+36
-6
lines changed

pynvim/msgpack_rpc/event_loop/asyncio.py

+36-6
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,10 @@ def _on_data(data: bytes) -> None:
117117
)
118118
self._protocol = None
119119

120-
# The communication channel (endpoint) created by _connect_*() method.
120+
# The communication channel (endpoint) created by _connect_*() methods,
121+
# where we write request messages to be sent to neovim
121122
self._transport = None
122-
self._raw_transport = None
123+
self._to_close: List[asyncio.BaseTransport] = []
123124
self._child_watcher = None
124125

125126
super().__init__(transport_type, *args, **kwargs)
@@ -161,7 +162,8 @@ async def connect_stdin():
161162
transport, protocol = await self._loop.connect_read_pipe(
162163
self._protocol_factory, pipe)
163164
debug("native stdin connection successful")
164-
del transport, protocol
165+
self._to_close.append(transport)
166+
del protocol
165167
self._loop.run_until_complete(connect_stdin())
166168

167169
# Make sure subprocesses don't clobber stdout,
@@ -200,6 +202,16 @@ async def create_subprocess():
200202
transport.get_pipe_transport(0)) # stdin
201203
self._protocol = protocol
202204

205+
# proactor transport implementations do not close the pipes
206+
# automatically, so make sure they are closed upon shutdown
207+
def _close_later(transport):
208+
if transport is not None:
209+
self._to_close.append(transport)
210+
211+
_close_later(transport.get_pipe_transport(1))
212+
_close_later(transport.get_pipe_transport(2))
213+
_close_later(transport)
214+
203215
# await until child process have been launched and the transport has
204216
# been established
205217
self._loop.run_until_complete(create_subprocess())
@@ -230,10 +242,28 @@ def _stop(self) -> None:
230242

231243
@override
232244
def _close(self) -> None:
233-
# TODO close all the transports
234-
if self._raw_transport is not None:
235-
self._raw_transport.close() # type: ignore[unreachable]
245+
def _close_transport(transport):
246+
transport.close()
247+
248+
# Windows: for ProactorBasePipeTransport, close() doesn't take in
249+
# effect immediately (closing happens asynchronously inside the
250+
# event loop), need to wait a bit for completing graceful shutdown.
251+
if os.name == 'nt' and hasattr(transport, '_sock'):
252+
async def wait_until_closed():
253+
# pylint: disable-next=protected-access
254+
while transport._sock is not None:
255+
await asyncio.sleep(0.01)
256+
self._loop.run_until_complete(wait_until_closed())
257+
258+
if self._transport:
259+
_close_transport(self._transport)
260+
self._transport = None
261+
for transport in self._to_close:
262+
_close_transport(transport)
263+
self._to_close[:] = []
264+
236265
self._loop.close()
266+
237267
if self._child_watcher is not None:
238268
self._child_watcher.close()
239269
self._child_watcher = None

0 commit comments

Comments
 (0)