Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

potential pypy mutability bug #620

Closed
1 task done
iscai-msft opened this issue Aug 26, 2021 · 3 comments · Fixed by #628
Closed
1 task done

potential pypy mutability bug #620

iscai-msft opened this issue Aug 26, 2021 · 3 comments · Fixed by #628

Comments

@iscai-msft
Copy link
Contributor

Describe the bug

when using pypy and mutating a CIMultiDict, I get a RuntimeError of "Dictionary changed during iteration". This doesn't happen using regular python, and I don't see this error when using pypy and other kinds of dictionaries.

To Reproduce

I've added the following tests to repro with pypy

@pytest.mark.asyncio
async def test_headers_response_items_multidict_compare():
    h = CIMultiDict({'a': '123, 456', 'b': '789'})
    before_mutation_items = h.items()
    h['c'] = '000'
    with pytest.raises(RuntimeError):
        list(before_mutation_items)

def test_headers_response_items_requests_compare():
    from requests.structures import CaseInsensitiveDict
    h = CaseInsensitiveDict({'a': '123, 456', 'b': '789'})
    before_mutation_items = h.items()
    h['c'] = '000'
    list(before_mutation_items)

def test_headers_response_items_python_compare():
    h = {'a': '123, 456', 'b': '789'}
    before_mutation_items = h.items()
    h['c'] = '000'
    list(before_mutation_items)

Expected behavior

My thinking is that if mutating the dictionary doesn't raise for regular python is that it shouldn't raise for pypy. Is this expected behavior?

Logs/tracebacks

self = _ItemsView('a': '123, 456', 'b': '789', 'c': '000')

    def _iter(self):
        for i, k, v in self._impl._items:
            if self._version != self._impl._version:
>               raise RuntimeError("Dictionary changed during iteration")
E               RuntimeError: Dictionary changed during iteration

envs/corepypy3venv/site-packages/multidict/_multidict_py.py:459: RuntimeError

Python Version

$ python --version
Python 3.7.10 (51efa818fd9b24f625078c65e8e2f6a5ac24d572, Apr 08 2021, 17:43:00)
[PyPy 7.3.4 with GCC Apple LLVM 12.0.0 (clang-1200.0.32.29)]

aiohttp Version

$ python -m pip show aiohttp
Version: 3.7.4.post0

multidict Version

$ python -m pip show multidict
Version: 5.1.0

yarl Version

$ python -m pip show yarl
Version: 1.6.3

OS

macOS

Related component

Client

Additional context

No response

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@Dreamsorcerer Dreamsorcerer transferred this issue from aio-libs/aiohttp Aug 26, 2021
@Dreamsorcerer
Copy link
Member

Looks like we have pypy running on the CI. So, if you could make a PR with the test reproducing the issue, that would be great.

@Dreamsorcerer Dreamsorcerer changed the title [multidict] potential pypy mutability bug potential pypy mutability bug Aug 26, 2021
@iscai-msft
Copy link
Contributor Author

hey apologies @Dreamsorcerer for the delayed response, I made a PR here #627. Still waiting on CI, also not sure if I'm supposed to add a changelog entry for just adding a test. Thanks!

@Dreamsorcerer
Copy link
Member

I'm not really familiar with the code, but this appears to be an explicit check:
https://github.com/aio-libs/multidict/blob/master/multidict/_multidict_py.py#L483

I suspect one option would be to reset the version at the beginning of that _iter() method, rather than causing an error any time that is reused. It may also be fine to just remove that check, as I suspect the same error would occur if changing the items while iterating over in the for loop.

asvetlov added a commit that referenced this issue Sep 30, 2021
asvetlov added a commit that referenced this issue Sep 30, 2021
* add pypy test

* Lint

* Fix #620: Don't raise 'Dictionary changed during iteration' when the error should not be raised

Co-authored-by: iscai-msft <iscai@microsoft.com>
Co-authored-by: Sam Bull <aa6bs0@sambull.org>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this issue Oct 12, 2021
-License-Update: Delete the description of the license and use the license address instead.
 You may obtain a copy of the License at
     http://www.apache.org/licenses/LICENSE-2.0

5.2.0 (2021-10-03)
=====================
Features
--------
- 1. Added support Python 3.10
  2. Started shipping platform-specific wheels with the ``musl`` tag targeting typical Alpine Linux runtimes.
  3. Started shipping platform-specific arm64 wheels for Apple Silicon. (`openembedded#629 <https://github.com/aio-libs/multidict/issues/629>`_)
Bugfixes
--------
- Fixed pure-python implementation that used to raise "Dictionary changed during iteration" error when iterated view (``.keys()``, ``.values()`` or ``.items()``) was created before the dictionary's content change. (`openembedded#620 <https://github.com/aio-libs/multidict/issues/620>`_)

Signed-off-by: Zang Ruochen <zangrc.fnst@fujitsu.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this issue Oct 12, 2021
-License-Update: Delete the description of the license and use the license address instead.
 You may obtain a copy of the License at
     http://www.apache.org/licenses/LICENSE-2.0

5.2.0 (2021-10-03)
=====================
Features
--------
- 1. Added support Python 3.10
  2. Started shipping platform-specific wheels with the ``musl`` tag targeting typical Alpine Linux runtimes.
  3. Started shipping platform-specific arm64 wheels for Apple Silicon. (`openembedded#629 <https://github.com/aio-libs/multidict/issues/629>`_)
Bugfixes
--------
- Fixed pure-python implementation that used to raise "Dictionary changed during iteration" error when iterated view (``.keys()``, ``.values()`` or ``.items()``) was created before the dictionary's content change. (`openembedded#620 <https://github.com/aio-libs/multidict/issues/620>`_)

Signed-off-by: Zang Ruochen <zangrc.fnst@fujitsu.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this issue Oct 12, 2021
-License-Update: Delete the description of the license and use the license address instead.
 You may obtain a copy of the License at
     http://www.apache.org/licenses/LICENSE-2.0

5.2.0 (2021-10-03)
=====================
Features
--------
- 1. Added support Python 3.10
  2. Started shipping platform-specific wheels with the ``musl`` tag targeting typical Alpine Linux runtimes.
  3. Started shipping platform-specific arm64 wheels for Apple Silicon. (`openembedded#629 <https://github.com/aio-libs/multidict/issues/629>`_)
Bugfixes
--------
- Fixed pure-python implementation that used to raise "Dictionary changed during iteration" error when iterated view (``.keys()``, ``.values()`` or ``.items()``) was created before the dictionary's content change. (`openembedded#620 <https://github.com/aio-libs/multidict/issues/620>`_)

Signed-off-by: Zang Ruochen <zangrc.fnst@fujitsu.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this issue Oct 13, 2021
-License-Update: Delete the description of the license and use the license address instead.
 You may obtain a copy of the License at
     http://www.apache.org/licenses/LICENSE-2.0

5.2.0 (2021-10-03)
=====================
Features
--------
- 1. Added support Python 3.10
  2. Started shipping platform-specific wheels with the ``musl`` tag targeting typical Alpine Linux runtimes.
  3. Started shipping platform-specific arm64 wheels for Apple Silicon. (`openembedded#629 <https://github.com/aio-libs/multidict/issues/629>`_)
Bugfixes
--------
- Fixed pure-python implementation that used to raise "Dictionary changed during iteration" error when iterated view (``.keys()``, ``.values()`` or ``.items()``) was created before the dictionary's content change. (`openembedded#620 <https://github.com/aio-libs/multidict/issues/620>`_)

Signed-off-by: Zang Ruochen <zangrc.fnst@fujitsu.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this issue Oct 13, 2021
-License-Update: Delete the description of the license and use the license address instead.
 You may obtain a copy of the License at
     http://www.apache.org/licenses/LICENSE-2.0

5.2.0 (2021-10-03)
=====================
Features
--------
- 1. Added support Python 3.10
  2. Started shipping platform-specific wheels with the ``musl`` tag targeting typical Alpine Linux runtimes.
  3. Started shipping platform-specific arm64 wheels for Apple Silicon. (`openembedded#629 <https://github.com/aio-libs/multidict/issues/629>`_)
Bugfixes
--------
- Fixed pure-python implementation that used to raise "Dictionary changed during iteration" error when iterated view (``.keys()``, ``.values()`` or ``.items()``) was created before the dictionary's content change. (`openembedded#620 <https://github.com/aio-libs/multidict/issues/620>`_)

Signed-off-by: Zang Ruochen <zangrc.fnst@fujitsu.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this issue Oct 13, 2021
-License-Update: Delete the description of the license and use the license address instead.
 You may obtain a copy of the License at
     http://www.apache.org/licenses/LICENSE-2.0

5.2.0 (2021-10-03)
=====================
Features
--------
- 1. Added support Python 3.10
  2. Started shipping platform-specific wheels with the ``musl`` tag targeting typical Alpine Linux runtimes.
  3. Started shipping platform-specific arm64 wheels for Apple Silicon. (`openembedded#629 <https://github.com/aio-libs/multidict/issues/629>`_)
Bugfixes
--------
- Fixed pure-python implementation that used to raise "Dictionary changed during iteration" error when iterated view (``.keys()``, ``.values()`` or ``.items()``) was created before the dictionary's content change. (`openembedded#620 <https://github.com/aio-libs/multidict/issues/620>`_)

Signed-off-by: Zang Ruochen <zangrc.fnst@fujitsu.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this issue Oct 13, 2021
-License-Update: Delete the description of the license and use the license address instead.
 You may obtain a copy of the License at
     http://www.apache.org/licenses/LICENSE-2.0

5.2.0 (2021-10-03)
=====================
Features
--------
- 1. Added support Python 3.10
  2. Started shipping platform-specific wheels with the ``musl`` tag targeting typical Alpine Linux runtimes.
  3. Started shipping platform-specific arm64 wheels for Apple Silicon. (`openembedded#629 <https://github.com/aio-libs/multidict/issues/629>`_)
Bugfixes
--------
- Fixed pure-python implementation that used to raise "Dictionary changed during iteration" error when iterated view (``.keys()``, ``.values()`` or ``.items()``) was created before the dictionary's content change. (`openembedded#620 <https://github.com/aio-libs/multidict/issues/620>`_)

Signed-off-by: Zang Ruochen <zangrc.fnst@fujitsu.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this issue Oct 14, 2021
-License-Update: Delete the description of the license and use the license address instead.
 You may obtain a copy of the License at
     http://www.apache.org/licenses/LICENSE-2.0

5.2.0 (2021-10-03)
=====================
Features
--------
- 1. Added support Python 3.10
  2. Started shipping platform-specific wheels with the ``musl`` tag targeting typical Alpine Linux runtimes.
  3. Started shipping platform-specific arm64 wheels for Apple Silicon. (`openembedded#629 <https://github.com/aio-libs/multidict/issues/629>`_)
Bugfixes
--------
- Fixed pure-python implementation that used to raise "Dictionary changed during iteration" error when iterated view (``.keys()``, ``.values()`` or ``.items()``) was created before the dictionary's content change. (`openembedded#620 <https://github.com/aio-libs/multidict/issues/620>`_)

Signed-off-by: Zang Ruochen <zangrc.fnst@fujitsu.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
halstead pushed a commit to openembedded/meta-openembedded that referenced this issue Oct 14, 2021
-License-Update: Delete the description of the license and use the license address instead.
 You may obtain a copy of the License at
     http://www.apache.org/licenses/LICENSE-2.0

5.2.0 (2021-10-03)
=====================
Features
--------
- 1. Added support Python 3.10
  2. Started shipping platform-specific wheels with the ``musl`` tag targeting typical Alpine Linux runtimes.
  3. Started shipping platform-specific arm64 wheels for Apple Silicon. (`#629 <https://github.com/aio-libs/multidict/issues/629>`_)
Bugfixes
--------
- Fixed pure-python implementation that used to raise "Dictionary changed during iteration" error when iterated view (``.keys()``, ``.values()`` or ``.items()``) was created before the dictionary's content change. (`#620 <https://github.com/aio-libs/multidict/issues/620>`_)

Signed-off-by: Zang Ruochen <zangrc.fnst@fujitsu.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Signed-off-by: Trevor Gamblin <trevor.gamblin@windriver.com>
daregit pushed a commit to daregit/yocto-combined that referenced this issue May 22, 2024
-License-Update: Delete the description of the license and use the license address instead.
 You may obtain a copy of the License at
     http://www.apache.org/licenses/LICENSE-2.0

5.2.0 (2021-10-03)
=====================
Features
--------
- 1. Added support Python 3.10
  2. Started shipping platform-specific wheels with the ``musl`` tag targeting typical Alpine Linux runtimes.
  3. Started shipping platform-specific arm64 wheels for Apple Silicon. (`#629 <https://github.com/aio-libs/multidict/issues/629>`_)
Bugfixes
--------
- Fixed pure-python implementation that used to raise "Dictionary changed during iteration" error when iterated view (``.keys()``, ``.values()`` or ``.items()``) was created before the dictionary's content change. (`#620 <https://github.com/aio-libs/multidict/issues/620>`_)

Signed-off-by: Zang Ruochen <zangrc.fnst@fujitsu.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Signed-off-by: Trevor Gamblin <trevor.gamblin@windriver.com>
daregit pushed a commit to daregit/yocto-combined that referenced this issue May 22, 2024
-License-Update: Delete the description of the license and use the license address instead.
 You may obtain a copy of the License at
     http://www.apache.org/licenses/LICENSE-2.0

5.2.0 (2021-10-03)
=====================
Features
--------
- 1. Added support Python 3.10
  2. Started shipping platform-specific wheels with the ``musl`` tag targeting typical Alpine Linux runtimes.
  3. Started shipping platform-specific arm64 wheels for Apple Silicon. (`#629 <https://github.com/aio-libs/multidict/issues/629>`_)
Bugfixes
--------
- Fixed pure-python implementation that used to raise "Dictionary changed during iteration" error when iterated view (``.keys()``, ``.values()`` or ``.items()``) was created before the dictionary's content change. (`#620 <https://github.com/aio-libs/multidict/issues/620>`_)

Signed-off-by: Zang Ruochen <zangrc.fnst@fujitsu.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Signed-off-by: Trevor Gamblin <trevor.gamblin@windriver.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants