-
Notifications
You must be signed in to change notification settings - Fork 450
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move asyncio loop management from plugin to main. #3729
Move asyncio loop management from plugin to main. #3729
Conversation
3f32cf4
to
6072dcb
Compare
6072dcb
to
8e60abc
Compare
I tested the code with Python 3.8 and 3.11 on Windows (3.12 would require me to install Visual C++ first…). It works fine; 3.8 throws an exception if The exception 3.8 threw (I got it to happen twice): [2024-01-06 20:36:36] INFO: auto: Server is shutting down.
Exception ignored in: <function BaseSubprocessTransport.__del__ at 0x000001B2182C09D0>
Traceback (most recent call last):
File "C:\Program Files\Python38\lib\asyncio\base_subprocess.py", line 126, in __del__
self.close()
File "C:\Program Files\Python38\lib\asyncio\base_subprocess.py", line 104, in close
proto.pipe.close()
File "C:\Program Files\Python38\lib\asyncio\proactor_events.py", line 108, in close
self._loop.call_soon(self._call_connection_lost, None)
File "C:\Program Files\Python38\lib\asyncio\base_events.py", line 719, in call_soon
self._check_closed()
File "C:\Program Files\Python38\lib\asyncio\base_events.py", line 508, in _check_closed
raise RuntimeError('Event loop is closed')
RuntimeError: Event loop is closed
Exception ignored in: <function _ProactorBasePipeTransport.__del__ at 0x000001B2182F00D0>
Traceback (most recent call last):
File "C:\Program Files\Python38\lib\asyncio\proactor_events.py", line 115, in __del__
_warn(f"unclosed transport {self!r}", ResourceWarning, source=self)
File "C:\Program Files\Python38\lib\asyncio\proactor_events.py", line 79, in __repr__
info.append(f'fd={self._sock.fileno()}')
File "C:\Program Files\Python38\lib\asyncio\windows_utils.py", line 102, in fileno
raise ValueError("I/O operation on closed pipe")
ValueError: I/O operation on closed pipe |
Decades ago, with my private machine, I went directly from DOS to Linux. I had to use Windows some professionally, but on computers privately used by me, I never had Windows. So I have no Windows machine at my disposal, short of borrowing one. In summary, I feel I'm a less-than-perfect person to debug any Windows-specific exceptions. That said, what is a good way to proceed now? I can offer to remove the special code from Would that be helpful? |
Yeah, please remove the |
969b9a2
to
ec370bc
Compare
There is something weird going on with the tests here. I removed the Not entirely sure how Github works this detail. Maybe some test expectancy from the |
The list of expected checks is defined in repository settings. I removed 3.7 from that list. |
My present plan is to wait for #3740 (which duplicates some of the changes here), and only afterwards clean up this branch (remove the duplicate material and rebase). Update 2024-01-24: Done. This wants to be merged again. |
f51fe1b
to
5e820ae
Compare
5e820ae
to
705e098
Compare
It is maybe better to mention this in a comment by itself: This PR has been |
Pull Request Checklist
Description
The code review remark in my recent PR made me think: Where does loop management really belong?
I think it should not be done by a mere plugin, such as
auto
.Doing so was ok as long as
auto
was the only place where the loop was used. The moment something else used the loop, problems started. This was the case when I wrote my tests. It could also have been the case if some other plugin different fromauto
also decides to use the loop, although many such uses might go through without issues. Still,auto
's implicit insistence "the loop is mine" makes it a somewhat unpleasant neighbor.With this pull request, I make a suggestion how else to organize this:
auto
plugin.main
.auto
module load time and some cleanup code in the middle ofauto
, to a Python-ish context handler.auto
could be cleaned up and looks better now. In particular, the doubtful remark that triggered the code review comment vanished.