jupyter / jupyter_console

Jupyter Terminal Console
http://jupyter-console.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
250 stars 146 forks source link

Remove loop=loop in asyncio.wait call. Fix #245 #258

Closed lf- closed 2 years ago

lf- commented 2 years ago

Fixes #245

amerlyq commented 2 years ago

@lf- This PR is not enough. With c.ZMQTerminalInteractiveShell.include_other_output = True it still crashes:

$ jupyter console
Jupyter console 6.4.1dev

Python 3.10.1 (main, Dec 18 2021, 23:53:45) [GCC 11.1.0]
Type 'copyright', 'credits' or 'license' for more information
IPython 7.30.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]:
Shutting down kernel
Task exception was never retrieved
future: <Task finished name='Task-9' coro=<ZMQTerminalInteractiveShell.handle_external_iopub() done, defined at /home/user/.local/lib/python3.10/site-packages/jupyter_console/ptshell.py:853> exception=RuntimeError("There is no current event loop in thread 'asyncio_0'.")>
Traceback (most recent call last):
  File "/usr/lib/python3.10/asyncio/tasks.py", line 234, in __step
    result = coro.throw(exc)
  File "/home/user/.local/lib/python3.10/site-packages/jupyter_console/ptshell.py", line 857, in handle_external_iopub
    poll_result = await loop.run_in_executor(
  File "/usr/lib/python3.10/asyncio/futures.py", line 284, in __await__
    yield self  # This tells Task to wait for completion.
  File "/usr/lib/python3.10/asyncio/tasks.py", line 304, in __wakeup
    future.result()
  File "/usr/lib/python3.10/asyncio/futures.py", line 201, in result
    raise self._exception
  File "/usr/lib/python3.10/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/lib/python3.10/site-packages/zmq/_future.py", line 306, in poll
    f = p.poll(timeout)
  File "/usr/lib/python3.10/site-packages/zmq/_future.py", line 61, in poll
    future = self._Future()
  File "/usr/lib/python3.10/asyncio/events.py", line 656, in get_event_loop
    raise RuntimeError('There is no current event loop in thread %r.'
RuntimeError: There is no current event loop in thread 'asyncio_0'.
lf- commented 2 years ago

That's absolutely bizarre. The implication of run_in_executor further up in that stack is that there is in fact a current event loop, so I really don't know what the issue is. Can someone else help?

NeilGirdhar commented 2 years ago

@tinmarino Any ideas?

tinmarino commented 2 years ago

@NeilGirdhar sry no idea, no time to investigate. It seems that the thread registration has changed. To troubleshoot, see if removing the await keyword works. In that case, maybe divide this line in 2: await loop.run_in_executor. Sry <= in southern summer break

NeilGirdhar commented 2 years ago

@tinmarino Thank you very much for the pointers! Very kind of you. Sorry if my ping was an imposition. I just saw you as the last person to touch these lines. Have a nice summer break, que lo pases bien. :smile:

tinmarino commented 2 years ago

@NeilGirdhar that was fast ! Do not worry, I am actually glad (proud) you called for my point of view 😎. Gracias desde Chile 🐧

ahesford commented 2 years ago

I don't use jupyter for anything, but we're looking at this patch for Void Linux in https://github.com/void-linux/void-packages/pull/35376. Looking very narrowly, this seems to be wrong because loop = asyncio.get_event_loop() will cause the creation of an event loop when called outside of the event loop itself. Prior to Python 3.7, the asyncio.wait(..., loop=loop, ...) call would properly wait within the created event loop. Since 3.8, however, this has been ignored and the wait attempts to run in the current event loop if that exists; I don't know what happens when called outside an event loop (cf. https://docs.python.org/3/library/asyncio-task.html#waiting-primitives). Obviously, with Python 3.10, the interface breaks and this triggers an obvious failure.

Note that loop.run_in_executor may imply that an event loop exists, but the call being run within the executor isn't an async coroutine function but a normal synchronous function (cf. https://docs.python.org/3/library/asyncio-eventloop.html#executing-code-in-thread-or-process-pools) and is probably not aware of an existing asyncio event loop because it isn't running in that context---it is running in an entirely different thread.

I suspect the right fix here is to move the body of the while loop into its own async coroutine function so you can do

loop = asyncio.get_event_loop()
while True:
    loop.run_until_complete(self.do_loop_body())

or something to that effect. This will allow things like asyncio.wait to run in the proper event loop.

davidbrochart commented 2 years ago

Sorry @lf-, I didn't see this PR and I merged the change in #264. Thanks anyway!

amerlyq commented 2 years ago

@davidbrochart

With c.ZMQTerminalInteractiveShell.include_other_output = True it still crashes:

I was able to dirty-fix that by

    async def handle_external_iopub(self, loop=None):
        while self.keep_running:
            # we need to check for keep_running from time to time as
            # we are blocking in an executor block which cannot be cancelled.
            # poll_result = await loop.run_in_executor(
            #     None, self.client.iopub_channel.socket.poll, 500)
            poll_result = await self.client.iopub_channel.socket.poll(500)
            if poll_result:
                self.handle_iopub()

Looks like run_in_executor() uses threadpool by default, and none of its threads have eventloop initialized. Therefore .poll() fails, as Jupyter started to depend on eventloop from some time ago. Not sure if it's proper, but at least it works.

davidbrochart commented 2 years ago

Thanks @amerlyq, I think you fix is the right way to do it. Since we now use an async ZMQ context in jupyter_client, we don't need a thread anymore. We will need to depend on jupyter_client>=7.0.0. I opened #266.