From c5173c954e8c045702d697a8aeeda6169ca4a282 Mon Sep 17 00:00:00 2001 From: Eugene Toder Date: Wed, 24 Aug 2022 17:59:17 -0400 Subject: [PATCH 01/38] Fix threading issues with register_post_import_hook 1. Add a missing lock to ImportHookFinder.find_spec(). It should have the same lock as find_module() since it accesses _post_import_hooks. 2. Fix a deadlock between notify_module_loaded() and ImportHookFinder. This occurs because the former is calling import hooks under _post_import_hooks_lock. If a hook is making any imports itself while another thread is also making imports, the thread can grab the import lock and wait for _post_import_hooks_lock in ImportHookFinder, while the hook already has _post_import_hooks_lock and is waiting for the import lock. This results in a deadlock. The solution is to call the hooks outside of _post_import_hooks_lock. Same for register_post_import_hook() and ImportHookFinder. 3. Also fix an issue if register_post_import_hook() was called after a module was already imported but was later removed from sys.modules. This also makes the logic simpler. --- src/wrapt/importer.py | 77 +++++++++++---------------------- tests/module1.py | 2 + tests/module2.py | 2 + tests/test_post_import_hooks.py | 57 ++++++++++++++++++++++-- 4 files changed, 82 insertions(+), 56 deletions(-) create mode 100644 tests/module1.py create mode 100644 tests/module2.py diff --git a/src/wrapt/importer.py b/src/wrapt/importer.py index 5c4d4cc6..9d42f05c 100644 --- a/src/wrapt/importer.py +++ b/src/wrapt/importer.py @@ -15,8 +15,6 @@ string_types = str, from importlib.util import find_spec -from .decorators import synchronized - # The dictionary registering any post import hooks to be triggered once # the target module has been imported. Once a module has been imported # and the hooks fired, the list of hooks recorded against the target @@ -45,7 +43,6 @@ def import_hook(module): return callback(module) return import_hook -@synchronized(_post_import_hooks_lock) def register_post_import_hook(hook, name): # Create a deferred import hook if hook is a string name rather than # a callable function. @@ -53,51 +50,28 @@ def register_post_import_hook(hook, name): if isinstance(hook, string_types): hook = _create_import_hook_from_string(hook) - # Automatically install the import hook finder if it has not already - # been installed. - - global _post_import_hooks_init - - if not _post_import_hooks_init: - _post_import_hooks_init = True - sys.meta_path.insert(0, ImportHookFinder()) + with _post_import_hooks_lock: + # Automatically install the import hook finder if it has not already + # been installed. - # Determine if any prior registration of a post import hook for - # the target modules has occurred and act appropriately. + global _post_import_hooks_init - hooks = _post_import_hooks.get(name, None) + if not _post_import_hooks_init: + _post_import_hooks_init = True + sys.meta_path.insert(0, ImportHookFinder()) - if hooks is None: - # No prior registration of post import hooks for the target - # module. We need to check whether the module has already been - # imported. If it has we fire the hook immediately and add an - # empty list to the registry to indicate that the module has - # already been imported and hooks have fired. Otherwise add - # the post import hook to the registry. + # Check if the module is already imported. If not, register the hook + # to be called after import. module = sys.modules.get(name, None) + if module is None: + _post_import_hooks.setdefault(name, []).append(hook) - if module is not None: - _post_import_hooks[name] = [] - hook(module) - - else: - _post_import_hooks[name] = [hook] - - elif hooks == []: - # A prior registration of port import hooks for the target - # module was done and the hooks already fired. Fire the hook - # immediately. - - module = sys.modules[name] + # If the module is already imported, fire the hook right away. + # NOTE: Call the hook outside of the lock to avoid deadlocks. + if module is not None: hook(module) - else: - # A prior registration of port import hooks for the target - # module was done but the module has not yet been imported. - - _post_import_hooks[name].append(hook) - # Register post import hooks defined as package entry points. def _create_import_hook_from_entrypoint(entrypoint): @@ -124,16 +98,14 @@ def discover_post_import_hooks(group): # exception is raised in any of the post import hooks, that will cause # the import of the target module to fail. -@synchronized(_post_import_hooks_lock) def notify_module_loaded(module): name = getattr(module, '__name__', None) - hooks = _post_import_hooks.get(name, None) - - if hooks: - _post_import_hooks[name] = [] + with _post_import_hooks_lock: + hooks = _post_import_hooks.pop(name, ()) - for hook in hooks: - hook(module) + # NOTE: Call hooks outside of the lock to avoid deadlocks. + for hook in hooks: + hook(module) # A custom module import finder. This intercepts attempts to import # modules and watches out for attempts to import target modules of @@ -181,14 +153,14 @@ class ImportHookFinder: def __init__(self): self.in_progress = {} - @synchronized(_post_import_hooks_lock) def find_module(self, fullname, path=None): # If the module being imported is not one we have registered # post import hooks for, we can return immediately. We will # take no further part in the importing of this module. - if not fullname in _post_import_hooks: - return None + with _post_import_hooks_lock: + if fullname not in _post_import_hooks: + return None # When we are interested in a specific module, we will call back # into the import system a second time to defer to the import @@ -244,8 +216,9 @@ def find_spec(self, fullname, path=None, target=None): # post import hooks for, we can return immediately. We will # take no further part in the importing of this module. - if not fullname in _post_import_hooks: - return None + with _post_import_hooks_lock: + if fullname not in _post_import_hooks: + return None # When we are interested in a specific module, we will call back # into the import system a second time to defer to the import diff --git a/tests/module1.py b/tests/module1.py new file mode 100644 index 00000000..7dfba642 --- /dev/null +++ b/tests/module1.py @@ -0,0 +1,2 @@ +import time +time.sleep(0.1) # simulate slow code diff --git a/tests/module2.py b/tests/module2.py new file mode 100644 index 00000000..7dfba642 --- /dev/null +++ b/tests/module2.py @@ -0,0 +1,2 @@ +import time +time.sleep(0.1) # simulate slow code diff --git a/tests/test_post_import_hooks.py b/tests/test_post_import_hooks.py index b7d65882..c00a6190 100644 --- a/tests/test_post_import_hooks.py +++ b/tests/test_post_import_hooks.py @@ -2,6 +2,7 @@ import unittest import sys +import threading import wrapt from wrapt.importer import _post_import_hooks @@ -14,10 +15,8 @@ def setUp(self): # So we can import 'this' and test post-import hooks multiple times # below in the context of a single Python process, remove 'this' from # sys.modules and post import hooks. - if 'this' in sys.modules: - del sys.modules['this'] - if 'this' in _post_import_hooks: - del _post_import_hooks['this'] + sys.modules.pop('this', None) + _post_import_hooks.pop('this', None) def test_before_import(self): invoked = [] @@ -72,5 +71,55 @@ def hook_this_two(module): self.assertEqual(len(invoked_one), 1) self.assertEqual(len(invoked_two), 1) + def test_remove_from_sys_modules(self): + invoked = [] + + @wrapt.when_imported('this') + def hook_this(module): + self.assertEqual(module.__name__, 'this') + invoked.append(1) + + import this + self.assertEqual(len(invoked), 1) + + del sys.modules['this'] + wrapt.register_post_import_hook(hook_this, 'this') + import this + self.assertEqual(len(invoked), 2) + + def test_import_deadlock(self): + @wrapt.when_imported('this') + def hook_this(module): + ev.set() + # The hook used to be called under _post_import_hooks_lock. Then + # the import tried to acquire the import lock. If the other thread + # already held it and was waiting for _post_import_hooks_lock, we + # deadlocked. + for _ in range(5): + import module1 + del sys.modules['module1'] + + def worker(): + ev.wait() + # The import tries to acquire the import lock. ImportHookFinder + # then tries to acquire _post_import_hooks_lock under it. + for _ in range(5): + import module2 + del sys.modules['module2'] + + # A deadlock between notify_module_loaded and ImportHookFinder. + ev = threading.Event() + thread = threading.Thread(target=worker) + thread.start() + import this + thread.join() + + # A deadlock between register_post_import_hook and ImportHookFinder. + ev = threading.Event() + thread = threading.Thread(target=worker) + thread.start() + wrapt.register_post_import_hook(hook_this, 'this') + thread.join() + if __name__ == '__main__': unittest.main() From cd4498354e647c0f9e158e92fd94555c20a381e3 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Fri, 28 Oct 2022 13:41:09 +1100 Subject: [PATCH 02/38] Update wheel builder for GitHub actions. --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 663d7b1a..1149e7af 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -244,7 +244,7 @@ jobs: if: ${{ matrix.arch == 'aarch64' }} uses: docker/setup-qemu-action@v1 - name: Build wheels - uses: pypa/cibuildwheel@v2.4.0 + uses: pypa/cibuildwheel@v2.11.2 with: output-dir: dist env: From a5cbece5e068b582a9421f60e83a78f1083e936d Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Fri, 28 Oct 2022 14:59:00 +1100 Subject: [PATCH 03/38] Disable aarch64 builds as OS platform broken. --- .github/workflows/main.yml | 66 +++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1149e7af..10748f7c 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -42,39 +42,39 @@ jobs: name: coverage path: .coverage.* - test_aarch64_linux: - name: Test (${{ matrix.python.os }}, ${{ matrix.python.python-version }}, aarch64) - runs-on: ${{ matrix.python.os }} - strategy: - matrix: - python: - #- {os: ubuntu-latest, python-version: 3.7, pyver: py37} - #- {os: ubuntu-latest, python-version: 3.8, pyver: py38} - #- {os: ubuntu-latest, python-version: 3.9, pyver: py39} - - {os: ubuntu-latest, python-version: "3.10", pyver: py310} - env: - py: python${{ matrix.python.python-version }} - steps: - - name: Checkout - uses: actions/checkout@v2 - - name: Set up QEMU - id: qemu - uses: docker/setup-qemu-action@v1 - - name: Test with tox - run: | - docker run --rm -v ${{ github.workspace }}:/io:rw --workdir=/io \ - arm64v8/ubuntu \ - bash -exc 'apt-get update && \ - apt install software-properties-common -y && \ - add-apt-repository ppa:deadsnakes/ppa -y && \ - apt install -y ${{ env.py }} && \ - apt install -y ${{ env.py }}-venv && \ - ${{ env.py }} -m venv .env && \ - source .env/bin/activate && \ - pip install -U pip wheel setuptools && \ - pip install tox tox-gh-actions && \ - tox -e ${{ matrix.python.pyver }} && \ - deactivate' + # test_aarch64_linux: + # name: Test (${{ matrix.python.os }}, ${{ matrix.python.python-version }}, aarch64) + # runs-on: ${{ matrix.python.os }} + # strategy: + # matrix: + # python: + # #- {os: ubuntu-latest, python-version: 3.7, pyver: py37} + # #- {os: ubuntu-latest, python-version: 3.8, pyver: py38} + # #- {os: ubuntu-latest, python-version: 3.9, pyver: py39} + # - {os: ubuntu-latest, python-version: "3.10", pyver: py310} + # env: + # py: python${{ matrix.python.python-version }} + # steps: + # - name: Checkout + # uses: actions/checkout@v2 + # - name: Set up QEMU + # id: qemu + # uses: docker/setup-qemu-action@v1 + # - name: Test with tox + # run: | + # docker run --rm -v ${{ github.workspace }}:/io:rw --workdir=/io \ + # arm64v8/ubuntu \ + # bash -exc 'apt-get update && \ + # apt install software-properties-common -y && \ + # add-apt-repository ppa:deadsnakes/ppa -y && \ + # apt install -y ${{ env.py }} && \ + # apt install -y ${{ env.py }}-venv && \ + # ${{ env.py }} -m venv .env && \ + # source .env/bin/activate && \ + # pip install -U pip wheel setuptools && \ + # pip install tox tox-gh-actions && \ + # tox -e ${{ matrix.python.pyver }} && \ + # deactivate' test_macos: name: Test (${{ matrix.os }}, ${{ matrix.python-version }}) From a46df95d1dffea4be5222f03faf0c95e549a3b91 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Fri, 28 Oct 2022 15:00:22 +1100 Subject: [PATCH 04/38] Disable build dependency. --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 10748f7c..8ac59976 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -224,7 +224,7 @@ jobs: name: Build wheels (3.6+) on ${{ matrix.os }} for ${{ matrix.arch }} needs: - test_linux - - test_aarch64_linux + #- test_aarch64_linux - test_macos - test_windows_py27 - test_windows From 2d9fd857f1e4c29f904c485312f4a73d6ae5e485 Mon Sep 17 00:00:00 2001 From: Eugene Toder Date: Tue, 27 Dec 2022 16:52:01 -0500 Subject: [PATCH 05/38] Restore original loader after import Closes #221 --- src/wrapt/importer.py | 16 ++++++++++++++++ tests/test_post_import_hooks.py | 15 +++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/wrapt/importer.py b/src/wrapt/importer.py index 5c4d4cc6..c3da6409 100644 --- a/src/wrapt/importer.py +++ b/src/wrapt/importer.py @@ -160,8 +160,23 @@ def __init__(self, loader): if hasattr(loader, "exec_module"): self.exec_module = self._exec_module + def _set_loader(self, module): + # Set module's loader to self.loader unless it's already set to + # something else. Import machinery will set it to spec.loader if it is + # None, so handle None as well. The module may not support attribute + # assignment, in which case we simply skip it. + if getattr(module, "__loader__", None) in (None, self): + try: + module.__loader__ = self.loader + except AttributeError: + pass + if (getattr(module, "__spec__", None) is not None + and getattr(module.__spec__, "loader", None) is self): + module.__spec__.loader = self.loader + def _load_module(self, fullname): module = self.loader.load_module(fullname) + self._set_loader(module) notify_module_loaded(module) return module @@ -173,6 +188,7 @@ def _create_module(self, spec): return self.loader.create_module(spec) def _exec_module(self, module): + self._set_loader(module) self.loader.exec_module(module) notify_module_loaded(module) diff --git a/tests/test_post_import_hooks.py b/tests/test_post_import_hooks.py index b7d65882..391c5fd5 100644 --- a/tests/test_post_import_hooks.py +++ b/tests/test_post_import_hooks.py @@ -72,5 +72,20 @@ def hook_this_two(module): self.assertEqual(len(invoked_one), 1) self.assertEqual(len(invoked_two), 1) + def test_loader(self): + try: + from importlib.machinery import SourceFileLoader + except ImportError: + return + + @wrapt.when_imported('this') + def hook_this(module): + pass + + import this + + self.assertIsInstance(this.__loader__, SourceFileLoader) + self.assertIsInstance(this.__spec__.loader, SourceFileLoader) + if __name__ == '__main__': unittest.main() From c81275517faedfc885988c7cc5c96737c170eee7 Mon Sep 17 00:00:00 2001 From: Stevie Date: Fri, 6 Jan 2023 14:06:20 +0100 Subject: [PATCH 06/38] chore(ci): upgrade actions --- .github/workflows/main.yml | 44 +++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 8ac59976..fb391526 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -25,9 +25,9 @@ jobs: - pypy-3.7 steps: - name: Checkout code - uses: actions/checkout@v1 + uses: actions/checkout@v3 - name: Setup Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} - name: Update pip @@ -37,7 +37,7 @@ jobs: - name: Test with tox run: python -m tox - name: Store partial coverage reports - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: coverage path: .coverage.* @@ -97,9 +97,9 @@ jobs: - pypy-3.7 steps: - name: Checkout code - uses: actions/checkout@v1 + uses: actions/checkout@v3 - name: Setup Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} - name: Update pip @@ -109,7 +109,7 @@ jobs: - name: Test with tox run: python -m tox - name: Store partial coverage reports - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: coverage path: .coverage.* @@ -125,9 +125,9 @@ jobs: - 2.7 steps: - name: Checkout code - uses: actions/checkout@v1 + uses: actions/checkout@v3 - name: Setup Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} - name: Update pip @@ -157,9 +157,9 @@ jobs: - pypy-3.7 steps: - name: Checkout code - uses: actions/checkout@v1 + uses: actions/checkout@v3 - name: Setup Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} - name: Update pip @@ -178,17 +178,17 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: submodules: true - name: Set up Python 3.9 - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: python-version: 3.9 - name: Build source distribution run: python setup.py sdist - name: Store built wheels - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: dist path: dist/* @@ -205,7 +205,7 @@ jobs: matrix: os: [ubuntu-latest, windows-latest, macos-latest] steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Build wheels uses: pypa/cibuildwheel@v1.11.1.post1 with: @@ -215,7 +215,7 @@ jobs: CIBW_BUILD: cp27* cp35* CIBW_SKIP: cp27-win* CIBW_BUILD_VERBOSITY: 1 - - uses: actions/upload-artifact@v2 + - uses: actions/upload-artifact@v3 with: name: dist path: dist/*.whl @@ -239,12 +239,12 @@ jobs: - os: macos-latest arch: arm64 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Set up QEMU if: ${{ matrix.arch == 'aarch64' }} - uses: docker/setup-qemu-action@v1 + uses: docker/setup-qemu-action@v2 - name: Build wheels - uses: pypa/cibuildwheel@v2.11.2 + uses: pypa/cibuildwheel@v2.11.4 with: output-dir: dist env: @@ -252,7 +252,7 @@ jobs: CIBW_SKIP: pp* CIBW_BUILD_VERBOSITY: 1 CIBW_ARCHS: ${{ matrix.arch }} - - uses: actions/upload-artifact@v2 + - uses: actions/upload-artifact@v3 with: name: dist path: dist/*.whl @@ -268,15 +268,15 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v1 + uses: actions/checkout@v3 - name: Setup Python 3.9 - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: python-version: 3.9 - name: Install coverage package run: python -m pip install -U coverage - name: Download partial coverage reports - uses: actions/download-artifact@v2 + uses: actions/download-artifact@v3 with: name: coverage - name: Combine coverage From 8c9265b70211cd1f68acc415ed0e98656302a58d Mon Sep 17 00:00:00 2001 From: Stevie Date: Fri, 6 Jan 2023 15:08:55 +0100 Subject: [PATCH 07/38] chore(tox): run tests on Python 3.11 final release --- .github/workflows/main.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index fb391526..d0515683 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -18,8 +18,8 @@ jobs: - 3.7 - 3.8 - 3.9 - - "3.10" - - 3.11-dev + - 3.10 + - 3.11 - pypy-2.7 - pypy-3.6 - pypy-3.7 @@ -90,8 +90,8 @@ jobs: - 3.7 - 3.8 - 3.9 - - "3.10" - - 3.11-dev + - 3.10 + - 3.11 - pypy-2.7 #- pypy-3.6 - pypy-3.7 @@ -150,8 +150,8 @@ jobs: - 3.7 - 3.8 - 3.9 - - "3.10" - - 3.11-dev + - 3.10 + - 3.11 - pypy-2.7 - pypy-3.6 - pypy-3.7 From 59e389fac767dacf46e28b32ae4d5d200cf5b319 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 09:34:33 +1100 Subject: [PATCH 08/38] Fix up formatting in change notes. --- docs/changes.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 203a8f93..b9e4aab5 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -10,8 +10,8 @@ Version 1.14.1 its own custom module importer was used, importing modules could fail if the custom module importer didn't use the latest Python import hook finder/loader APIs and instead used the deprecated API. This was actually occurring with the - `zipimporter` in Python itself, which was not updated to use the newer Python - APIs until Python 3.10. + ``zipimporter`` in Python itself, which was not updated to use the newer + Python APIs until Python 3.10. Version 1.14.0 -------------- From cb29969fa524381f6f48de1c937f155316609676 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 09:34:51 +1100 Subject: [PATCH 09/38] Add makefile target to remove coverage files. --- Makefile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index acf66890..ccf3e293 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,10 @@ package : release : clean package twine upload dist/* -clean : +mostlyclean: + rm -rf .coverage.* + +clean: mostlyclean rm -rf build dist wrapt.egg-info test : From 920bb175d4db1f05346b7c2015ceb0fee9d9153e Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 09:43:01 +1100 Subject: [PATCH 10/38] Version 3.10 must be quoted else is interpreted as 3.1. --- .github/workflows/main.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index d0515683..e4a6509a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -18,7 +18,7 @@ jobs: - 3.7 - 3.8 - 3.9 - - 3.10 + - "3.10" - 3.11 - pypy-2.7 - pypy-3.6 @@ -90,7 +90,7 @@ jobs: - 3.7 - 3.8 - 3.9 - - 3.10 + - "3.10" - 3.11 - pypy-2.7 #- pypy-3.6 @@ -150,7 +150,7 @@ jobs: - 3.7 - 3.8 - 3.9 - - 3.10 + - "3.10" - 3.11 - pypy-2.7 - pypy-3.6 From 8c160511610dae1f9eb141e88e333347fee3674d Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 09:53:04 +1100 Subject: [PATCH 11/38] Pin runner to old ubuntu version as latest fails. --- .github/workflows/main.yml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index e4a6509a..f93e3fb5 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -10,7 +10,7 @@ jobs: strategy: matrix: os: - - ubuntu-latest + - ubuntu-20.04 python-version: - 2.7 - 3.5 @@ -48,10 +48,10 @@ jobs: # strategy: # matrix: # python: - # #- {os: ubuntu-latest, python-version: 3.7, pyver: py37} - # #- {os: ubuntu-latest, python-version: 3.8, pyver: py38} - # #- {os: ubuntu-latest, python-version: 3.9, pyver: py39} - # - {os: ubuntu-latest, python-version: "3.10", pyver: py310} + # #- {os: ubuntu-20.04, python-version: 3.7, pyver: py37} + # #- {os: ubuntu-20.04, python-version: 3.8, pyver: py38} + # #- {os: ubuntu-20.04, python-version: 3.9, pyver: py39} + # - {os: ubuntu-20.04, python-version: "3.10", pyver: py310} # env: # py: python${{ matrix.python.python-version }} # steps: @@ -175,7 +175,7 @@ jobs: - test_linux - test_macos - test_windows - runs-on: ubuntu-latest + runs-on: ubuntu-20.04 steps: - name: Checkout code uses: actions/checkout@v3 @@ -203,7 +203,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-latest, windows-latest, macos-latest] + os: [ubuntu-20.04, windows-latest, macos-latest] steps: - uses: actions/checkout@v3 - name: Build wheels @@ -231,10 +231,10 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-latest, windows-latest, macos-latest] + os: [ubuntu-20.04, windows-latest, macos-latest] arch: [auto] include: - - os: ubuntu-latest + - os: ubuntu-20.04 arch: aarch64 - os: macos-latest arch: arm64 @@ -265,7 +265,7 @@ jobs: - test_macos - test_windows_py27 - test_windows - runs-on: ubuntu-latest + runs-on: ubuntu-20.04 steps: - name: Checkout code uses: actions/checkout@v3 From 629bff1ba13fbc6f7331902e06d5e0981184f94b Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 10:09:39 +1100 Subject: [PATCH 12/38] Pin tox to older versions to avoid unknown pip issues. --- .github/workflows/main.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f93e3fb5..52e86ba1 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -33,7 +33,7 @@ jobs: - name: Update pip run: python -m pip install -U pip wheel setuptools - name: Install tox - run: python -m pip install tox tox-gh-actions + run: python -m pip install "tox<4.0.0" "tox-gh-actions<3.0.0" - name: Test with tox run: python -m tox - name: Store partial coverage reports @@ -72,7 +72,7 @@ jobs: # ${{ env.py }} -m venv .env && \ # source .env/bin/activate && \ # pip install -U pip wheel setuptools && \ - # pip install tox tox-gh-actions && \ + # pip install "tox<4.0.0" "tox-gh-actions<3.0.0" && \ # tox -e ${{ matrix.python.pyver }} && \ # deactivate' @@ -105,7 +105,7 @@ jobs: - name: Update pip run: python -m pip install -U pip wheel setuptools - name: Install tox - run: python -m pip install tox tox-gh-actions + run: python -m pip install "tox<4.0.0" "tox-gh-actions<3.0.0" - name: Test with tox run: python -m tox - name: Store partial coverage reports @@ -133,7 +133,7 @@ jobs: - name: Update pip run: python -m pip install -U pip wheel setuptools - name: Install tox - run: python -m pip install tox tox-gh-actions + run: python -m pip install "tox<4.0.0" "tox-gh-actions<3.0.0" - name: Test with tox run: python -m tox -e py27,py27-without-extensions @@ -165,7 +165,7 @@ jobs: - name: Update pip run: python -m pip install -U pip wheel setuptools - name: Install tox - run: python -m pip install tox tox-gh-actions + run: python -m pip install "tox<4.0.0" "tox-gh-actions<3.0.0" - name: Test with tox run: python -m tox From c9e66692e80595b4b2e097c27c8a1f5ef80b3db9 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 10:16:41 +1100 Subject: [PATCH 13/38] Update pypy versions used for testing. --- .github/workflows/main.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 52e86ba1..ad69aa0b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -21,8 +21,8 @@ jobs: - "3.10" - 3.11 - pypy-2.7 - - pypy-3.6 - - pypy-3.7 + - pypy-3.8 + - pypy-3.9 steps: - name: Checkout code uses: actions/checkout@v3 @@ -93,8 +93,8 @@ jobs: - "3.10" - 3.11 - pypy-2.7 - #- pypy-3.6 - - pypy-3.7 + - pypy-3.8 + - pypy-3.9 steps: - name: Checkout code uses: actions/checkout@v3 @@ -153,8 +153,8 @@ jobs: - "3.10" - 3.11 - pypy-2.7 - - pypy-3.6 - - pypy-3.7 + - pypy-3.8 + - pypy-3.9 steps: - name: Checkout code uses: actions/checkout@v3 From 292d9f725fcc06ccc4094580e038f756d3e413f2 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 11:06:47 +1100 Subject: [PATCH 14/38] Eliminate use of imp module in tests as removed in Python 3.12. --- tests/test_adapter.py | 4 ++-- tests/test_adapter_py3.py | 4 ++-- tests/test_adapter_py33.py | 4 ++-- tests/test_class.py | 4 ++-- tests/test_class_py37.py | 2 +- tests/test_function.py | 4 ++-- tests/test_inner_classmethod.py | 4 ++-- tests/test_inner_staticmethod.py | 4 ++-- tests/test_instancemethod.py | 4 ++-- tests/test_nested_function.py | 4 ++-- tests/test_object_proxy.py | 4 ++-- tests/test_outer_classmethod.py | 4 ++-- tests/test_outer_staticmethod.py | 4 ++-- 13 files changed, 25 insertions(+), 25 deletions(-) diff --git a/tests/test_adapter.py b/tests/test_adapter.py index 906210a8..85063c28 100644 --- a/tests/test_adapter.py +++ b/tests/test_adapter.py @@ -2,7 +2,7 @@ import unittest import inspect -import imp +import types import wrapt @@ -18,7 +18,7 @@ def adapter1(wrapped, instance, args, kwargs): return wrapped(*args, **kwargs) """ -decorators = imp.new_module('decorators') +decorators = types.ModuleType('decorators') exec_(DECORATORS_CODE, decorators.__dict__, decorators.__dict__) def function1(arg1, arg2): diff --git a/tests/test_adapter_py3.py b/tests/test_adapter_py3.py index 9291bd82..b6d48f17 100644 --- a/tests/test_adapter_py3.py +++ b/tests/test_adapter_py3.py @@ -2,7 +2,7 @@ import inspect import unittest -import imp +import types from typing import Iterable @@ -27,7 +27,7 @@ def adapter2(wrapped, instance, args, kwargs): return wrapped(*args, **kwargs) """ -decorators = imp.new_module('decorators') +decorators = types.ModuleType('decorators') exec_(DECORATORS_CODE, decorators.__dict__, decorators.__dict__) def function1(arg1, arg2) -> Iterable: diff --git a/tests/test_adapter_py33.py b/tests/test_adapter_py33.py index 063fba06..0cc8b2eb 100644 --- a/tests/test_adapter_py33.py +++ b/tests/test_adapter_py33.py @@ -2,7 +2,7 @@ import unittest import inspect -import imp +import types import wrapt @@ -18,7 +18,7 @@ def adapter1(wrapped, instance, args, kwargs): return wrapped(*args, **kwargs) """ -decorators = imp.new_module('decorators') +decorators = types.ModuleType('decorators') exec_(DECORATORS_CODE, decorators.__dict__, decorators.__dict__) def function1(arg1, arg2): diff --git a/tests/test_class.py b/tests/test_class.py index cf3b7e57..d0f83845 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -2,7 +2,7 @@ import unittest import inspect -import imp +import types import wrapt @@ -16,7 +16,7 @@ def passthru_decorator(wrapped, instance, args, kwargs): return wrapped(*args, **kwargs) """ -decorators = imp.new_module('decorators') +decorators = types.ModuleType('decorators') exec_(DECORATORS_CODE, decorators.__dict__, decorators.__dict__) class class1(object): diff --git a/tests/test_class_py37.py b/tests/test_class_py37.py index 435b46e2..cdab1b6d 100644 --- a/tests/test_class_py37.py +++ b/tests/test_class_py37.py @@ -2,7 +2,7 @@ import unittest import inspect -import imp +import types import wrapt diff --git a/tests/test_function.py b/tests/test_function.py index 73a3b432..cb77b35c 100644 --- a/tests/test_function.py +++ b/tests/test_function.py @@ -2,7 +2,7 @@ import unittest import inspect -import imp +import types import wrapt @@ -16,7 +16,7 @@ def passthru_decorator(wrapped, instance, args, kwargs): return wrapped(*args, **kwargs) """ -decorators = imp.new_module('decorators') +decorators = types.ModuleType('decorators') exec_(DECORATORS_CODE, decorators.__dict__, decorators.__dict__) def function1(arg): diff --git a/tests/test_inner_classmethod.py b/tests/test_inner_classmethod.py index d442d4f0..e2127ab9 100644 --- a/tests/test_inner_classmethod.py +++ b/tests/test_inner_classmethod.py @@ -1,7 +1,7 @@ from __future__ import print_function import unittest -import imp +import types import wrapt @@ -15,7 +15,7 @@ def passthru_decorator(wrapped, instance, args, kwargs): return wrapped(*args, **kwargs) """ -decorators = imp.new_module('decorators') +decorators = types.ModuleType('decorators') exec_(DECORATORS_CODE, decorators.__dict__, decorators.__dict__) class Class(object): diff --git a/tests/test_inner_staticmethod.py b/tests/test_inner_staticmethod.py index 31948653..bd3756f4 100644 --- a/tests/test_inner_staticmethod.py +++ b/tests/test_inner_staticmethod.py @@ -1,7 +1,7 @@ from __future__ import print_function import unittest -import imp +import types import wrapt @@ -15,7 +15,7 @@ def passthru_decorator(wrapped, instance, args, kwargs): return wrapped(*args, **kwargs) """ -decorators = imp.new_module('decorators') +decorators = types.ModuleType('decorators') exec_(DECORATORS_CODE, decorators.__dict__, decorators.__dict__) class Class(object): diff --git a/tests/test_instancemethod.py b/tests/test_instancemethod.py index 9ad6bb83..90e13ad7 100644 --- a/tests/test_instancemethod.py +++ b/tests/test_instancemethod.py @@ -2,7 +2,7 @@ import unittest import inspect -import imp +import types import wrapt @@ -16,7 +16,7 @@ def passthru_decorator(wrapped, instance, args, kwargs): return wrapped(*args, **kwargs) """ -decorators = imp.new_module('decorators') +decorators = types.ModuleType('decorators') exec_(DECORATORS_CODE, decorators.__dict__, decorators.__dict__) class OldClass1(): diff --git a/tests/test_nested_function.py b/tests/test_nested_function.py index a6c3bcd8..ca65263b 100644 --- a/tests/test_nested_function.py +++ b/tests/test_nested_function.py @@ -1,7 +1,7 @@ from __future__ import print_function import unittest -import imp +import types import wrapt @@ -15,7 +15,7 @@ def passthru_decorator(wrapped, instance, args, kwargs): return wrapped(*args, **kwargs) """ -decorators = imp.new_module('decorators') +decorators = types.ModuleType('decorators') exec_(DECORATORS_CODE, decorators.__dict__, decorators.__dict__) def function1(): diff --git a/tests/test_object_proxy.py b/tests/test_object_proxy.py index e2493063..b6d730ce 100644 --- a/tests/test_object_proxy.py +++ b/tests/test_object_proxy.py @@ -1,7 +1,7 @@ from __future__ import print_function import unittest -import imp +import types import operator import sys @@ -23,7 +23,7 @@ def target(): pass """ -objects = imp.new_module('objects') +objects = types.ModuleType('objects') exec_(OBJECTS_CODE, objects.__dict__, objects.__dict__) class TestAttributeAccess(unittest.TestCase): diff --git a/tests/test_outer_classmethod.py b/tests/test_outer_classmethod.py index c4c2d345..ab807646 100644 --- a/tests/test_outer_classmethod.py +++ b/tests/test_outer_classmethod.py @@ -1,7 +1,7 @@ from __future__ import print_function import unittest -import imp +import types import wrapt @@ -15,7 +15,7 @@ def passthru_decorator(wrapped, instance, args, kwargs): return wrapped(*args, **kwargs) """ -decorators = imp.new_module('decorators') +decorators = types.ModuleType('decorators') exec_(DECORATORS_CODE, decorators.__dict__, decorators.__dict__) class Class(object): diff --git a/tests/test_outer_staticmethod.py b/tests/test_outer_staticmethod.py index 2a8c1938..959c7ffd 100644 --- a/tests/test_outer_staticmethod.py +++ b/tests/test_outer_staticmethod.py @@ -1,7 +1,7 @@ from __future__ import print_function import unittest -import imp +import types import wrapt @@ -15,7 +15,7 @@ def passthru_decorator(wrapped, instance, args, kwargs): return wrapped(*args, **kwargs) """ -decorators = imp.new_module('decorators') +decorators = types.ModuleType('decorators') exec_(DECORATORS_CODE, decorators.__dict__, decorators.__dict__) class Class(object): From 46644e6514283b27c32c701343739cc31d35742a Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 11:11:25 +1100 Subject: [PATCH 15/38] Attempt to enable testing for Python 3.12 dev versions. --- .github/workflows/main.yml | 1 + setup.cfg | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ad69aa0b..dea7c38e 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -20,6 +20,7 @@ jobs: - 3.9 - "3.10" - 3.11 + - 3.12-dev - pypy-2.7 - pypy-3.8 - pypy-3.9 diff --git a/setup.cfg b/setup.cfg index 07831687..bb377761 100644 --- a/setup.cfg +++ b/setup.cfg @@ -72,8 +72,8 @@ norecursedirs = .tox venv [tox:tox] envlist = - py{27,35,36,37,38,39,310,311} - py{27,35,36,37,38,39,310,311}-{without,install,disable}-extensions, + py{27,35,36,37,38,39,310,311,312} + py{27,35,36,37,38,39,310,311,312}-{without,install,disable}-extensions, pypy-without-extensions [gh-actions] @@ -86,9 +86,10 @@ python = 3.9: py39, py39-without-extensions, py39-install-extensions, py39-disable-extensions 3.10: py310, py310-without-extensions, py310-install-extensions, py310-disable-extensions 3.11: py311, py311-without-extensions, py311-install-extensions, py311-disable-extensions + 3.12: py312, py312-without-extensions, py312-install-extensions, py312-disable-extensions pypy-2.7: pypy-without-extensions - pypy-3.6: pypy-without-extensions - pypy-3.7: pypy-without-extensions + pypy-3.8: pypy-without-extensions + pypy-3.9: pypy-without-extensions [testenv] deps = From dc5afd39533ce0daf933507f8a90c095448a5cbc Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 11:26:25 +1100 Subject: [PATCH 16/38] Update copyright year. --- LICENSE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LICENSE b/LICENSE index 1cc51172..bd8c7124 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2013-2022, Graham Dumpleton +Copyright (c) 2013-2023, Graham Dumpleton All rights reserved. Redistribution and use in source and binary forms, with or without From 230e1271364c904678f0975881a790a5ced5c19d Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 11:26:44 +1100 Subject: [PATCH 17/38] Update version for new changes. --- src/wrapt/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wrapt/__init__.py b/src/wrapt/__init__.py index ee6539b7..c5363524 100644 --- a/src/wrapt/__init__.py +++ b/src/wrapt/__init__.py @@ -1,4 +1,4 @@ -__version_info__ = ('1', '14', '1') +__version_info__ = ('1', '15', '0') __version__ = '.'.join(__version_info__) from .wrappers import (ObjectProxy, CallableObjectProxy, FunctionWrapper, From dd81bd7b5b604c58572c4863c25bdd2385d56468 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 12:52:34 +1100 Subject: [PATCH 18/38] Fix handing of exceptions in attribute wrappers on a proxy object. --- docs/changes.rst | 23 +++++++++++++++++ src/wrapt/_wrappers.c | 3 +++ tests/test_object_proxy.py | 52 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index b9e4aab5..aa231a62 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -1,6 +1,29 @@ Release Notes ============= +Version 1.15.0 +-------------- + +**Bugs Fixed** + +* When the C extension for wrapt was being used, and a property was used on an + object proxy wrapping another object to intercept access to an attribute of + the same name on the wrapped object, if the function implementing the property + raised an exception, then the exception was ignored and not propagated back to + the caller. What happened instead was that the original value of the attribute + from the wrapped object was returned, thus silently suppressing that an + exception had occurred in the wrapper. This behaviour was not happening when + the pure Python version of wrapt was being used, with it raising the + exception. The pure Python and C extension implementations thus did not behave + the same. + + Note that in the specific case that the exception raised is AttributeError it + still wouldn't be raised. This is the case for both Python and C extension + implementations. If a wrapper for an attribute internally raises an + AttributeError for some reason, the wrapper should if necessary catch the + exception and deal with it, or propagate it as a different exception type if + it is important that an exception still be passed back. + Version 1.14.1 -------------- diff --git a/src/wrapt/_wrappers.c b/src/wrapt/_wrappers.c index 67c5d5e1..7ff1085a 100644 --- a/src/wrapt/_wrappers.c +++ b/src/wrapt/_wrappers.c @@ -1535,6 +1535,9 @@ static PyObject *WraptObjectProxy_getattro( if (object) return object; + if (!PyErr_ExceptionMatches(PyExc_AttributeError)) + return NULL; + PyErr_Clear(); if (!getattr_str) { diff --git a/tests/test_object_proxy.py b/tests/test_object_proxy.py index b6d730ce..93586268 100644 --- a/tests/test_object_proxy.py +++ b/tests/test_object_proxy.py @@ -143,6 +143,58 @@ def function1(*args, **kwargs): self.assertEqual(getattr(function2, 'variable', None), None) + def test_attribute_lookup_modified(self): + class Object: + @property + def value(self): + return "value" + + class WrappedObject(wrapt.ObjectProxy): + @property + def value(self): + return 2 * self.__wrapped__.value + + WrappedObject(Object()).value == "valuevalue" + + def test_attribute_lookup_value_exception(self): + class Object: + @property + def value(self): + return "value" + + class WrappedObject(wrapt.ObjectProxy): + @property + def value(self): + raise ValueError("value-error") + + try: + WrappedObject(Object()).value == "value" + + except ValueError as e: + pass + + else: + raise RuntimeError("should not fail here") + + def test_attribute_lookup_attribute_exception(self): + class Object: + @property + def value(self): + return "value" + + class WrappedObject(wrapt.ObjectProxy): + @property + def value(self): + raise AttributeError("attribute-error") + + # Raising of an AttributeError in this case is a sort of odd situation + # because the exception results in it being determined there was no + # wrapper for the value attribute and so it returns the original value + # instead and robs the wrapper of the chance to return an alternate + # value. + + WrappedObject(Object()).value == "value" + class TestNamingObjectProxy(unittest.TestCase): def test_class_object_name(self): From 9bab80231385d4620d79c13b3d711f5264e21dbc Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 14:16:44 +1100 Subject: [PATCH 19/38] Add extra check on whether __loader__ attribute exists. --- src/wrapt/importer.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/wrapt/importer.py b/src/wrapt/importer.py index c3da6409..7d4a95d2 100644 --- a/src/wrapt/importer.py +++ b/src/wrapt/importer.py @@ -164,12 +164,22 @@ def _set_loader(self, module): # Set module's loader to self.loader unless it's already set to # something else. Import machinery will set it to spec.loader if it is # None, so handle None as well. The module may not support attribute - # assignment, in which case we simply skip it. - if getattr(module, "__loader__", None) in (None, self): + # assignment, in which case we simply skip it. Note that we also deal + # with __loader__ not existing at all. This is to future proof things + # due to proposal to remove the attribue as described in the GitHub + # issue at https://github.com/python/cpython/issues/77458. Also prior + # to Python 3.3, the __loader__ attribute was only set if a custom + # module loader was used. It isn't clear whether the attribute still + # existed in that case or was set to None. + + class UNDEFINED: pass + + if getattr(module, "__loader__", UNDEFINED) in (None, self): try: module.__loader__ = self.loader except AttributeError: pass + if (getattr(module, "__spec__", None) is not None and getattr(module.__spec__, "loader", None) is self): module.__spec__.loader = self.loader From 2d508e98121948feec8be7048b12dacf500f2064 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 14:34:26 +1100 Subject: [PATCH 20/38] Update test to validate behaviour prior to Python 3.3. --- tests/test_post_import_hooks.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/test_post_import_hooks.py b/tests/test_post_import_hooks.py index 391c5fd5..4f476591 100644 --- a/tests/test_post_import_hooks.py +++ b/tests/test_post_import_hooks.py @@ -73,19 +73,19 @@ def hook_this_two(module): self.assertEqual(len(invoked_two), 1) def test_loader(self): - try: - from importlib.machinery import SourceFileLoader - except ImportError: - return - @wrapt.when_imported('this') def hook_this(module): pass import this - self.assertIsInstance(this.__loader__, SourceFileLoader) - self.assertIsInstance(this.__spec__.loader, SourceFileLoader) + if sys.version_info[:2] >= (3, 3): + from importlib.machinery import SourceFileLoader + self.assertIsInstance(this.__loader__, SourceFileLoader) + self.assertIsInstance(this.__spec__.loader, SourceFileLoader) + + else: + self.assertIsInstance(this.__loader__, None) if __name__ == '__main__': unittest.main() From 424762f4b25213beb6e4977b21eb6fc68f2c32c6 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 14:38:47 +1100 Subject: [PATCH 21/38] Loader attribute will not exist for this case prior to Python 3.3. --- tests/test_post_import_hooks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_post_import_hooks.py b/tests/test_post_import_hooks.py index 4f476591..6330dd37 100644 --- a/tests/test_post_import_hooks.py +++ b/tests/test_post_import_hooks.py @@ -85,7 +85,7 @@ def hook_this(module): self.assertIsInstance(this.__spec__.loader, SourceFileLoader) else: - self.assertIsInstance(this.__loader__, None) + self.assertEqual(hasattr(this, "__loader__"), False) if __name__ == '__main__': unittest.main() From b1683a32a4c7eaf4225559fdaa94752a12016d50 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 15:06:44 +1100 Subject: [PATCH 22/38] Add details on changes to module loader tracking in module. --- docs/changes.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index aa231a62..2107bdb6 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -24,6 +24,12 @@ Version 1.15.0 exception and deal with it, or propagate it as a different exception type if it is important that an exception still be passed back. +* Address issue where the post import hook mechanism of wrapt wasn't transparent + and left the ``__loader__`` and ``__spec__.loader`` attributes of a module as + the wrapt import hook loader and not the original loader. That the original + loader wasn't preserved could interfere with code which needed access to the + original loader. + Version 1.14.1 -------------- From b729fb4b92a825b6a2267a4c67321952e7ce7a53 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 18:18:16 +1100 Subject: [PATCH 23/38] Clarify code comments. --- src/wrapt/importer.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/wrapt/importer.py b/src/wrapt/importer.py index fbbbb8e1..1e5e6886 100644 --- a/src/wrapt/importer.py +++ b/src/wrapt/importer.py @@ -64,11 +64,15 @@ def register_post_import_hook(hook, name): # to be called after import. module = sys.modules.get(name, None) + if module is None: _post_import_hooks.setdefault(name, []).append(hook) - # If the module is already imported, fire the hook right away. - # NOTE: Call the hook outside of the lock to avoid deadlocks. + # If the module is already imported, we fire the hook right away. Note that + # the hook is called outside of the lock to avoid deadlocks if code run as a + # consequence of calling the module import hook in turn triggers a separate + # thread which tries to register an import hook. + if module is not None: hook(module) @@ -100,10 +104,14 @@ def discover_post_import_hooks(group): def notify_module_loaded(module): name = getattr(module, '__name__', None) + with _post_import_hooks_lock: hooks = _post_import_hooks.pop(name, ()) - # NOTE: Call hooks outside of the lock to avoid deadlocks. + # Note that the hook is called outside of the lock to avoid deadlocks if + # code run as a consequence of calling the module import hook in turn + # triggers a separate thread which tries to register an import hook. + for hook in hooks: hook(module) From b27d6a693f8df39435377d165e740ddf64f4e9de Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 18:18:49 +1100 Subject: [PATCH 24/38] Replace user supplied tests for deadlock checks with three separate tests. --- tests/test_post_import_hooks.py | 101 +++++++++++++++++++++++--------- 1 file changed, 72 insertions(+), 29 deletions(-) diff --git a/tests/test_post_import_hooks.py b/tests/test_post_import_hooks.py index 298428cd..44e162c3 100644 --- a/tests/test_post_import_hooks.py +++ b/tests/test_post_import_hooks.py @@ -15,6 +15,7 @@ def setUp(self): # So we can import 'this' and test post-import hooks multiple times # below in the context of a single Python process, remove 'this' from # sys.modules and post import hooks. + sys.modules.pop('this', None) _post_import_hooks.pop('this', None) @@ -85,41 +86,83 @@ def hook_this(module): del sys.modules['this'] wrapt.register_post_import_hook(hook_this, 'this') import this + self.assertEqual(len(invoked), 2) - def test_import_deadlock(self): + def test_import_deadlock_1(self): + # This tries to verify that we haven't created a deadlock situation when + # code executed from a post module import, for a module that has already + # been imported, creates a thread which in turn attempts to register + # another import hook. + + import this + + @wrapt.when_imported('this') + def hook_this(module): + def worker(): + @wrapt.when_imported('xxx') + def hook_xxx(module): + pass + + thread = threading.Thread(target=worker) + thread.start() + thread.join(timeout=10) + + self.assertFalse(thread.is_alive()) + + del sys.modules['this'] + + def test_import_deadlock_2(self): + # This tries to verify that we haven't created a deadlock situation when + # code executed from a post module import, for a module that has not yet + # been imported, creates a thread which in turn attempts to register + # another import hook. + + @wrapt.when_imported('this') + def hook_this(module): + def worker(): + @wrapt.when_imported('xxx') + def hook_xxx(module): + pass + + thread = threading.Thread(target=worker) + thread.start() + thread.join(timeout=10) + + self.assertFalse(thread.is_alive()) + + import this + del sys.modules['this'] + + def test_import_deadlock_3(self): + # This tries to verify that we haven't created a deadlock situation when + # code executed from a post module import hook imports another module. + + hooks_called = [] + @wrapt.when_imported('this') def hook_this(module): - ev.set() - # The hook used to be called under _post_import_hooks_lock. Then - # the import tried to acquire the import lock. If the other thread - # already held it and was waiting for _post_import_hooks_lock, we - # deadlocked. - for _ in range(5): - import module1 - del sys.modules['module1'] - - def worker(): - ev.wait() - # The import tries to acquire the import lock. ImportHookFinder - # then tries to acquire _post_import_hooks_lock under it. - for _ in range(5): - import module2 - del sys.modules['module2'] - - # A deadlock between notify_module_loaded and ImportHookFinder. - ev = threading.Event() - thread = threading.Thread(target=worker) - thread.start() + hooks_called.append('this') + + self.assertFalse('wsgiref' in sys.modules) + + @wrapt.when_imported('wsgiref') + def hook_wsgiref(module): + hooks_called.append('wsgiref') + + def worker(): + import wsgiref + + thread = threading.Thread(target=worker) + thread.start() + thread.join(timeout=10) + + self.assertFalse(thread.is_alive()) + import this - thread.join() + del sys.modules['this'] - # A deadlock between register_post_import_hook and ImportHookFinder. - ev = threading.Event() - thread = threading.Thread(target=worker) - thread.start() - wrapt.register_post_import_hook(hook_this, 'this') - thread.join() + self.assertEqual(hooks_called, ['this', 'wsgiref']) def test_loader(self): @wrapt.when_imported('this') From a2d2c6d8142219ce7c259502749e9f384fe1ba57 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 18:41:14 +1100 Subject: [PATCH 25/38] Test cannot be done on Python 2.X. --- tests/test_post_import_hooks.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_post_import_hooks.py b/tests/test_post_import_hooks.py index 44e162c3..5113e8d2 100644 --- a/tests/test_post_import_hooks.py +++ b/tests/test_post_import_hooks.py @@ -7,6 +7,8 @@ import wrapt from wrapt.importer import _post_import_hooks +from compat import PY2, PY3 + class TestPostImportHooks(unittest.TestCase): def setUp(self): @@ -138,6 +140,17 @@ def test_import_deadlock_3(self): # This tries to verify that we haven't created a deadlock situation when # code executed from a post module import hook imports another module. + # Note that we cannot run this test on Python 2.X as it has a single + # global module import lock which means that if a thread runs during + # module import and it in turns does an import that it will then block + # on the parent thread which holds the global module import lock. This + # is a fundamental behaviour of Python and not wrapt. In Python 3.X + # there is a module import lock per named module and so we do not have + # this problem. + + if PY2: + return + hooks_called = [] @wrapt.when_imported('this') From eb5adbc17a0bb6a19e88e4d521a9d0af09985062 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Wed, 11 Jan 2023 18:41:28 +1100 Subject: [PATCH 26/38] Add version check for Python 3.12. --- tests/conftest.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index f5f73179..318d2263 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -37,6 +37,8 @@ def pytest_pycollect_makemodule(path, parent): return construct_dummy(path, parent) if '_py311' in path.basename and version < (3, 11): return construct_dummy(path, parent) + if '_py312' in path.basename and version < (3, 12): + return construct_dummy(path, parent) if '_py3' in path.basename and version < (3, 0): return construct_dummy(path, parent) if '_py2' in path.basename and version >= (3, 0): From 7de364be610fbe2b6fcd4864e34a7d765dff7cc8 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Thu, 12 Jan 2023 09:44:13 +1100 Subject: [PATCH 27/38] Document fix for import loader thread deadlocks. --- docs/changes.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index 2107bdb6..69f7058b 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -30,6 +30,11 @@ Version 1.15.0 loader wasn't preserved could interfere with code which needed access to the original loader. +* Address issues where a thread deadlock could occur within the wrapt module + import handler, when code executed from a post import hook created a new + thread and code executed in the context of the new thread itself tried to + register a post import hook, or imported a new module. + Version 1.14.1 -------------- From 2418edd2d4fb64527a5e56fdee04fe810fc47fbe Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Thu, 12 Jan 2023 13:53:27 +1100 Subject: [PATCH 28/38] Fix issues when using keyword argument named self with object proxies. --- docs/changes.rst | 11 ++ src/wrapt/wrappers.py | 21 ++- tests/test_object_proxy.py | 356 +++++++++++++++++++++++++++++++++++++ 3 files changed, 385 insertions(+), 3 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 69f7058b..f7c79087 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -35,6 +35,17 @@ Version 1.15.0 thread and code executed in the context of the new thread itself tried to register a post import hook, or imported a new module. +* When using `CallableObjectProxy` as a wrapper for a type or function and + calling the wrapped object, it was not possible to pass a keyword argument + named ``self``. This only occurred when using the pure Python version of + wrapt and did not occur when using the C extension based implementation. + +* When using `PartialCallableObjectProxy` as a wrapper for a type or function, + when constructing the partial object and when calling the partial object, it + was not possible to pass a keyword argument named ``self``. This only occurred + when using the pure Python version of wrapt and did not occur when using the C + extension based implementation. + Version 1.14.1 -------------- diff --git a/src/wrapt/wrappers.py b/src/wrapt/wrappers.py index 2716cd1d..46c8e0e4 100644 --- a/src/wrapt/wrappers.py +++ b/src/wrapt/wrappers.py @@ -445,12 +445,22 @@ def __reduce_ex__(self, protocol): class CallableObjectProxy(ObjectProxy): - def __call__(self, *args, **kwargs): + def __call__(*args, **kwargs): + def _unpack_self(self, *args): + return self, args + + self, args = _unpack_self(*args) + return self.__wrapped__(*args, **kwargs) class PartialCallableObjectProxy(ObjectProxy): - def __init__(self, *args, **kwargs): + def __init__(*args, **kwargs): + def _unpack_self(self, *args): + return self, args + + self, args = _unpack_self(*args) + if len(args) < 1: raise TypeError('partial type takes at least one argument') @@ -464,7 +474,12 @@ def __init__(self, *args, **kwargs): self._self_args = args self._self_kwargs = kwargs - def __call__(self, *args, **kwargs): + def __call__(*args, **kwargs): + def _unpack_self(self, *args): + return self, args + + self, args = _unpack_self(*args) + _args = self._self_args + args _kwargs = dict(self._self_kwargs) diff --git a/tests/test_object_proxy.py b/tests/test_object_proxy.py index 93586268..19deffed 100644 --- a/tests/test_object_proxy.py +++ b/tests/test_object_proxy.py @@ -1843,5 +1843,361 @@ def test_fractions_round(self): self.assertEqual(round(instance), round(proxy)) +class TestArgumentUnpacking(unittest.TestCase): + + def test_self_keyword_argument_on_dict(self): + # A dict when given self as keyword argument uses it to create item in + # the dict and no attempt is made to use a positional argument. + + d = wrapt.wrappers.CallableObjectProxy(dict)(self='self') + + self.assertEqual(d, dict(self='self')) + + def test_self_positional_argument_on_class_init(self): + class Object: + def __init__(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + + o = Object('arg1') + + self.assertEqual(o._args, ('arg1',)) + self.assertEqual(o._kwargs, {}) + + o = wrapt.wrappers.CallableObjectProxy(Object)('arg1') + + self.assertEqual(o._args, ('arg1',)) + self.assertEqual(o._kwargs, {}) + + def test_self_keyword_argument_on_class_init_1(self): + class Object: + def __init__(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + + with self.assertRaises(TypeError) as e: + Object(self='self') + + self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + + with self.assertRaises(TypeError) as e: + wrapt.wrappers.CallableObjectProxy(Object)(self='self') + + self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + + def test_self_keyword_argument_on_class_init_2(self): + class Object: + def __init__(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + + with self.assertRaises(TypeError) as e: + Object(arg1='arg1', self='self') + + self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + + with self.assertRaises(TypeError) as e: + wrapt.wrappers.CallableObjectProxy(Object)(arg1='arg1', self='self') + + self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + + def test_self_keyword_argument_on_class_init_renamed(self): + class Object: + def __init__(_self, *args, **kwargs): + _self._args = args + _self._kwargs = kwargs + + o = Object(self='self') + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, dict(self="self")) + + o = wrapt.wrappers.CallableObjectProxy(Object)(self='self') + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, dict(self="self")) + + def test_self_keyword_argument_on_class_init_overloaded_1(self): + class Object: + def __init__(_self, self, *args, **kwargs): + _self._self = self + _self._args = args + _self._kwargs = kwargs + + o = Object(self='self') + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, {}) + self.assertEqual(o._self, 'self') + + o = wrapt.wrappers.CallableObjectProxy(Object)(self='self') + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, {}) + self.assertEqual(o._self, 'self') + + def test_self_keyword_argument_on_class_init_overloaded_2(self): + class Object: + def __init__(_self, self, *args, **kwargs): + _self._self = self + _self._args = args + _self._kwargs = kwargs + + with self.assertRaises(TypeError) as e: + Object(_self='self') + + self.assertTrue("got multiple values for argument '_self'" in str(e.exception)) + + with self.assertRaises(TypeError) as e: + wrapt.wrappers.CallableObjectProxy(Object)(_self='self') + + self.assertTrue("got multiple values for argument '_self'" in str(e.exception)) + +class TestArgumentUnpackingPartial(unittest.TestCase): + + def test_self_keyword_argument_on_dict_1(self): + # A dict when given self as keyword argument uses it to create item in + # the dict and no attempt is made to use a positional argument. + + wrapper = wrapt.wrappers.PartialCallableObjectProxy(dict, arg1='arg1') + + d = wrapper(self='self') + + self.assertEqual(d, dict(self='self', arg1='arg1')) + + def test_self_keyword_argument_on_dict_2(self): + # A dict when given self as keyword argument uses it to create item in + # the dict and no attempt is made to use a positional argument. + + wrapper = wrapt.wrappers.PartialCallableObjectProxy(dict, self='self') + + d = wrapper(arg1='arg1') + + self.assertEqual(d, dict(self='self', arg1='arg1')) + + def test_self_positional_argument_on_class_init_1(self): + class Object: + def __init__(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + + o = Object('arg1') + + self.assertEqual(o._args, ('arg1',)) + self.assertEqual(o._kwargs, {}) + + wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object, 'arg1') + + o = wrapper() + + self.assertEqual(o._args, ('arg1',)) + self.assertEqual(o._kwargs, {}) + + def test_self_positional_argument_on_class_init_2(self): + class Object: + def __init__(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + + o = Object('arg1') + + self.assertEqual(o._args, ('arg1',)) + self.assertEqual(o._kwargs, {}) + + wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object) + + o = wrapper('arg1') + + self.assertEqual(o._args, ('arg1',)) + self.assertEqual(o._kwargs, {}) + + def test_self_keyword_argument_on_class_init_1a(self): + class Object: + def __init__(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + + with self.assertRaises(TypeError) as e: + Object(self='self') + + self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + + wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object, self='self') + + with self.assertRaises(TypeError) as e: + o = wrapper() + + self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + + def test_self_keyword_argument_on_class_init_1b(self): + class Object: + def __init__(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + + with self.assertRaises(TypeError) as e: + Object(self='self') + + self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + + wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object) + + with self.assertRaises(TypeError) as e: + o = wrapper(self='self') + + self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + + def test_self_keyword_argument_on_class_init_2a(self): + class Object: + def __init__(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + + with self.assertRaises(TypeError) as e: + Object(arg1='arg1', self='self') + + self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + + wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object, arg1='arg1', self='self') + + with self.assertRaises(TypeError) as e: + o = wrapper() + + self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + + def test_self_keyword_argument_on_class_init_2b(self): + class Object: + def __init__(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + + with self.assertRaises(TypeError) as e: + Object(arg1='arg1', self='self') + + self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + + wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object) + + with self.assertRaises(TypeError) as e: + o = wrapper(arg1='arg1', self='self') + + self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + + def test_self_keyword_argument_on_class_init_renamed_1(self): + class Object: + def __init__(_self, *args, **kwargs): + _self._args = args + _self._kwargs = kwargs + + o = Object(self='self') + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, dict(self="self")) + + wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object, self='self') + + o = wrapper() + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, dict(self="self")) + + def test_self_keyword_argument_on_class_init_renamed_2(self): + class Object: + def __init__(_self, *args, **kwargs): + _self._args = args + _self._kwargs = kwargs + + o = Object(self='self') + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, dict(self="self")) + + wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object) + + o = wrapper(self='self') + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, dict(self="self")) + + def test_self_keyword_argument_on_class_init_overloaded_1a(self): + class Object: + def __init__(_self, self, *args, **kwargs): + _self._self = self + _self._args = args + _self._kwargs = kwargs + + o = Object(self='self') + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, {}) + self.assertEqual(o._self, 'self') + + wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object, self='self') + + o = wrapper() + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, {}) + self.assertEqual(o._self, 'self') + + def test_self_keyword_argument_on_class_init_overloaded_1b(self): + class Object: + def __init__(_self, self, *args, **kwargs): + _self._self = self + _self._args = args + _self._kwargs = kwargs + + o = Object(self='self') + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, {}) + self.assertEqual(o._self, 'self') + + wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object) + + o = wrapper(self='self') + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, {}) + self.assertEqual(o._self, 'self') + + def test_self_keyword_argument_on_class_init_overloaded_2a(self): + class Object: + def __init__(_self, self, *args, **kwargs): + _self._self = self + _self._args = args + _self._kwargs = kwargs + + with self.assertRaises(TypeError) as e: + Object(_self='self') + + self.assertTrue("got multiple values for argument '_self'" in str(e.exception)) + + wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object, _self='self') + + with self.assertRaises(TypeError) as e: + o = wrapper() + + self.assertTrue("got multiple values for argument '_self'" in str(e.exception)) + + def test_self_keyword_argument_on_class_init_overloaded_2b(self): + class Object: + def __init__(_self, self, *args, **kwargs): + _self._self = self + _self._args = args + _self._kwargs = kwargs + + with self.assertRaises(TypeError) as e: + Object(_self='self') + + self.assertTrue("got multiple values for argument '_self'" in str(e.exception)) + + wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object) + + with self.assertRaises(TypeError) as e: + o = wrapper(_self='self') + + self.assertTrue("got multiple values for argument '_self'" in str(e.exception)) + if __name__ == '__main__': unittest.main() From 7cbdc9524411526f32e2a171b0b3195a24114065 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Thu, 12 Jan 2023 14:08:27 +1100 Subject: [PATCH 29/38] On Python 2 the format of exception differs to Python 3. --- tests/test_object_proxy.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/test_object_proxy.py b/tests/test_object_proxy.py index 19deffed..2c9f631b 100644 --- a/tests/test_object_proxy.py +++ b/tests/test_object_proxy.py @@ -2,8 +2,8 @@ import unittest import types -import operator import sys +import re is_pypy = '__pypy__' in sys.builtin_module_names @@ -1878,12 +1878,12 @@ def __init__(self, *args, **kwargs): with self.assertRaises(TypeError) as e: Object(self='self') - self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) with self.assertRaises(TypeError) as e: wrapt.wrappers.CallableObjectProxy(Object)(self='self') - self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) def test_self_keyword_argument_on_class_init_2(self): class Object: @@ -1894,12 +1894,12 @@ def __init__(self, *args, **kwargs): with self.assertRaises(TypeError) as e: Object(arg1='arg1', self='self') - self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) with self.assertRaises(TypeError) as e: wrapt.wrappers.CallableObjectProxy(Object)(arg1='arg1', self='self') - self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) def test_self_keyword_argument_on_class_init_renamed(self): class Object: @@ -2020,14 +2020,14 @@ def __init__(self, *args, **kwargs): with self.assertRaises(TypeError) as e: Object(self='self') - self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object, self='self') with self.assertRaises(TypeError) as e: o = wrapper() - self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) def test_self_keyword_argument_on_class_init_1b(self): class Object: @@ -2038,14 +2038,14 @@ def __init__(self, *args, **kwargs): with self.assertRaises(TypeError) as e: Object(self='self') - self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object) with self.assertRaises(TypeError) as e: o = wrapper(self='self') - self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) def test_self_keyword_argument_on_class_init_2a(self): class Object: @@ -2056,14 +2056,14 @@ def __init__(self, *args, **kwargs): with self.assertRaises(TypeError) as e: Object(arg1='arg1', self='self') - self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object, arg1='arg1', self='self') with self.assertRaises(TypeError) as e: o = wrapper() - self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) def test_self_keyword_argument_on_class_init_2b(self): class Object: @@ -2074,14 +2074,14 @@ def __init__(self, *args, **kwargs): with self.assertRaises(TypeError) as e: Object(arg1='arg1', self='self') - self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object) with self.assertRaises(TypeError) as e: o = wrapper(arg1='arg1', self='self') - self.assertTrue("got multiple values for argument 'self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) def test_self_keyword_argument_on_class_init_renamed_1(self): class Object: From ee6bab6989fdcf1eeedf1601f8f23fcff3e95730 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Thu, 12 Jan 2023 14:13:54 +1100 Subject: [PATCH 30/38] On Python 2 the format of exception differs to Python 3. --- tests/test_object_proxy.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_object_proxy.py b/tests/test_object_proxy.py index 2c9f631b..3cd3cd66 100644 --- a/tests/test_object_proxy.py +++ b/tests/test_object_proxy.py @@ -1946,12 +1946,12 @@ def __init__(_self, self, *args, **kwargs): with self.assertRaises(TypeError) as e: Object(_self='self') - self.assertTrue("got multiple values for argument '_self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument '_self'.*", str(e.exception)), None) with self.assertRaises(TypeError) as e: wrapt.wrappers.CallableObjectProxy(Object)(_self='self') - self.assertTrue("got multiple values for argument '_self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument '_self'.*", str(e.exception)), None) class TestArgumentUnpackingPartial(unittest.TestCase): @@ -2171,14 +2171,14 @@ def __init__(_self, self, *args, **kwargs): with self.assertRaises(TypeError) as e: Object(_self='self') - self.assertTrue("got multiple values for argument '_self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument '_self'.*", str(e.exception)), None) wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object, _self='self') with self.assertRaises(TypeError) as e: o = wrapper() - self.assertTrue("got multiple values for argument '_self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument '_self'.*", str(e.exception)), None) def test_self_keyword_argument_on_class_init_overloaded_2b(self): class Object: @@ -2190,14 +2190,14 @@ def __init__(_self, self, *args, **kwargs): with self.assertRaises(TypeError) as e: Object(_self='self') - self.assertTrue("got multiple values for argument '_self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument '_self'.*", str(e.exception)), None) wrapper = wrapt.wrappers.PartialCallableObjectProxy(Object) with self.assertRaises(TypeError) as e: o = wrapper(_self='self') - self.assertTrue("got multiple values for argument '_self'" in str(e.exception)) + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument '_self'.*", str(e.exception)), None) if __name__ == '__main__': unittest.main() From 4e09f7ac1d8e936a707ad33f84edb55c248a144c Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Thu, 12 Jan 2023 16:05:30 +1100 Subject: [PATCH 31/38] Fix issues when using keyword argument named self with function wrappers and decorators. --- docs/changes.rst | 18 ++- src/wrapt/wrappers.py | 14 ++- tests/test_object_proxy.py | 218 +++++++++++++++++++++++++++++++++++++ 3 files changed, 244 insertions(+), 6 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index f7c79087..7e0f5132 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -35,17 +35,27 @@ Version 1.15.0 thread and code executed in the context of the new thread itself tried to register a post import hook, or imported a new module. -* When using `CallableObjectProxy` as a wrapper for a type or function and +* When using ``CallableObjectProxy`` as a wrapper for a type or function and calling the wrapped object, it was not possible to pass a keyword argument - named ``self``. This only occurred when using the pure Python version of - wrapt and did not occur when using the C extension based implementation. + named ``self``. This only occurred when using the pure Python version of wrapt + and did not occur when using the C extension based implementation. -* When using `PartialCallableObjectProxy` as a wrapper for a type or function, +* When using ``PartialCallableObjectProxy`` as a wrapper for a type or function, when constructing the partial object and when calling the partial object, it was not possible to pass a keyword argument named ``self``. This only occurred when using the pure Python version of wrapt and did not occur when using the C extension based implementation. +* When using ``FunctionWrapper`` as a wrapper for a type or function and calling + the wrapped object, it was not possible to pass a keyword argument named + ``self``. Because ``FunctionWrapper`` is also used by decorators, this also + affected decorators on functions and class types. A similar issue also arose + when these were applied to class and instance methods where binding occurred + when the method was accessed. In that case it was in ``BoundFunctionWrapper`` + that the problem could arise. These all only occurred when using the pure + Python version of wrapt and did not occur when using the C extension based + implementation. + Version 1.14.1 -------------- diff --git a/src/wrapt/wrappers.py b/src/wrapt/wrappers.py index 46c8e0e4..062b0956 100644 --- a/src/wrapt/wrappers.py +++ b/src/wrapt/wrappers.py @@ -559,7 +559,12 @@ def __get__(self, instance, owner): return self - def __call__(self, *args, **kwargs): + def __call__(*args, **kwargs): + def _unpack_self(self, *args): + return self, args + + self, args = _unpack_self(*args) + # If enabled has been specified, then evaluate it at this point # and if the wrapper is not to be executed, then simply return # the bound function rather than a bound wrapper for the bound @@ -622,7 +627,12 @@ def __subclasscheck__(self, subclass): class BoundFunctionWrapper(_FunctionWrapperBase): - def __call__(self, *args, **kwargs): + def __call__(*args, **kwargs): + def _unpack_self(self, *args): + return self, args + + self, args = _unpack_self(*args) + # If enabled has been specified, then evaluate it at this point # and if the wrapper is not to be executed, then simply return # the bound function rather than a bound wrapper for the bound diff --git a/tests/test_object_proxy.py b/tests/test_object_proxy.py index 3cd3cd66..50522b27 100644 --- a/tests/test_object_proxy.py +++ b/tests/test_object_proxy.py @@ -2199,5 +2199,223 @@ def __init__(_self, self, *args, **kwargs): self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument '_self'.*", str(e.exception)), None) +class TestArgumentUnpackingWrapperBase(unittest.TestCase): + + def test_self_keyword_argument_on_dict(self): + # A dict when given self as keyword argument uses it to create item in + # the dict and no attempt is made to use a positional argument. + + def wrapper(wrapped, instance, args, kwargs): + return wrapped(*args, **kwargs) + + d = wrapt.wrappers.FunctionWrapper(dict, wrapper)(self='self') + + self.assertEqual(d, dict(self='self')) + + def test_self_positional_argument_on_class_init(self): + def wrapper(wrapped, instance, args, kwargs): + return wrapped(*args, **kwargs) + + class Object: + def __init__(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + + o = Object('arg1') + + self.assertEqual(o._args, ('arg1',)) + self.assertEqual(o._kwargs, {}) + + o = wrapt.wrappers.FunctionWrapper(Object, wrapper)('arg1') + + self.assertEqual(o._args, ('arg1',)) + self.assertEqual(o._kwargs, {}) + + def test_self_keyword_argument_on_class_init_1(self): + def wrapper(wrapped, instance, args, kwargs): + return wrapped(*args, **kwargs) + + class Object: + def __init__(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + + with self.assertRaises(TypeError) as e: + Object(self='self') + + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) + + with self.assertRaises(TypeError) as e: + wrapt.wrappers.FunctionWrapper(Object, wrapper)(self='self') + + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) + + def test_self_keyword_argument_on_class_init_2(self): + def wrapper(wrapped, instance, args, kwargs): + return wrapped(*args, **kwargs) + + class Object: + def __init__(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + + with self.assertRaises(TypeError) as e: + Object(arg1='arg1', self='self') + + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) + + with self.assertRaises(TypeError) as e: + wrapt.wrappers.FunctionWrapper(Object, wrapper)(arg1='arg1', self='self') + + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument 'self'.*", str(e.exception)), None) + + def test_self_keyword_argument_on_class_init_renamed(self): + def wrapper(wrapped, instance, args, kwargs): + return wrapped(*args, **kwargs) + + class Object: + def __init__(_self, *args, **kwargs): + _self._args = args + _self._kwargs = kwargs + + o = Object(self='self') + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, dict(self="self")) + + o = wrapt.wrappers.FunctionWrapper(Object, wrapper)(self='self') + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, dict(self="self")) + + def test_self_keyword_argument_on_class_init_overloaded_1(self): + def wrapper(wrapped, instance, args, kwargs): + return wrapped(*args, **kwargs) + + class Object: + def __init__(_self, self, *args, **kwargs): + _self._self = self + _self._args = args + _self._kwargs = kwargs + + o = Object(self='self') + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, {}) + self.assertEqual(o._self, 'self') + + o = wrapt.wrappers.FunctionWrapper(Object, wrapper)(self='self') + + self.assertEqual(o._args, ()) + self.assertEqual(o._kwargs, {}) + self.assertEqual(o._self, 'self') + + def test_self_keyword_argument_on_class_init_overloaded_2(self): + def wrapper(wrapped, instance, args, kwargs): + return wrapped(*args, **kwargs) + + class Object: + def __init__(_self, self, *args, **kwargs): + _self._self = self + _self._args = args + _self._kwargs = kwargs + + with self.assertRaises(TypeError) as e: + Object(_self='self') + + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument '_self'.*", str(e.exception)), None) + + with self.assertRaises(TypeError) as e: + wrapt.wrappers.FunctionWrapper(Object, wrapper)(_self='self') + + self.assertNotEqual(re.match(".*got multiple values for (keyword )?argument '_self'.*", str(e.exception)), None) + +class TestArgumentUnpackingBoundFunctionWrapper(unittest.TestCase): + + def test_self_keyword_argument_on_classmethod(self): + def wrapper(wrapped, instance, args, kwargs): + return wrapped(*args, **kwargs) + + class Object: + @classmethod + def function(cls, self, *args, **kwargs): + return self, args, kwargs + + function = wrapt.wrappers.FunctionWrapper(function, wrapper) + + result = Object().function(self='self') + + self.assertEqual(result, ('self', (), {})) + + def test_self_keyword_argument_on_instancemethod(self): + def wrapper(wrapped, instance, args, kwargs): + return wrapped(*args, **kwargs) + + class Object: + def function(_self, self, *args, **kwargs): + return self, args, kwargs + + function = wrapt.wrappers.FunctionWrapper(function, wrapper) + + result = Object().function(self='self') + + self.assertEqual(result, ('self', (), {})) + +class TestArgumentUnpackingDecorator(unittest.TestCase): + + def test_self_keyword_argument_on_function(self): + @wrapt.decorator + def wrapper(wrapped, instance, args, kwargs): + return wrapped(*args, **kwargs) + + @wrapper + def function(self, *args, **kwargs): + return self, args, kwargs + + result = function(self='self') + + self.assertEqual(result, ('self', (), {})) + + result = function('self') + + self.assertEqual(result, ('self', (), {})) + + def test_self_keyword_argument_on_classmethod(self): + @wrapt.decorator + def wrapper(wrapped, instance, args, kwargs): + return wrapped(*args, **kwargs) + + class Object: + @wrapper + @classmethod + def function(cls, self, *args, **kwargs): + return self, args, kwargs + + result = Object().function(self='self') + + self.assertEqual(result, ('self', (), {})) + + result = Object().function('self', arg1='arg1') + + self.assertEqual(result, ('self', (), dict(arg1='arg1'))) + + def test_self_keyword_argument_on_instancemethod(self): + @wrapt.decorator + def wrapper(wrapped, instance, args, kwargs): + return wrapped(*args, **kwargs) + + class Object: + @wrapper + def function(_self, self, *args, **kwargs): + return self, args, kwargs + + result = Object().function(self='self', arg1='arg1') + + self.assertEqual(result, ('self', (), dict(arg1='arg1'))) + + result = Object().function('self', arg1='arg1') + + self.assertEqual(result, ('self', (), dict(arg1='arg1'))) + if __name__ == '__main__': unittest.main() From 32a07205a41020019add7a9e4e9bbf2def1d5aaf Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Thu, 12 Jan 2023 16:33:07 +1100 Subject: [PATCH 32/38] Fix issues when using keyword argument named self with weak function proxy. --- docs/changes.rst | 4 ++++ src/wrapt/wrappers.py | 7 ++++++- tests/test_weak_function_proxy.py | 10 ++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/docs/changes.rst b/docs/changes.rst index 7e0f5132..2e5c6c34 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -56,6 +56,10 @@ Version 1.15.0 Python version of wrapt and did not occur when using the C extension based implementation. +* When using ``WeakFunctionProxy`` as a wrapper for a function, when calling the + function via the proxy object, it was not possible to pass a keyword argument + named ``self``. + Version 1.14.1 -------------- diff --git a/src/wrapt/wrappers.py b/src/wrapt/wrappers.py index 062b0956..48f334ee 100644 --- a/src/wrapt/wrappers.py +++ b/src/wrapt/wrappers.py @@ -992,7 +992,12 @@ def __init__(self, wrapped, callback=None): super(WeakFunctionProxy, self).__init__( weakref.proxy(wrapped, _callback)) - def __call__(self, *args, **kwargs): + def __call__(*args, **kwargs): + def _unpack_self(self, *args): + return self, args + + self, args = _unpack_self(*args) + # We perform a boolean check here on the instance and wrapped # function as that will trigger the reference error prior to # calling if the reference had expired. diff --git a/tests/test_weak_function_proxy.py b/tests/test_weak_function_proxy.py index d34da0e1..fa4b516b 100644 --- a/tests/test_weak_function_proxy.py +++ b/tests/test_weak_function_proxy.py @@ -190,5 +190,15 @@ def squeal(self): self.assertEqual(method(), 'bark') +class TestArgumentUnpackingWeakFunctionProxy(unittest.TestCase): + + def test_self_keyword_argument(self): + def function(self, *args, **kwargs): + return self, args, kwargs + + proxy = wrapt.wrappers.WeakFunctionProxy(function) + + self.assertEqual(proxy(self='self', arg1='arg1'), ('self', (), dict(arg1='arg1'))) + if __name__ == '__main__': unittest.main() From 511a8cec1d19486ed12c5f18ee74f52f6c0e7e06 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Thu, 12 Jan 2023 16:51:23 +1100 Subject: [PATCH 33/38] Remove .tox directory when doing clean. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ccf3e293..3f9392c2 100644 --- a/Makefile +++ b/Makefile @@ -15,7 +15,7 @@ mostlyclean: rm -rf .coverage.* clean: mostlyclean - rm -rf build dist wrapt.egg-info + rm -rf build dist wrapt.egg-info .tox test : tox --skip-missing-interpreters From bb8d37af60eea5204fdf32f76c9f00b971a381bb Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Thu, 12 Jan 2023 16:57:51 +1100 Subject: [PATCH 34/38] Attempt to build a pure Python source wheel. --- .github/workflows/main.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index dea7c38e..6dbdb27f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -186,8 +186,10 @@ jobs: uses: actions/setup-python@v4 with: python-version: 3.9 - - name: Build source distribution + - name: Build full source distribution as tar.gz run: python setup.py sdist + - name: Build pure Python source wheel + run: WRAPT_INSTALL_EXTENSIONS=false python setup.py bdist_wheel - name: Store built wheels uses: actions/upload-artifact@v3 with: From 3a6a771877c934a740e4de538fbbddb4498ffd24 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Thu, 12 Jan 2023 17:09:35 +1100 Subject: [PATCH 35/38] Need to install wheel package for bdist_wheel. --- .github/workflows/main.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6dbdb27f..b4a29d8a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -188,6 +188,8 @@ jobs: python-version: 3.9 - name: Build full source distribution as tar.gz run: python setup.py sdist + - name: Install the wheel package + run: pip install wheel - name: Build pure Python source wheel run: WRAPT_INSTALL_EXTENSIONS=false python setup.py bdist_wheel - name: Store built wheels From c11181dc7b891c9e59391a24379f89a8cdac9092 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Thu, 12 Jan 2023 17:57:37 +1100 Subject: [PATCH 36/38] Set version to 1.15.0rc1 for release. --- src/wrapt/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wrapt/__init__.py b/src/wrapt/__init__.py index c5363524..bff03660 100644 --- a/src/wrapt/__init__.py +++ b/src/wrapt/__init__.py @@ -1,4 +1,4 @@ -__version_info__ = ('1', '15', '0') +__version_info__ = ('1', '15', '0rc1') __version__ = '.'.join(__version_info__) from .wrappers import (ObjectProxy, CallableObjectProxy, FunctionWrapper, From d446a4f340b93fed41dfe884aa0311195b301f16 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Mon, 27 Feb 2023 11:46:58 +1100 Subject: [PATCH 37/38] Remove egg info directory. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 3f9392c2..7655f037 100644 --- a/Makefile +++ b/Makefile @@ -15,7 +15,7 @@ mostlyclean: rm -rf .coverage.* clean: mostlyclean - rm -rf build dist wrapt.egg-info .tox + rm -rf build dist src/wrapt.egg-info .tox test : tox --skip-missing-interpreters From ba845c6793eab868cad957b2763fa121333fdfc1 Mon Sep 17 00:00:00 2001 From: Graham Dumpleton Date: Mon, 27 Feb 2023 11:50:00 +1100 Subject: [PATCH 38/38] Update version to 1.15.0 for release. --- src/wrapt/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wrapt/__init__.py b/src/wrapt/__init__.py index bff03660..c5363524 100644 --- a/src/wrapt/__init__.py +++ b/src/wrapt/__init__.py @@ -1,4 +1,4 @@ -__version_info__ = ('1', '15', '0rc1') +__version_info__ = ('1', '15', '0') __version__ = '.'.join(__version_info__) from .wrappers import (ObjectProxy, CallableObjectProxy, FunctionWrapper,