From a70348e374d05fb2b75d69295e9ddbede15e188b Mon Sep 17 00:00:00 2001 From: Wasim Lorgat Date: Tue, 31 Oct 2023 10:22:01 +0000 Subject: [PATCH] Merged PR posit-dev/positron-python#240: Support `quit` and `exit` to exit a runtime Merge pull request #240 from posit-dev/1587-python-support-quit-to-exit-a-runtime Support `quit` and `exit` to exit a runtime -------------------- Commit message for posit-dev/positron-python@782f31f1fbd25cd249a8e72021783bc801722969: Merge remote-tracking branch 'origin/main' into 1587-python-support-quit-to-exit-a-runtime -------------------- Commit message for posit-dev/positron-python@eae703411bc3e152b5557239a76508cd1b793d13: formatting -------------------- Commit message for posit-dev/positron-python@ec9ebc5fe0d107fec329199d71ba8d51909b8d51: fix `quit` hanging on python 3.12 -------------------- Commit message for posit-dev/positron-python@922b3009f42164a58cf1ca0e225c80b5c5251ba4: stop the lsp server thread during kernel shutdown Addresses https://github.com/posit-dev/positron/issues/1587. -------------------- Commit message for posit-dev/positron-python@2e08c9357d2618f5a5f3427c731e4f9875dbf4ce: initiate kernel shutdown sequence on `quit` and `exit` Authored-by: Wasim Lorgat Signed-off-by: Wasim Lorgat --- .../pythonFiles/positron/lsp.py | 5 ++- .../pythonFiles/positron/positron_ipkernel.py | 28 ++++++++++++- .../pythonFiles/positron/positron_jedilsp.py | 41 +++++++++++++++---- 3 files changed, 63 insertions(+), 11 deletions(-) diff --git a/extensions/positron-python/pythonFiles/positron/lsp.py b/extensions/positron-python/pythonFiles/positron/lsp.py index 8a84bdac83d..a2d6b55fad1 100644 --- a/extensions/positron-python/pythonFiles/positron/lsp.py +++ b/extensions/positron-python/pythonFiles/positron/lsp.py @@ -47,7 +47,7 @@ def on_comm_open(self, comm, msg: Dict[str, Any]) -> None: logger.warning(f"Could not parse host and port from client address: {client_address}") return - # Start the language server + # Start the language server thread POSITRON.start(lsp_host=host, lsp_port=port, kernel=self._kernel, comm=comm) def receive_message(self, msg: Dict[str, Any]) -> None: @@ -57,6 +57,9 @@ def receive_message(self, msg: Dict[str, Any]) -> None: pass def shutdown(self) -> None: + # Stop the language server thread + POSITRON.stop() + if self._comm is not None: try: self._comm.close() diff --git a/extensions/positron-python/pythonFiles/positron/positron_ipkernel.py b/extensions/positron-python/pythonFiles/positron/positron_ipkernel.py index c8386f4aee2..1269b91434b 100644 --- a/extensions/positron-python/pythonFiles/positron/positron_ipkernel.py +++ b/extensions/positron-python/pythonFiles/positron/positron_ipkernel.py @@ -117,10 +117,36 @@ def init_hooks(self): super().init_hooks() # For paged output, send display_data messages instead of using the legacy "payload" - # functionality of execute_reply messages. The priority of 90 is chosen arbitrary as long + # functionality of execute_reply messages. The priority of 90 is chosen arbitrarily, as long # as its lower than other hooks registered by IPython and ipykernel. self.set_hook("show_in_pager", page.as_hook(page.display_page), 90) + async def _stop(self): + # Initiate the kernel shutdown sequence. + await self.kernel.do_shutdown(restart=False) + + # Stop the main event loop. + self.kernel.io_loop.stop() + + @traitlets.observe("exit_now") + def _update_exit_now(self, change): + """stop eventloop when exit_now fires""" + if change["new"]: + if hasattr(self.kernel, "io_loop"): + loop = self.kernel.io_loop + # --- Start Positron --- + # This is reached when a user types `quit` or `exit` into the Positron Console. + # Perform a full kernel shutdown sequence before stopping the loop. + # TODO: We'll need to update this once Positron has a way for kernels to kick off + # Positron's shutdown sequencing. Currently, this is seen as a kernel crash. + # See: https://github.com/posit-dev/positron/issues/628. + loop.call_later(0.1, self._stop) + # --- End Positron --- + if self.kernel.eventloop: + exit_hook = getattr(self.kernel.eventloop, "exit_hook", None) + if exit_hook: + exit_hook(self.kernel) + class PositronIPyKernel(IPythonKernel): """ diff --git a/extensions/positron-python/pythonFiles/positron/positron_jedilsp.py b/extensions/positron-python/pythonFiles/positron/positron_jedilsp.py index d77453b24b7..7c52d5c0f69 100644 --- a/extensions/positron-python/pythonFiles/positron/positron_jedilsp.py +++ b/extensions/positron-python/pythonFiles/positron/positron_jedilsp.py @@ -143,28 +143,26 @@ def start_tcp(self, host: str, port: int) -> None: """Starts TCP server.""" logger.info("Starting TCP server on %s:%s", host, port) - # --- Start Positron --- # Set the event loop's debug mode. self.loop.set_debug(self._debug) # Use our event loop as the thread's main event loop. asyncio.set_event_loop(self.loop) - # --- End Positron --- self._stop_event = threading.Event() self._server = self.loop.run_until_complete(self.loop.create_server(self.lsp, host, port)) - # --- Start Positron --- # Notify the frontend that the LSP server is ready if self._comm is None: logger.warning("LSP comm was not set, could not send server_started message") else: logger.info("LSP server is ready, sending server_started message") self._comm.send({"msg_type": "server_started", "content": {}}) - # --- End Positron --- + # Run the event loop until the stop event is set. try: - self.loop.run_forever() + while not self._stop_event.is_set(): + self.loop.run_until_complete(asyncio.sleep(1)) except (KeyboardInterrupt, SystemExit): pass finally: @@ -194,19 +192,44 @@ def start( ) self._server_thread.start() - def shutdown(self): + def shutdown(self) -> None: logger.info("Shutting down LSP server thread") - super().shutdown() + + # Below is taken as-is from pygls.server.Server.shutdown to remove awaiting + # server.wait_closed since it is a no-op if called after server.close in <=3.11 and blocks + # forever in >=3.12. See: https://github.com/python/cpython/issues/79033 for more. + if self._stop_event is not None: + self._stop_event.set() + + if self._thread_pool: + self._thread_pool.terminate() + self._thread_pool.join() + + if self._thread_pool_executor: + self._thread_pool_executor.shutdown() + + if self._server: + self._server.close() + # This is where we should wait for the server to close but don't due to the issue + # described above. # Reset the loop and thread reference to allow starting a new server in the same process, # e.g. when a browser-based Positron is refreshed. if not self.loop.is_closed(): - logger.info("Closing the event loop.") self.loop.close() + self.loop = asyncio.new_event_loop() self._server_thread = None - def set_debug(self, debug): + def stop(self) -> None: + """Notify the LSP server thread to stop from another thread.""" + if self._stop_event is None: + logger.warning("Cannot stop the LSP server thread, it was not started") + return + + self._stop_event.set() + + def set_debug(self, debug: bool) -> None: self._debug = debug