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@782f31f:

Merge remote-tracking branch 'origin/main' into 1587-python-support-quit-to-exit-a-runtime

--------------------
Commit message for posit-dev/positron-python@eae7034:

formatting

--------------------
Commit message for posit-dev/positron-python@ec9ebc5:

fix `quit` hanging on python 3.12

--------------------
Commit message for posit-dev/positron-python@922b300:

stop the lsp server thread during kernel shutdown

Addresses #1587.

--------------------
Commit message for posit-dev/positron-python@2e08c93:

initiate kernel shutdown sequence on `quit` and `exit`


Authored-by: Wasim Lorgat <[email protected]>
Signed-off-by: Wasim Lorgat <[email protected]>

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()

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):
     """

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