Skip to content

Commit

Permalink
Revert "Skip test when the GLIBC race conditions are met, instead of …
Browse files Browse the repository at this point in the history
…failing."

This reverts commit f79aba3, it wasn't
the right fix. The right fix can be seen in saltstack/salt#62078
  • Loading branch information
s0undt3ch committed May 19, 2022
1 parent 96b291f commit 539ebcd
Show file tree
Hide file tree
Showing 5 changed files with 3 additions and 124 deletions.
1 change: 0 additions & 1 deletion .pylint-spelling-words
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ favicons
fds
freebsd
getsockname
glibc
https
ico
illumos
Expand Down
3 changes: 3 additions & 0 deletions changelog/16.improvement.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Revert `"Skip test when the GLIBC race conditions are met, instead of failing." <https://github.com/saltstack/pytest-shell-utilities/commit/f79aba3c5c0c7e4bdd895ae422d2f35ed22ea2e6>`_

It wasn't the right fix/workaround. The right fix can be seen in `the Salt repo <https://github.com/saltstack/salt/pull/62078>`_
48 changes: 0 additions & 48 deletions src/pytestshellutils/downgraded/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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:
"""
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down
50 changes: 0 additions & 50 deletions src/pytestshellutils/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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:
"""
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down
25 changes: 0 additions & 25 deletions tests/functional/shell/test_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import os
import sys
from typing import cast
from unittest import mock

import pytest

Expand Down Expand Up @@ -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)

0 comments on commit 539ebcd

Please sign in to comment.