Review feedback

py/server/deephaven_internal/auto_completer/__init__.py

@@ -20,14 +20,15 @@
 from jedi import preload_module, Interpreter
 
 
-"""
-For Python 3.9 and 3.10, there is a bug in recursion which can result in a segfault. Lowering this
-limit to 2000 or less seems to mitigate it.
-"""
-MAX_RECURSION_LIMIT = 2000
-
-
 jedi_settings = Completer()
 # warm jedi up a little. We could probably off-thread this.
 preload_module("deephaven")
 Interpreter("", []).complete(1, 0)
+
+
+def set_max_recursion_limit(limit: int) -> None:
+    """
+    Utility method to raise/lower the limit that our autocompletion will impose on Python 3.9 and 3.10.
+    """
+    from . import _completer
+    _completer.MAX_RECURSION_LIMIT = limit

py/server/deephaven_internal/auto_completer/_completer.py

@@ -39,6 +39,13 @@ def __str__(self) -> str:
 }
 
 
+"""
+For Python 3.9 and 3.10, there is a bug in recursion which can result in a segfault. Lowering this
+limit to 2000 or less seems to mitigate it.
+"""
+MAX_RECURSION_LIMIT = 2000
+
+
 def wrap_python(txt: str) -> str:
     """ Wraps a string in a Python fenced codeblock for markdown
 
@@ -84,6 +91,7 @@ def __init__(self):
             self.__can_jedi = False
             self.mode = Mode.OFF
         self.recursion_limit_already_warned = False
+        self.check_recursion_limit(True)
 
     @property
     def mode(self) -> Mode:
@@ -259,35 +267,26 @@ def do_hover(
 
         return hoverstring.strip()
 
-    def check_recursion_limit(self):
+    def check_recursion_limit(self, suppress_warning: bool = False) -> None:
         """
         Tests for python+jedi+numpy versions that are susceptible to a RecursionError/segfault issue, and lowers
-        the recursion limit, warning if the limit is raised externally
+        the recursion limit, warning if the limit is raised externally.
         """
-        if not sys.version.startswith('3.9.') and not sys.version.startswith('3.10.'):
+        if sys.version_info < (3, 9) or sys.version_info >= (3, 11):
             return
 
-        if not self.__can_jedi:
-            return
-
-        # numpy is a required dependency, tested for jedi above
-        if not version('numpy').startswith('2.0') or not version('jedi') == '0.19.1':
-            return
-
-        from . import MAX_RECURSION_LIMIT
-
         if sys.getrecursionlimit() <= MAX_RECURSION_LIMIT:
             return
 
         sys.setrecursionlimit(MAX_RECURSION_LIMIT)
 
-        if not self.recursion_limit_already_warned:
+        # Log a warning if the user (or some user code) seems to have tried to raise the limit again after we lowered it.
+        # This is not a fool-proof way to keep the limit down, and isn't meant to be, only to guard against the primary
+        # way we've seen to cause this issue.
+        if not suppress_warning and not self.recursion_limit_already_warned:
             self.recursion_limit_already_warned = True
-            warnings.warn(f"""Recursion limit has been set to {MAX_RECURSION_LIMIT} to avoid a known segfault in Python related to RecursionErrors.
-This limit will be set to {MAX_RECURSION_LIMIT} whenever autocomplete takes place to avoid this. Other steps you can
-take to avoid this:
- * Use numpy 1.x
- * Use Python 3.8 or 3.11+
- * When available, use a newer version of Jedi
- * Disable autocomplete
+            warnings.warn(f"""Recursion limit has been set to {MAX_RECURSION_LIMIT} to avoid a known segfault in Python 3.9 and 3.10
+related to RecursionErrors. This limit will be set to {MAX_RECURSION_LIMIT} whenever autocomplete takes place
+to avoid this, because the jedi library sets this to 3000, above the safe limit. Disabling autocomplete
+will prevent this check, as it will also prevent jedi from raising the limit.
 See https://github.com/deephaven/deephaven-core/issues/5878 for more information.""")