From 4581ce5e2e0f988de50c82c46d6cb0b0d53d81ee Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 23 Aug 2024 14:26:47 -0500 Subject: [PATCH] fix: Provide a warning when running autocomplete in a way that could segfault (#5980) Simple mitigation for jedi setting the recursion limit too high for Python 3.9 and 3.10 to correctly handle RecursionErrors. This prevents a segfault for a known bug in jedi, and only applies in cases where we know the bug can take place. Technically it is possible for these python versions to crash in this way without autocomplete or numpy, but it first would require the recursion limit to be raised, so the fix only applies when autocomplete is used. The `deephaven_internal.autocompleter` module has a `set_max_recursion_limit` method to define a different max recursion limit for an application, in case the default of 2000 is not sufficient for all cases. Fixes #5878 Backport #5954 --- .../auto_completer/__init__.py | 9 +++++ .../auto_completer/_completer.py | 37 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/py/server/deephaven_internal/auto_completer/__init__.py b/py/server/deephaven_internal/auto_completer/__init__.py index ffb6eea0888..8ba39520d27 100644 --- a/py/server/deephaven_internal/auto_completer/__init__.py +++ b/py/server/deephaven_internal/auto_completer/__init__.py @@ -19,7 +19,16 @@ from ._completer import Completer, Mode from jedi import preload_module, Interpreter + 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 diff --git a/py/server/deephaven_internal/auto_completer/_completer.py b/py/server/deephaven_internal/auto_completer/_completer.py index f87c3c0b56c..39ea08cc25a 100644 --- a/py/server/deephaven_internal/auto_completer/_completer.py +++ b/py/server/deephaven_internal/auto_completer/_completer.py @@ -6,6 +6,9 @@ from typing import Any, Union, List from jedi import Interpreter, Script from jedi.api.classes import Completion, Signature +from importlib.metadata import version +import sys +import warnings class Mode(Enum): @@ -36,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 @@ -80,6 +90,8 @@ def __init__(self): except ImportError: self.__can_jedi = False self.mode = Mode.OFF + self.recursion_limit_already_warned = False + self.check_recursion_limit(True) @property def mode(self) -> Mode: @@ -135,6 +147,7 @@ def do_completion( Modeled after Jedi language server https://github.com/pappasam/jedi-language-server/blob/main/jedi_language_server/server.py#L189 """ + self.check_recursion_limit() if not self._versions[uri] == version: # if you aren't the newest completion, you get nothing, quickly return [] @@ -253,3 +266,27 @@ def do_hover( hoverstring += '\n---\n' + wrap_plaintext(raw_docstring) return hoverstring.strip() + + 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. + """ + if sys.version_info < (3, 9) or sys.version_info >= (3, 11): + return + + if sys.getrecursionlimit() <= MAX_RECURSION_LIMIT: + return + + sys.setrecursionlimit(MAX_RECURSION_LIMIT) + + # 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 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.""")