timeout: Rename to unsafe_timeout.

This timeout strategy using asynchronous exceptions has a number of
safety caveats (read the docstring!!) and should only be used in very
specific circumstances.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit is contained in:
Anders Kaseorg 2024-04-18 10:44:46 -07:00 committed by Tim Abbott
parent 631c2f7d4c
commit 72018cc26b
5 changed files with 10 additions and 10 deletions

View File

@ -68,7 +68,7 @@ from zerver.lib.outgoing_http import OutgoingSession
from zerver.lib.subdomains import is_static_or_current_realm_url from zerver.lib.subdomains import is_static_or_current_realm_url
from zerver.lib.tex import render_tex from zerver.lib.tex import render_tex
from zerver.lib.thumbnail import user_uploads_or_external from zerver.lib.thumbnail import user_uploads_or_external
from zerver.lib.timeout import timeout from zerver.lib.timeout import unsafe_timeout
from zerver.lib.timezone import common_timezones from zerver.lib.timezone import common_timezones
from zerver.lib.types import LinkifierDict from zerver.lib.types import LinkifierDict
from zerver.lib.url_encoding import encode_stream, hash_util_encode from zerver.lib.url_encoding import encode_stream, hash_util_encode
@ -2667,7 +2667,7 @@ def do_convert(
# extremely inefficient in corner cases) as well as user # extremely inefficient in corner cases) as well as user
# errors (e.g. a linkifier that makes some syntax # errors (e.g. a linkifier that makes some syntax
# infinite-loop). # infinite-loop).
rendering_result.rendered_content = timeout(5, lambda: _md_engine.convert(content)) rendering_result.rendered_content = unsafe_timeout(5, lambda: _md_engine.convert(content))
# Throw an exception if the content is huge; this protects the # Throw an exception if the content is huge; this protects the
# rest of the codebase from any bugs where we end up rendering # rest of the codebase from any bugs where we end up rendering

View File

@ -1560,7 +1560,7 @@ Output:
markdown.__init__.do_convert. markdown.__init__.do_convert.
""" """
with mock.patch( with mock.patch(
"zerver.lib.markdown.timeout", side_effect=subprocess.CalledProcessError(1, []) "zerver.lib.markdown.unsafe_timeout", side_effect=subprocess.CalledProcessError(1, [])
), self.assertLogs(level="ERROR"): # For markdown_logger.exception ), self.assertLogs(level="ERROR"): # For markdown_logger.exception
yield yield

View File

@ -22,7 +22,7 @@ class TimeoutExpiredError(Exception):
ResultT = TypeVar("ResultT") ResultT = TypeVar("ResultT")
def timeout(timeout: float, func: Callable[[], ResultT]) -> ResultT: def unsafe_timeout(timeout: float, func: Callable[[], ResultT]) -> ResultT:
"""Call the function in a separate thread. """Call the function in a separate thread.
Return its return value, or raise an exception, Return its return value, or raise an exception,
within approximately 'timeout' seconds. within approximately 'timeout' seconds.

View File

@ -3305,7 +3305,7 @@ class MarkdownErrorTests(ZulipTestCase):
throws an exception""" throws an exception"""
msg = "mock rendered message\n" * 10 * settings.MAX_MESSAGE_LENGTH msg = "mock rendered message\n" * 10 * settings.MAX_MESSAGE_LENGTH
with mock.patch("zerver.lib.markdown.timeout", return_value=msg), mock.patch( with mock.patch("zerver.lib.markdown.unsafe_timeout", return_value=msg), mock.patch(
"zerver.lib.markdown.markdown_logger" "zerver.lib.markdown.markdown_logger"
): ):
with self.assertRaises(MarkdownRenderingError): with self.assertRaises(MarkdownRenderingError):

View File

@ -4,7 +4,7 @@ import traceback
from unittest import skipIf from unittest import skipIf
from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.timeout import TimeoutExpiredError, timeout from zerver.lib.timeout import TimeoutExpiredError, unsafe_timeout
class TimeoutTestCase(ZulipTestCase): class TimeoutTestCase(ZulipTestCase):
@ -20,12 +20,12 @@ class TimeoutTestCase(ZulipTestCase):
return 42 # nocoverage return 42 # nocoverage
def test_timeout_returns(self) -> None: def test_timeout_returns(self) -> None:
ret = timeout(1, lambda: 42) ret = unsafe_timeout(1, lambda: 42)
self.assertEqual(ret, 42) self.assertEqual(ret, 42)
def test_timeout_exceeded(self) -> None: def test_timeout_exceeded(self) -> None:
try: try:
timeout(1, lambda: self.sleep_x_seconds_y_times(0.1, 50)) unsafe_timeout(1, lambda: self.sleep_x_seconds_y_times(0.1, 50))
raise AssertionError("Failed to raise a timeout") raise AssertionError("Failed to raise a timeout")
except TimeoutExpiredError as exc: except TimeoutExpiredError as exc:
tb = traceback.format_tb(exc.__traceback__) tb = traceback.format_tb(exc.__traceback__)
@ -34,7 +34,7 @@ class TimeoutTestCase(ZulipTestCase):
def test_timeout_raises(self) -> None: def test_timeout_raises(self) -> None:
try: try:
timeout(1, self.something_exceptional) unsafe_timeout(1, self.something_exceptional)
raise AssertionError("Failed to raise an exception") raise AssertionError("Failed to raise an exception")
except ValueError as exc: except ValueError as exc:
tb = traceback.format_tb(exc.__traceback__) tb = traceback.format_tb(exc.__traceback__)
@ -47,7 +47,7 @@ class TimeoutTestCase(ZulipTestCase):
# kill it # kill it
with self.assertLogs(level="WARNING") as m: with self.assertLogs(level="WARNING") as m:
try: try:
timeout(1, lambda: self.sleep_x_seconds_y_times(5, 1)) unsafe_timeout(1, lambda: self.sleep_x_seconds_y_times(5, 1))
raise AssertionError("Failed to raise a timeout") raise AssertionError("Failed to raise a timeout")
except TimeoutExpiredError as exc: except TimeoutExpiredError as exc:
tb = traceback.format_tb(exc.__traceback__) tb = traceback.format_tb(exc.__traceback__)