From d9d19aca8cbc55a88ce76fddf86423b92bfd6762 Mon Sep 17 00:00:00 2001 From: noah-weingarden <33741795+noah-weingarden@users.noreply.github.com> Date: Fri, 28 Oct 2022 03:51:18 -0400 Subject: [PATCH] gh-98624 Add mutex to unittest.mock.NonCallableMock (GH-98688) * Added lock to NonCallableMock in unittest.mock * Add blurb * Nitpick blurb * Edit comment based on @Jason-Y-Z's review * Add link to GH issue (cherry picked from commit 0346eddbe933b5f1f56151bdebf5bd49392bc275) Co-authored-by: noah-weingarden <33741795+noah-weingarden@users.noreply.github.com> --- Lib/unittest/mock.py | 66 +++++++++++-------- ...2-10-25-20-17-34.gh-issue-98624.YQUPFy.rst | 2 + 2 files changed, 40 insertions(+), 28 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-10-25-20-17-34.gh-issue-98624.YQUPFy.rst diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index cd46fea5162aba..b8f4e57f0b4927 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -35,6 +35,7 @@ from types import CodeType, ModuleType, MethodType from unittest.util import safe_repr from functools import wraps, partial +from threading import RLock class InvalidSpecError(Exception): @@ -402,6 +403,14 @@ def __init__(self, /, *args, **kwargs): class NonCallableMock(Base): """A non-callable version of `Mock`""" + # Store a mutex as a class attribute in order to protect concurrent access + # to mock attributes. Using a class attribute allows all NonCallableMock + # instances to share the mutex for simplicity. + # + # See https://github.com/python/cpython/issues/98624 for why this is + # necessary. + _lock = RLock() + def __new__(cls, /, *args, **kw): # every instance has its own class # so we can create magic methods on the @@ -644,35 +653,36 @@ def __getattr__(self, name): f"{name!r} is not a valid assertion. Use a spec " f"for the mock if {name!r} is meant to be an attribute.") - result = self._mock_children.get(name) - if result is _deleted: - raise AttributeError(name) - elif result is None: - wraps = None - if self._mock_wraps is not None: - # XXXX should we get the attribute without triggering code - # execution? - wraps = getattr(self._mock_wraps, name) - - result = self._get_child_mock( - parent=self, name=name, wraps=wraps, _new_name=name, - _new_parent=self - ) - self._mock_children[name] = result - - elif isinstance(result, _SpecState): - try: - result = create_autospec( - result.spec, result.spec_set, result.instance, - result.parent, result.name + with NonCallableMock._lock: + result = self._mock_children.get(name) + if result is _deleted: + raise AttributeError(name) + elif result is None: + wraps = None + if self._mock_wraps is not None: + # XXXX should we get the attribute without triggering code + # execution? + wraps = getattr(self._mock_wraps, name) + + result = self._get_child_mock( + parent=self, name=name, wraps=wraps, _new_name=name, + _new_parent=self ) - except InvalidSpecError: - target_name = self.__dict__['_mock_name'] or self - raise InvalidSpecError( - f'Cannot autospec attr {name!r} from target ' - f'{target_name!r} as it has already been mocked out. ' - f'[target={self!r}, attr={result.spec!r}]') - self._mock_children[name] = result + self._mock_children[name] = result + + elif isinstance(result, _SpecState): + try: + result = create_autospec( + result.spec, result.spec_set, result.instance, + result.parent, result.name + ) + except InvalidSpecError: + target_name = self.__dict__['_mock_name'] or self + raise InvalidSpecError( + f'Cannot autospec attr {name!r} from target ' + f'{target_name!r} as it has already been mocked out. ' + f'[target={self!r}, attr={result.spec!r}]') + self._mock_children[name] = result return result diff --git a/Misc/NEWS.d/next/Library/2022-10-25-20-17-34.gh-issue-98624.YQUPFy.rst b/Misc/NEWS.d/next/Library/2022-10-25-20-17-34.gh-issue-98624.YQUPFy.rst new file mode 100644 index 00000000000000..fb3a2b837fc3f9 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-10-25-20-17-34.gh-issue-98624.YQUPFy.rst @@ -0,0 +1,2 @@ +Add a mutex to unittest.mock.NonCallableMock to protect concurrent access +to mock attributes.