Skip to content

Commit 1762dde

Browse files
authored
Fix Python control thread hang (#7143)
Address #7142 by dropping the control thread if it doesn't exit after 5 seconds. ### QA Notes Hopefully this reduces flakiness in CI, else we can take another look with the new logs. e2e: @:sessions @:console
1 parent f034cf4 commit 1762dde

File tree

2 files changed

+51
-1
lines changed

2 files changed

+51
-1
lines changed

extensions/positron-python/python_files/posit/positron/positron_ipkernel.py

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,12 @@
3838
from .plots import PlotsService
3939
from .session_mode import SessionMode
4040
from .ui import UiService
41-
from .utils import BackgroundJobQueue, JsonRecord, get_qualname
41+
from .utils import BackgroundJobQueue, JsonRecord, get_qualname, with_logging
4242
from .variables import VariablesService
4343

4444
if TYPE_CHECKING:
4545
from ipykernel.comm.manager import CommManager
46+
from ipykernel.control import ControlThread
4647

4748

4849
class _CommTarget(str, enum.Enum):
@@ -562,6 +563,7 @@ def _showwarning(self, message, category, filename, lineno, file=None, line=None
562563

563564

564565
class PositronIPKernelApp(IPKernelApp):
566+
control_thread: ControlThread | None
565567
kernel: PositronIPyKernel
566568

567569
# Use the PositronIPyKernel class.
@@ -570,6 +572,20 @@ class PositronIPKernelApp(IPKernelApp):
570572
# Positron-specific attributes:
571573
session_mode: SessionMode = SessionMode.trait() # type: ignore
572574

575+
def init_control(self, context):
576+
result = super().init_control(context)
577+
# Should be defined in init_control().
578+
assert self.control_thread is not None
579+
# Add a bunch of debug logging to control thread methods.
580+
# See: https://github.com/posit-dev/positron/issues/7142.
581+
self.control_thread.io_loop.start = with_logging(self.control_thread.io_loop.start)
582+
self.control_thread.io_loop.stop = with_logging(self.control_thread.io_loop.stop)
583+
self.control_thread.io_loop.close = with_logging(self.control_thread.io_loop.close)
584+
self.control_thread.run = with_logging(self.control_thread.run)
585+
self.control_thread.stop = with_logging(self.control_thread.stop)
586+
self.control_thread.join = with_logging(self.control_thread.join)
587+
return result
588+
573589
def init_gui_pylab(self):
574590
# Enable the Positron matplotlib backend if we're not in a notebook.
575591
# If we're in a notebook, use IPython's default backend via the super() call below.
@@ -580,6 +596,23 @@ def init_gui_pylab(self):
580596

581597
return super().init_gui_pylab()
582598

599+
def close(self):
600+
# Stop the control thread if it's still alive. This is also attempted in super().close(),
601+
# but that doesn't timeout on join() so can hang forever if the control thread is stuck.
602+
# See https://github.com/posit-dev/positron/issues/7142.
603+
if self.control_thread and self.control_thread.is_alive():
604+
self.log.debug("Closing control thread")
605+
self.control_thread.stop()
606+
self.control_thread.join(timeout=5)
607+
# If the thread is still alive after 5 seconds, log a warning and drop the reference.
608+
# This leaves the thread dangling, but since it's a daemon thread it won't stop the
609+
# process from exiting.
610+
if self.control_thread.is_alive() and self.control_thread.daemon:
611+
self.log.warning("Control thread did not exit after 5 seconds, dropping it")
612+
self.control_thread = None
613+
614+
super().close()
615+
583616

584617
#
585618
# OSC8 functionality

extensions/positron-python/python_files/posit/positron/utils.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import concurrent.futures
88
import functools
99
import inspect
10+
import logging
1011
import numbers
1112
import pprint
1213
import sys
@@ -31,6 +32,8 @@
3132
)
3233
from urllib.parse import unquote, urlparse
3334

35+
logger = logging.getLogger(__name__)
36+
3437
JsonData = Union[Dict[str, "JsonData"], List["JsonData"], str, int, float, bool, None]
3538
JsonRecord = Dict[str, JsonData]
3639

@@ -497,3 +500,17 @@ def run() -> None:
497500
return debounced
498501

499502
return wrapper
503+
504+
505+
def with_logging(func: Callable):
506+
"""Decorator to log the execution of a function."""
507+
name = get_qualname(func)
508+
509+
@functools.wraps(func)
510+
def wrapper(*args, **kwargs):
511+
logger.debug(f"Calling {name} with args: {args}, kwargs: {kwargs}")
512+
result = func(*args, **kwargs)
513+
logger.debug(f"{name} returned: {result}")
514+
return result
515+
516+
return wrapper

0 commit comments

Comments
 (0)