neovim / pynvim

Python client and plugin host for Nvim
http://pynvim.readthedocs.io/en/latest/
Apache License 2.0
1.47k stars 118 forks source link

Several refactoring on asyncio eventloop implementations #545

Closed wookayin closed 7 months ago

wookayin commented 8 months ago

Comments:

refactor!: completely wipe out pyuv

fix: prevent closed pipe errors on closing asyncio transport

This follows up #543 and eliminates all the pytest warnings due to Proactor eventloop.

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

Other minor code improvements, dependent for the above changes

If you prefer breaking this down into several independent PRs, I can also do it -- please let me know.

wookayin commented 7 months ago

@justinmk I wished this were also included in 0.5.0. But since this is just a matter of refactoring (which was necessary to figure out and fix a very long-standing bug #543) we could include this in later versions (but I'm not sure if it makes much sense to merge towards "minor patch" versions such as 0.5.1 rather than 0.6.0).

While I was working on some more drastic change where we completely remove the asyncio abstraction layer and greenlet (which is no longer necessary in modern python), this would be a good starting point so that I can continue refactoring and improving the codebase.

justinmk commented 7 months ago

While I was working on some more drastic change where we completely remove the asyncio abstraction layer and greenlet (which is no longer necessary in modern python)

💯