diff --git a/.pylint-spelling-words b/.pylint-spelling-words index 1053114..f258185 100644 --- a/.pylint-spelling-words +++ b/.pylint-spelling-words @@ -28,7 +28,6 @@ favicons fds freebsd getsockname -glibc https ico illumos diff --git a/changelog/16.improvement.rst b/changelog/16.improvement.rst new file mode 100644 index 0000000..33012e3 --- /dev/null +++ b/changelog/16.improvement.rst @@ -0,0 +1,3 @@ +Revert `"Skip test when the GLIBC race conditions are met, instead of failing." `_ + +It wasn't the right fix/workaround. The right fix can be seen in `the Salt repo `_ diff --git a/src/pytestshellutils/downgraded/shell.py b/src/pytestshellutils/downgraded/shell.py index cc03831..d44bce6 100644 --- a/src/pytestshellutils/downgraded/shell.py +++ b/src/pytestshellutils/downgraded/shell.py @@ -14,7 +14,6 @@ import shutil import subprocess import sys -from functools import lru_cache from tempfile import SpooledTemporaryFile from typing import Any from typing import Callable @@ -26,10 +25,8 @@ from typing import Tuple from typing import TYPE_CHECKING from typing import Union -import _pytest._version import attr import psutil -import pytest from pytestskipmarkers.utils import platform from pytestshellutils.customtypes import Callback from pytestshellutils.customtypes import EnvironDict @@ -49,35 +46,9 @@ if TYPE_CHECKING: from typing import Type from pytestsysstats.plugin import StatsProcesses -PYTEST_GE_7 = getattr(_pytest._version, 'version_tuple', (-1, -1)) >= (7, 0) log = logging.getLogger(__name__) -@lru_cache(maxsize=1, typed=False) -def glibc_prone_to_race_condition() -> bool: - """ - Check if the GLIBC version is prone to race conditions. - - This should be fixed after version 2.34 of GLIBC - - See: - https://sourceware.org/bugzilla/show_bug.cgi?id=19329 - """ - ret = subprocess.run( - ['ldd', '--version'], - universal_newlines=True, - check=True, - shell=False, - stdout=subprocess.PIPE, - ) - glibc_version = tuple( - int(x) for x in ret.stdout.splitlines()[0].split()[-1].split('.') if x.isdigit() - ) - if glibc_version >= (2, 34): - return False - return True - - @attr.s(slots=True, kw_only=True) class BaseFactory: """ @@ -87,13 +58,10 @@ class BaseFactory: The path to the desired working directory :keyword dict environ: A dictionary of ``key``, ``value`` pairs to add to the environment. - :keyword bool skip_on_glibc_race_condition_hit: - Whether to skip test or not when the glibc race conditions are seen. """ cwd = attr.ib(converter=resolved_pathlib_path) environ = attr.ib(repr=False) - skip_on_glibc_race_condition_hit = attr.ib(default=True) @cwd.default def _default_cwd(self) -> pathlib.Path: @@ -281,22 +249,6 @@ def _terminate(self) -> ProcessResult: cmdline=cast(List[str], self._terminal.args), ) log.info('%s %s', self.factory.__class__.__name__, self._terminal_result) - if ( - self._terminal_result.returncode == 127 - and glibc_prone_to_race_condition() - ): - if ( - stderr - and 'Inconsistency detected by ld.so' in stderr - and '_dl_allocate_tls_init' in stderr - ) and self.factory.skip_on_glibc_race_condition_hit: - exc_kwargs = {} - if PYTEST_GE_7: - exc_kwargs['_use_item_location'] = True - raise pytest.skip.Exception( - 'GLIBC race condition bug hit. See https://sourceware.org/bugzilla/show_bug.cgi?id=19329', - **exc_kwargs, - ) return self._terminal_result finally: self._terminal = None diff --git a/src/pytestshellutils/shell.py b/src/pytestshellutils/shell.py index 29d470a..e93c9e1 100644 --- a/src/pytestshellutils/shell.py +++ b/src/pytestshellutils/shell.py @@ -13,7 +13,6 @@ import shutil import subprocess import sys -from functools import lru_cache from tempfile import SpooledTemporaryFile from typing import Any from typing import Callable @@ -26,10 +25,8 @@ from typing import TYPE_CHECKING from typing import Union -import _pytest._version import attr import psutil -import pytest from pytestskipmarkers.utils import platform from pytestshellutils.customtypes import Callback @@ -51,39 +48,9 @@ from typing import Type from pytestsysstats.plugin import StatsProcesses -PYTEST_GE_7 = getattr(_pytest._version, "version_tuple", (-1, -1)) >= (7, 0) - log = logging.getLogger(__name__) -@lru_cache(maxsize=1, typed=False) -def glibc_prone_to_race_condition() -> bool: - """ - Check if the GLIBC version is prone to race conditions. - - This should be fixed after version 2.34 of GLIBC - - See: - https://sourceware.org/bugzilla/show_bug.cgi?id=19329 - """ - ret = subprocess.run( - ["ldd", "--version"], - universal_newlines=True, - check=True, - shell=False, - stdout=subprocess.PIPE, - ) - glibc_version: Tuple[int, ...] = tuple( - int(x) for x in ret.stdout.splitlines()[0].split()[-1].split(".") if x.isdigit() - ) - if glibc_version >= (2, 34): - # After glibc 3.34, the race condition bug should have been fixed - # See: - # https://sourceware.org/bugzilla/show_bug.cgi?id=19329 - return False - return True - - @attr.s(slots=True, kw_only=True) class BaseFactory: """ @@ -93,13 +60,10 @@ class BaseFactory: The path to the desired working directory :keyword dict environ: A dictionary of ``key``, ``value`` pairs to add to the environment. - :keyword bool skip_on_glibc_race_condition_hit: - Whether to skip test or not when the glibc race conditions are seen. """ cwd: pathlib.Path = attr.ib(converter=resolved_pathlib_path) environ: EnvironDict = attr.ib(repr=False) - skip_on_glibc_race_condition_hit: bool = attr.ib(default=True) @cwd.default def _default_cwd(self) -> pathlib.Path: @@ -317,20 +281,6 @@ def _terminate(self) -> ProcessResult: cmdline=cast(List[str], self._terminal.args), ) log.info("%s %s", self.factory.__class__.__name__, self._terminal_result) - if self._terminal_result.returncode == 127 and glibc_prone_to_race_condition(): - if ( - stderr - and "Inconsistency detected by ld.so" in stderr - and "_dl_allocate_tls_init" in stderr - ) and self.factory.skip_on_glibc_race_condition_hit: - exc_kwargs = {} - if PYTEST_GE_7: - exc_kwargs["_use_item_location"] = True - raise pytest.skip.Exception( - "GLIBC race condition bug hit. " - "See https://sourceware.org/bugzilla/show_bug.cgi?id=19329", - **exc_kwargs, - ) return self._terminal_result finally: self._terminal = None diff --git a/tests/functional/shell/test_subprocess.py b/tests/functional/shell/test_subprocess.py index 76fb625..048cf20 100644 --- a/tests/functional/shell/test_subprocess.py +++ b/tests/functional/shell/test_subprocess.py @@ -4,7 +4,6 @@ import os import sys from typing import cast -from unittest import mock import pytest @@ -208,27 +207,3 @@ def test_display_name(tempfiles): result = shell.run(sys.executable, script) assert result.returncode == 0 assert shell.get_display_name() == "Subprocess([{!r}, {!r}])".format(sys.executable, script) - - -def test_glibc_race_condition_handling(tempfiles): - shell = Subprocess() - stderr = "Inconsistency detected by ld.so ... _dl_allocate_tls_init\n" - script = tempfiles.makepyfile( - """ - # coding=utf-8 - import sys - import time - print("{}", file=sys.stderr, flush=True) - time.sleep(0.1) - exit(127) - """.format( - stderr.strip() - ) - ) - with mock.patch("pytestshellutils.shell.glibc_prone_to_race_condition", return_value=False): - ret = shell.run(sys.executable, script) - assert ret.returncode == 127 - assert ret.stderr == stderr - with mock.patch("pytestshellutils.shell.glibc_prone_to_race_condition", return_value=True): - with pytest.raises(pytest.skip.Exception): - shell.run(sys.executable, script)