From 5dd22cd8effca54b65ed98b2c2d8d4bb862aee44 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 17 Apr 2022 06:13:00 -0500 Subject: [PATCH 01/22] fix shutdown behavior --- jupyter_client/multikernelmanager.py | 41 ++++++++++------------------ 1 file changed, 14 insertions(+), 27 deletions(-) diff --git a/jupyter_client/multikernelmanager.py b/jupyter_client/multikernelmanager.py index ee5863e93..5e281216c 100644 --- a/jupyter_client/multikernelmanager.py +++ b/jupyter_client/multikernelmanager.py @@ -171,8 +171,9 @@ async def _add_kernel_when_ready( try: await kernel_awaitable self._kernels[kernel_id] = km - finally: self._pending_kernels.pop(kernel_id, None) + except Exception as e: + self.log.exception(e) async def _remove_kernel_when_ready( self, kernel_id: str, kernel_awaitable: t.Awaitable @@ -180,8 +181,9 @@ async def _remove_kernel_when_ready( try: await kernel_awaitable self.remove_kernel(kernel_id) - finally: self._pending_kernels.pop(kernel_id, None) + except Exception as e: + self.log.exception(e) def _using_pending_kernels(self): """Returns a boolean; a clearer method for determining if @@ -224,22 +226,6 @@ async def _async_start_kernel(self, kernel_name: t.Optional[str] = None, **kwarg start_kernel = run_sync(_async_start_kernel) - async def _shutdown_kernel_when_ready( - self, - kernel_id: str, - now: t.Optional[bool] = False, - restart: t.Optional[bool] = False, - ) -> None: - """Wait for a pending kernel to be ready - before shutting the kernel down. - """ - # Only do this if using pending kernels - if self._using_pending_kernels(): - kernel = self._kernels[kernel_id] - await kernel.ready - # Once out of a pending state, we can call shutdown. - await ensure_async(self.shutdown_kernel(kernel_id, now=now, restart=restart)) - async def _async_shutdown_kernel( self, kernel_id: str, @@ -258,11 +244,8 @@ async def _async_shutdown_kernel( Will the kernel be restarted? """ self.log.info("Kernel shutdown: %s" % kernel_id) - # If we're using pending kernels, block shutdown when a kernel is pending. - if self._using_pending_kernels() and kernel_id in self._pending_kernels: - raise RuntimeError("Kernel is in a pending state. Cannot shutdown.") # If the kernel is still starting, wait for it to be ready. - elif kernel_id in self._pending_kernels: + if kernel_id in self._pending_kernels: kernel = self._pending_kernels[kernel_id] try: await kernel @@ -320,13 +303,17 @@ async def _async_shutdown_all(self, now: bool = False) -> None: """Shutdown all kernels.""" kids = self.list_kernel_ids() kids += list(self._pending_kernels) - futs = [ensure_async(self._shutdown_kernel_when_ready(kid, now=now)) for kid in set(kids)] + kms = list(self._kernels.values()) + futs = [ensure_async(self.shutdown_kernel(kid, now=now)) for kid in set(kids)] await asyncio.gather(*futs) - # When using "shutdown all", all pending kernels - # should be awaited before exiting this method. + # If using pending kernels, the kernels will not have been fully shut down. if self._using_pending_kernels(): - for km in self._kernels.values(): - await km.ready + for km in kms: + try: + await km.ready + except Exception as e: + # Will have been logged in _add_kernel_when_ready + pass shutdown_all = run_sync(_async_shutdown_all) From e409fd1463ece1a0d479596be098e7132c77ed3c Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 17 Apr 2022 06:17:26 -0500 Subject: [PATCH 02/22] lint --- jupyter_client/multikernelmanager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyter_client/multikernelmanager.py b/jupyter_client/multikernelmanager.py index 5e281216c..a2f898b98 100644 --- a/jupyter_client/multikernelmanager.py +++ b/jupyter_client/multikernelmanager.py @@ -311,7 +311,7 @@ async def _async_shutdown_all(self, now: bool = False) -> None: for km in kms: try: await km.ready - except Exception as e: + except Exception: # Will have been logged in _add_kernel_when_ready pass From 164f45d7d48b5c1401829ee7d01e5b8338a33825 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 17 Apr 2022 06:47:43 -0500 Subject: [PATCH 03/22] fix test and fix loop handling --- jupyter_client/channels.py | 1 + jupyter_client/tests/test_kernelmanager.py | 2 ++ jupyter_client/tests/test_multikernelmanager.py | 8 ++++---- jupyter_client/threaded.py | 3 +++ jupyter_client/utils.py | 2 ++ pyproject.toml | 4 ---- 6 files changed, 12 insertions(+), 8 deletions(-) diff --git a/jupyter_client/channels.py b/jupyter_client/channels.py index 584f26f7a..7be778054 100644 --- a/jupyter_client/channels.py +++ b/jupyter_client/channels.py @@ -109,6 +109,7 @@ def run(self) -> None: loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) loop.run_until_complete(self._async_run()) + loop.close() async def _async_run(self) -> None: """The thread's main activity. Call start() instead.""" diff --git a/jupyter_client/tests/test_kernelmanager.py b/jupyter_client/tests/test_kernelmanager.py index f71d74209..e1198597b 100644 --- a/jupyter_client/tests/test_kernelmanager.py +++ b/jupyter_client/tests/test_kernelmanager.py @@ -194,6 +194,7 @@ async def test_async_signal_kernel_subprocesses(self, name, install, expected): class TestKernelManager: def test_lifecycle(self, km): km.start_kernel(stdout=PIPE, stderr=PIPE) + kc = km.client() assert km.is_alive() is_done = km.ready.done() assert is_done @@ -201,6 +202,7 @@ def test_lifecycle(self, km): assert km.is_alive() km.interrupt_kernel() assert isinstance(km, KernelManager) + kc.stop_channels() km.shutdown_kernel(now=True) assert km.context.closed diff --git a/jupyter_client/tests/test_multikernelmanager.py b/jupyter_client/tests/test_multikernelmanager.py index bf24496e0..cbcfda802 100644 --- a/jupyter_client/tests/test_multikernelmanager.py +++ b/jupyter_client/tests/test_multikernelmanager.py @@ -88,9 +88,11 @@ def _run_lifecycle(km, test_kid=None): assert kid in km.list_kernel_ids() km.interrupt_kernel(kid) k = km.get_kernel(kid) + kc = k.client() assert isinstance(k, KernelManager) km.shutdown_kernel(kid, now=True) assert kid not in km, f"{kid} not in {km}" + kc.stop_channels() def _run_cinfo(self, km, transport, ip): kid = km.start_kernel(stdout=PIPE, stderr=PIPE) @@ -415,10 +417,6 @@ async def test_use_pending_kernels_early_shutdown(self): kernel = km.get_kernel(kid) assert not kernel.ready.done() # Try shutting down while the kernel is pending - with pytest.raises(RuntimeError): - await ensure_future(km.shutdown_kernel(kid, now=True)) - await kernel.ready - # Shutdown once the kernel is ready await ensure_future(km.shutdown_kernel(kid, now=True)) # Wait for the kernel to shutdown await kernel.ready @@ -476,6 +474,7 @@ def tcp_lifecycle_with_loop(self): loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) loop.run_until_complete(self.raw_tcp_lifecycle()) + loop.close() # static so picklable for multiprocessing on Windows @classmethod @@ -491,6 +490,7 @@ def raw_tcp_lifecycle_sync(cls, test_kid=None): loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) loop.run_until_complete(cls.raw_tcp_lifecycle(test_kid=test_kid)) + loop.close() @gen_test async def test_start_parallel_thread_kernels(self): diff --git a/jupyter_client/threaded.py b/jupyter_client/threaded.py index 46de54154..af7d64540 100644 --- a/jupyter_client/threaded.py +++ b/jupyter_client/threaded.py @@ -243,6 +243,9 @@ def stop(self) -> None: self.close() self.ioloop = None + def __del__(self): + self.close() + def close(self) -> None: if self.ioloop is not None: try: diff --git a/jupyter_client/utils.py b/jupyter_client/utils.py index 9dea2bc2e..63e9e3a31 100644 --- a/jupyter_client/utils.py +++ b/jupyter_client/utils.py @@ -4,6 +4,7 @@ - vendor functions from ipython_genutils that should be retired at some point. """ import asyncio +import atexit import inspect import os @@ -14,6 +15,7 @@ def wrapped(*args, **kwargs): loop = asyncio.get_running_loop() except RuntimeError: loop = asyncio.new_event_loop() + atexit.register(loop.close) asyncio.set_event_loop(loop) import nest_asyncio # type: ignore diff --git a/pyproject.toml b/pyproject.toml index 597c47544..e49c08cb9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,10 +41,6 @@ filterwarnings= [ # Fail on warnings "error", - # Workarounds for https://github.com/pytest-dev/pytest-asyncio/issues/77 - "ignore:unclosed Date: Sun, 17 Apr 2022 07:27:53 -0500 Subject: [PATCH 04/22] more cleanup --- jupyter_client/client.py | 5 ++++- jupyter_client/tests/test_multikernelmanager.py | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/jupyter_client/client.py b/jupyter_client/client.py index 7dd64731e..ec42a10ab 100644 --- a/jupyter_client/client.py +++ b/jupyter_client/client.py @@ -116,6 +116,10 @@ def _context_default(self) -> zmq.asyncio.Context: # flag for whether execute requests should be allowed to call raw_input: allow_stdin: bool = True + def __del__(self): + """Destroy our context when we are garbage collected.""" + self.context.destroy() + # -------------------------------------------------------------------------- # Channel proxy methods # -------------------------------------------------------------------------- @@ -320,7 +324,6 @@ def stop_channels(self) -> None: self.control_channel.stop() if self._created_context: self._created_context = False - self.context.destroy() @property def channels_running(self) -> bool: diff --git a/jupyter_client/tests/test_multikernelmanager.py b/jupyter_client/tests/test_multikernelmanager.py index cbcfda802..0437e5349 100644 --- a/jupyter_client/tests/test_multikernelmanager.py +++ b/jupyter_client/tests/test_multikernelmanager.py @@ -160,8 +160,10 @@ def test_start_sequence_ipc_kernels(self): def tcp_lifecycle_with_loop(self): # Ensure each thread has an event loop - asyncio.set_event_loop(asyncio.new_event_loop()) + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) self.test_tcp_lifecycle() + loop.close() def test_start_parallel_thread_kernels(self): self.test_tcp_lifecycle() From bf678f2692b8253247e34e46aabd4a19aec8f9b3 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 17 Apr 2022 08:06:42 -0500 Subject: [PATCH 05/22] fix handling of context --- jupyter_client/client.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/jupyter_client/client.py b/jupyter_client/client.py index ec42a10ab..d29f5b470 100644 --- a/jupyter_client/client.py +++ b/jupyter_client/client.py @@ -93,10 +93,7 @@ class KernelClient(ConnectionFileMixin): # The PyZMQ Context to use for communication with the kernel. context = Instance(zmq.asyncio.Context) - _created_context: Bool = Bool(False) - def _context_default(self) -> zmq.asyncio.Context: - self._created_context = True return zmq.asyncio.Context() # The classes to use for the various channels @@ -290,9 +287,6 @@ def start_channels( :meth:`start_kernel`. If the channels have been stopped and you call this, :class:`RuntimeError` will be raised. """ - # Create the context if needed. - if not self._created_context: - self.context = self._context_default() if iopub: self.iopub_channel.start() if shell: @@ -322,8 +316,6 @@ def stop_channels(self) -> None: self.hb_channel.stop() if self.control_channel.is_alive(): self.control_channel.stop() - if self._created_context: - self._created_context = False @property def channels_running(self) -> bool: From d22789c8f8ba38f1ec3da913202b43ba2a54fe3f Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 17 Apr 2022 08:08:09 -0500 Subject: [PATCH 06/22] fix handling of cov-fail-under --- .github/workflows/main.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 436d8bcaa..92cba0e24 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -71,8 +71,8 @@ jobs: - name: Run the tests if: ${{ !startsWith(matrix.python-version, 'pypy') && !startsWith(matrix.os, 'windows') }} run: | - args="-vv --cov jupyter_client --cov-branch --cov-report term-missing:skip-covered --cov-fail-under 70" - python -m pytest $args || python -m pytest $args --lf + args="-vv --cov jupyter_client --cov-branch --cov-report term-missing:skip-covered" + python -m pytest $args --cov-fail-under 70 || python -m pytest $args --lf - name: Run the tests on pypy and windows if: ${{ startsWith(matrix.python-version, 'pypy') || startsWith(matrix.os, 'windows') }} run: | From f23f9fb19d918f80e0c41be233200fa27bafa698 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 17 Apr 2022 08:19:03 -0500 Subject: [PATCH 07/22] lint --- jupyter_client/client.py | 1 - 1 file changed, 1 deletion(-) diff --git a/jupyter_client/client.py b/jupyter_client/client.py index d29f5b470..c357bd6f9 100644 --- a/jupyter_client/client.py +++ b/jupyter_client/client.py @@ -11,7 +11,6 @@ import zmq.asyncio from traitlets import Any -from traitlets import Bool from traitlets import Instance from traitlets import Type From dc47363d8a1b095455dc99529e9b63dfecae4665 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 18 Apr 2022 04:49:24 -0500 Subject: [PATCH 08/22] try again --- .github/workflows/main.yml | 2 ++ pyproject.toml | 3 +++ 2 files changed, 5 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 92cba0e24..1f9e34a41 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -150,3 +150,5 @@ jobs: steps: - uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1 - uses: jupyterlab/maintainer-tools/.github/actions/test-sdist@v1 + with: + test_command: pytest --vv || pytest -vv --lf diff --git a/pyproject.toml b/pyproject.toml index e49c08cb9..6fe7467c5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,6 +45,9 @@ filterwarnings= [ # (To be fixed in Tornado 6.2) "ignore:There is no current event loop:DeprecationWarning:tornado", + # Randomly occuring. + "ignore:unclosed Date: Mon, 18 Apr 2022 05:12:32 -0500 Subject: [PATCH 09/22] try again --- jupyter_client/client.py | 9 ++++----- pyproject.toml | 7 ++++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/jupyter_client/client.py b/jupyter_client/client.py index c357bd6f9..24f22e857 100644 --- a/jupyter_client/client.py +++ b/jupyter_client/client.py @@ -2,6 +2,7 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. import asyncio +import atexit import sys import time import typing as t @@ -93,7 +94,9 @@ class KernelClient(ConnectionFileMixin): context = Instance(zmq.asyncio.Context) def _context_default(self) -> zmq.asyncio.Context: - return zmq.asyncio.Context() + context = zmq.asyncio.Context() + atexit.register(context.destroy) + return context # The classes to use for the various channels shell_channel_class = Type(ChannelABC) @@ -112,10 +115,6 @@ def _context_default(self) -> zmq.asyncio.Context: # flag for whether execute requests should be allowed to call raw_input: allow_stdin: bool = True - def __del__(self): - """Destroy our context when we are garbage collected.""" - self.context.destroy() - # -------------------------------------------------------------------------- # Channel proxy methods # -------------------------------------------------------------------------- diff --git a/pyproject.toml b/pyproject.toml index 6fe7467c5..b00dd7e4d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,13 +41,14 @@ filterwarnings= [ # Fail on warnings "error", + # We're missing an event loop close somewhere. + "ignore:unclosed Date: Mon, 18 Apr 2022 05:31:04 -0500 Subject: [PATCH 10/22] try to fix ipykernel --- jupyter_client/threaded.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/jupyter_client/threaded.py b/jupyter_client/threaded.py index af7d64540..46de54154 100644 --- a/jupyter_client/threaded.py +++ b/jupyter_client/threaded.py @@ -243,9 +243,6 @@ def stop(self) -> None: self.close() self.ioloop = None - def __del__(self): - self.close() - def close(self) -> None: if self.ioloop is not None: try: From af02fadbd882563d556599e585a594d960abba03 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 18 Apr 2022 05:36:24 -0500 Subject: [PATCH 11/22] try to fix ipykernel again --- jupyter_client/channels.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyter_client/channels.py b/jupyter_client/channels.py index 7be778054..9c98f7c0c 100644 --- a/jupyter_client/channels.py +++ b/jupyter_client/channels.py @@ -109,7 +109,7 @@ def run(self) -> None: loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) loop.run_until_complete(self._async_run()) - loop.close() + # loop.close() async def _async_run(self) -> None: """The thread's main activity. Call start() instead.""" From 8039c95a8b4a11625cfb941c47b2113cc98ad75a Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 18 Apr 2022 05:40:09 -0500 Subject: [PATCH 12/22] try to fix ipykernel again --- jupyter_client/utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/jupyter_client/utils.py b/jupyter_client/utils.py index 63e9e3a31..11a4e67b2 100644 --- a/jupyter_client/utils.py +++ b/jupyter_client/utils.py @@ -4,10 +4,11 @@ - vendor functions from ipython_genutils that should be retired at some point. """ import asyncio -import atexit import inspect import os +# import atexit + def run_sync(coro): def wrapped(*args, **kwargs): @@ -15,7 +16,7 @@ def wrapped(*args, **kwargs): loop = asyncio.get_running_loop() except RuntimeError: loop = asyncio.new_event_loop() - atexit.register(loop.close) + # atexit.register(loop.close) asyncio.set_event_loop(loop) import nest_asyncio # type: ignore From 7754880f28c49f9205e8c92e0e5920aa76d8b789 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 18 Apr 2022 05:48:57 -0500 Subject: [PATCH 13/22] restore other loop closes --- jupyter_client/channels.py | 2 +- jupyter_client/threaded.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/jupyter_client/channels.py b/jupyter_client/channels.py index 9c98f7c0c..7be778054 100644 --- a/jupyter_client/channels.py +++ b/jupyter_client/channels.py @@ -109,7 +109,7 @@ def run(self) -> None: loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) loop.run_until_complete(self._async_run()) - # loop.close() + loop.close() async def _async_run(self) -> None: """The thread's main activity. Call start() instead.""" diff --git a/jupyter_client/threaded.py b/jupyter_client/threaded.py index 46de54154..af7d64540 100644 --- a/jupyter_client/threaded.py +++ b/jupyter_client/threaded.py @@ -243,6 +243,9 @@ def stop(self) -> None: self.close() self.ioloop = None + def __del__(self): + self.close() + def close(self) -> None: if self.ioloop is not None: try: From 834b83f404c289113e5a506d5b68cabf213d6192 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 18 Apr 2022 05:59:47 -0500 Subject: [PATCH 14/22] cleanup --- jupyter_client/utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/jupyter_client/utils.py b/jupyter_client/utils.py index 11a4e67b2..fff111f3b 100644 --- a/jupyter_client/utils.py +++ b/jupyter_client/utils.py @@ -16,7 +16,6 @@ def wrapped(*args, **kwargs): loop = asyncio.get_running_loop() except RuntimeError: loop = asyncio.new_event_loop() - # atexit.register(loop.close) asyncio.set_event_loop(loop) import nest_asyncio # type: ignore From ee2be0d06e665d388b0007e185ac26b44dc312cc Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 18 Apr 2022 06:04:08 -0500 Subject: [PATCH 15/22] cleanup --- jupyter_client/utils.py | 2 -- pyproject.toml | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/jupyter_client/utils.py b/jupyter_client/utils.py index fff111f3b..9dea2bc2e 100644 --- a/jupyter_client/utils.py +++ b/jupyter_client/utils.py @@ -7,8 +7,6 @@ import inspect import os -# import atexit - def run_sync(coro): def wrapped(*args, **kwargs): diff --git a/pyproject.toml b/pyproject.toml index b00dd7e4d..24e16a3db 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,7 +41,7 @@ filterwarnings= [ # Fail on warnings "error", - # We're missing an event loop close somewhere. + # We need to handle properly closing loops as part of https://github.com/jupyter/jupyter_client/issues/755. "ignore:unclosed Date: Thu, 21 Apr 2022 09:57:11 -0500 Subject: [PATCH 16/22] wip --- jupyter_client/client.py | 5 ++-- jupyter_client/multikernelmanager.py | 43 ++++++++++++++-------------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/jupyter_client/client.py b/jupyter_client/client.py index 24f22e857..b789bafaf 100644 --- a/jupyter_client/client.py +++ b/jupyter_client/client.py @@ -2,10 +2,10 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. import asyncio -import atexit import sys import time import typing as t +import weakref from functools import partial from getpass import getpass from queue import Empty @@ -95,7 +95,8 @@ class KernelClient(ConnectionFileMixin): def _context_default(self) -> zmq.asyncio.Context: context = zmq.asyncio.Context() - atexit.register(context.destroy) + # Use a finalizer to destroy the context. + self._finalizer = weakref.finalize(self, context.destroy) return context # The classes to use for the various channels diff --git a/jupyter_client/multikernelmanager.py b/jupyter_client/multikernelmanager.py index a2f898b98..e39e5286f 100644 --- a/jupyter_client/multikernelmanager.py +++ b/jupyter_client/multikernelmanager.py @@ -6,6 +6,7 @@ import socket import typing as t import uuid +import weakref import zmq from traitlets import Any @@ -95,8 +96,6 @@ def create_kernel_manager(*args: Any, **kwargs: Any) -> KernelManager: help="Share a single zmq.Context to talk to all my kernels", ).tag(config=True) - _created_context = Bool(False) - context = Instance("zmq.Context") _pending_kernels = Dict() @@ -108,20 +107,18 @@ def _starting_kernels(self): @default("context") # type:ignore[misc] def _context_default(self) -> zmq.Context: - self._created_context = True - return zmq.Context() - - def __del__(self): - if self._created_context and self.context and not self.context.closed: - if self.log: - self.log.debug("Destroying zmq context for %s", self) - self.context.destroy() - try: - super_del = super().__del__ - except AttributeError: - pass - else: - super_del() + context = zmq.Context() + # Use a finalizer to destroy the context. + # self._finalizer = weakref.finalize(self, context.destroy) + + raise ValueError('Notes here') + """ + The finalizer hangs because the context can't be destroyed. + There are two other things that are probably contributing: + - There is an open kernel subprocess at shutdown that is raising a warning on __del__ + - We are not properly canceling the _add_kernel_when_ready task + """ + return context connection_dir = Unicode("") @@ -209,15 +206,15 @@ async def _async_start_kernel(self, kernel_name: t.Optional[str] = None, **kwarg kwargs['kernel_id'] = kernel_id # Make kernel_id available to manager and provisioner starter = ensure_async(km.start_kernel(**kwargs)) - fut = asyncio.ensure_future(self._add_kernel_when_ready(kernel_id, km, starter)) - self._pending_kernels[kernel_id] = fut + task = asyncio.create_task(self._add_kernel_when_ready(kernel_id, km, starter)) + self._pending_kernels[kernel_id] = task # Handling a Pending Kernel if self._using_pending_kernels(): # If using pending kernels, do not block # on the kernel start. self._kernels[kernel_id] = km else: - await fut + await task # raise an exception if one occurred during kernel startup. if km.ready.exception(): raise km.ready.exception() # type: ignore @@ -246,9 +243,9 @@ async def _async_shutdown_kernel( self.log.info("Kernel shutdown: %s" % kernel_id) # If the kernel is still starting, wait for it to be ready. if kernel_id in self._pending_kernels: - kernel = self._pending_kernels[kernel_id] + task = self._pending_kernels[kernel_id] try: - await kernel + await task km = self.get_kernel(kernel_id) await t.cast(asyncio.Future, km.ready) except Exception: @@ -311,7 +308,9 @@ async def _async_shutdown_all(self, now: bool = False) -> None: for km in kms: try: await km.ready - except Exception: + except asyncio.exceptions.CancelledError: + self._pending_kernels[km.kernel_id].cancel() + except Exception as e: # Will have been logged in _add_kernel_when_ready pass From 0a2c2289347771d69cac7dda69e5a29dc4b445a9 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 24 Apr 2022 05:18:51 -0500 Subject: [PATCH 17/22] work around event loop limitation --- jupyter_client/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jupyter_client/utils.py b/jupyter_client/utils.py index 9dea2bc2e..3b14affd5 100644 --- a/jupyter_client/utils.py +++ b/jupyter_client/utils.py @@ -13,8 +13,8 @@ def wrapped(*args, **kwargs): try: loop = asyncio.get_running_loop() except RuntimeError: - loop = asyncio.new_event_loop() - asyncio.set_event_loop(loop) + # Workaround for bugs.python.org/issue39529. + loop = asyncio.get_event_loop_policy().get_event_loop() import nest_asyncio # type: ignore nest_asyncio.apply(loop) From 270cec62c32fefc3789825c21382fd2a7c634c6d Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 24 Apr 2022 06:01:14 -0500 Subject: [PATCH 18/22] more cleanup --- jupyter_client/multikernelmanager.py | 16 +++++----------- jupyter_client/utils.py | 6 +++++- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/jupyter_client/multikernelmanager.py b/jupyter_client/multikernelmanager.py index e39e5286f..6f61221fc 100644 --- a/jupyter_client/multikernelmanager.py +++ b/jupyter_client/multikernelmanager.py @@ -109,15 +109,7 @@ def _starting_kernels(self): def _context_default(self) -> zmq.Context: context = zmq.Context() # Use a finalizer to destroy the context. - # self._finalizer = weakref.finalize(self, context.destroy) - - raise ValueError('Notes here') - """ - The finalizer hangs because the context can't be destroyed. - There are two other things that are probably contributing: - - There is an open kernel subprocess at shutdown that is raising a warning on __del__ - - We are not properly canceling the _add_kernel_when_ready task - """ + self._finalizer = weakref.finalize(self, context.destroy) return context connection_dir = Unicode("") @@ -248,12 +240,14 @@ async def _async_shutdown_kernel( await task km = self.get_kernel(kernel_id) await t.cast(asyncio.Future, km.ready) + except asyncio.exceptions.CancelledError: + pass except Exception: self.remove_kernel(kernel_id) return km = self.get_kernel(kernel_id) # If a pending kernel raised an exception, remove it. - if km.ready.exception(): + if not km.ready.cancelled() and km.ready.exception(): self.remove_kernel(kernel_id) return stopper = ensure_async(km.shutdown_kernel(now, restart)) @@ -310,7 +304,7 @@ async def _async_shutdown_all(self, now: bool = False) -> None: await km.ready except asyncio.exceptions.CancelledError: self._pending_kernels[km.kernel_id].cancel() - except Exception as e: + except Exception: # Will have been logged in _add_kernel_when_ready pass diff --git a/jupyter_client/utils.py b/jupyter_client/utils.py index 3b14affd5..585bf1b17 100644 --- a/jupyter_client/utils.py +++ b/jupyter_client/utils.py @@ -14,7 +14,11 @@ def wrapped(*args, **kwargs): loop = asyncio.get_running_loop() except RuntimeError: # Workaround for bugs.python.org/issue39529. - loop = asyncio.get_event_loop_policy().get_event_loop() + try: + loop = asyncio.get_event_loop_policy().get_event_loop() + except RuntimeError: + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) import nest_asyncio # type: ignore nest_asyncio.apply(loop) From db270dc1a7e36325196d034cdc84c7466f75c8fa Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 24 Apr 2022 06:08:18 -0500 Subject: [PATCH 19/22] fix import --- jupyter_client/multikernelmanager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jupyter_client/multikernelmanager.py b/jupyter_client/multikernelmanager.py index 6f61221fc..6fc382026 100644 --- a/jupyter_client/multikernelmanager.py +++ b/jupyter_client/multikernelmanager.py @@ -240,7 +240,7 @@ async def _async_shutdown_kernel( await task km = self.get_kernel(kernel_id) await t.cast(asyncio.Future, km.ready) - except asyncio.exceptions.CancelledError: + except asyncio.CancelledError: pass except Exception: self.remove_kernel(kernel_id) @@ -302,7 +302,7 @@ async def _async_shutdown_all(self, now: bool = False) -> None: for km in kms: try: await km.ready - except asyncio.exceptions.CancelledError: + except asyncio.CancelledError: self._pending_kernels[km.kernel_id].cancel() except Exception: # Will have been logged in _add_kernel_when_ready From 0168bef37e7c0a09e04f4a4a18dea4c4c931d66e Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 24 Apr 2022 10:54:58 -0500 Subject: [PATCH 20/22] clean up context handling --- jupyter_client/client.py | 11 ++++++----- jupyter_client/multikernelmanager.py | 10 +++++----- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/jupyter_client/client.py b/jupyter_client/client.py index b789bafaf..3ced4f1f4 100644 --- a/jupyter_client/client.py +++ b/jupyter_client/client.py @@ -5,7 +5,6 @@ import sys import time import typing as t -import weakref from functools import partial from getpass import getpass from queue import Empty @@ -94,10 +93,7 @@ class KernelClient(ConnectionFileMixin): context = Instance(zmq.asyncio.Context) def _context_default(self) -> zmq.asyncio.Context: - context = zmq.asyncio.Context() - # Use a finalizer to destroy the context. - self._finalizer = weakref.finalize(self, context.destroy) - return context + return zmq.asyncio.Context() # The classes to use for the various channels shell_channel_class = Type(ChannelABC) @@ -116,6 +112,11 @@ def _context_default(self) -> zmq.asyncio.Context: # flag for whether execute requests should be allowed to call raw_input: allow_stdin: bool = True + def __del__(self): + """Clean up the context when garbage collected.""" + if not self.channels_running: + self.context.destroy() + # -------------------------------------------------------------------------- # Channel proxy methods # -------------------------------------------------------------------------- diff --git a/jupyter_client/multikernelmanager.py b/jupyter_client/multikernelmanager.py index 6fc382026..076f7a57e 100644 --- a/jupyter_client/multikernelmanager.py +++ b/jupyter_client/multikernelmanager.py @@ -6,7 +6,6 @@ import socket import typing as t import uuid -import weakref import zmq from traitlets import Any @@ -107,15 +106,16 @@ def _starting_kernels(self): @default("context") # type:ignore[misc] def _context_default(self) -> zmq.Context: - context = zmq.Context() - # Use a finalizer to destroy the context. - self._finalizer = weakref.finalize(self, context.destroy) - return context + return zmq.Context() connection_dir = Unicode("") _kernels = Dict() + def __del__(self): + """Clean up the context when garbage collected.""" + self.context.destroy() + def list_kernel_ids(self) -> t.List[str]: """Return a list of the kernel ids of the active kernels.""" # Create a copy so we can iterate over kernels in operations From a50d602bbab0e62085fc6772f458425e5d5ee662 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 24 Apr 2022 11:00:27 -0500 Subject: [PATCH 21/22] more context cleanup --- jupyter_client/client.py | 15 +++++++++++++-- jupyter_client/multikernelmanager.py | 15 +++++++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/jupyter_client/client.py b/jupyter_client/client.py index 3ced4f1f4..a56a9ec30 100644 --- a/jupyter_client/client.py +++ b/jupyter_client/client.py @@ -11,6 +11,7 @@ import zmq.asyncio from traitlets import Any +from traitlets import Bool from traitlets import Instance from traitlets import Type @@ -92,7 +93,10 @@ class KernelClient(ConnectionFileMixin): # The PyZMQ Context to use for communication with the kernel. context = Instance(zmq.asyncio.Context) + _created_context = Bool(False) + def _context_default(self) -> zmq.asyncio.Context: + self._created_context = True return zmq.asyncio.Context() # The classes to use for the various channels @@ -113,9 +117,16 @@ def _context_default(self) -> zmq.asyncio.Context: allow_stdin: bool = True def __del__(self): - """Clean up the context when garbage collected.""" - if not self.channels_running: + if self._created_context and self.context and not self.context.closed: + if self.log: + self.log.debug("Destroying zmq context for %s", self) self.context.destroy() + try: + super_del = super().__del__ + except AttributeError: + pass + else: + super_del() # -------------------------------------------------------------------------- # Channel proxy methods diff --git a/jupyter_client/multikernelmanager.py b/jupyter_client/multikernelmanager.py index 076f7a57e..daa5a6170 100644 --- a/jupyter_client/multikernelmanager.py +++ b/jupyter_client/multikernelmanager.py @@ -97,6 +97,8 @@ def create_kernel_manager(*args: Any, **kwargs: Any) -> KernelManager: context = Instance("zmq.Context") + _created_context = Bool(False) + _pending_kernels = Dict() @property @@ -106,6 +108,7 @@ def _starting_kernels(self): @default("context") # type:ignore[misc] def _context_default(self) -> zmq.Context: + self._created_context = True return zmq.Context() connection_dir = Unicode("") @@ -113,8 +116,16 @@ def _context_default(self) -> zmq.Context: _kernels = Dict() def __del__(self): - """Clean up the context when garbage collected.""" - self.context.destroy() + if self._created_context and self.context and not self.context.closed: + if self.log: + self.log.debug("Destroying zmq context for %s", self) + self.context.destroy() + try: + super_del = super().__del__ + except AttributeError: + pass + else: + super_del() def list_kernel_ids(self) -> t.List[str]: """Return a list of the kernel ids of the active kernels.""" From 523ba88d74bdce522c9804842e77bb02cef619f1 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 24 Apr 2022 11:02:55 -0500 Subject: [PATCH 22/22] more context cleanup --- jupyter_client/client.py | 11 ++++++++--- jupyter_client/multikernelmanager.py | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/jupyter_client/client.py b/jupyter_client/client.py index a56a9ec30..5ffa069d9 100644 --- a/jupyter_client/client.py +++ b/jupyter_client/client.py @@ -117,10 +117,15 @@ def _context_default(self) -> zmq.asyncio.Context: allow_stdin: bool = True def __del__(self): + """Handle garbage collection. Destroy context if applicable.""" if self._created_context and self.context and not self.context.closed: - if self.log: - self.log.debug("Destroying zmq context for %s", self) - self.context.destroy() + if self.channels_running: + if self.log: + self.log.warning("Could not destroy zmq context for %s", self) + else: + if self.log: + self.log.debug("Destroying zmq context for %s", self) + self.context.destroy() try: super_del = super().__del__ except AttributeError: diff --git a/jupyter_client/multikernelmanager.py b/jupyter_client/multikernelmanager.py index daa5a6170..aec04d546 100644 --- a/jupyter_client/multikernelmanager.py +++ b/jupyter_client/multikernelmanager.py @@ -116,6 +116,7 @@ def _context_default(self) -> zmq.Context: _kernels = Dict() def __del__(self): + """Handle garbage collection. Destroy context if applicable.""" if self._created_context and self.context and not self.context.closed: if self.log: self.log.debug("Destroying zmq context for %s", self)