Skip to content

gh-158254: Detect keys changing during reverse dict iteration - #158255

Merged
ZeroIntensity merged 1 commit into
python:mainfrom
jab:reversed-dict-keys-changed
Sep 27, 2026
Merged

ZeroIntensity merged 1 commit into
python:mainfrom
jab:reversed-dict-keys-changed

Conversation

@jab

@jab jab commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Reverse dict iterators only checked that the dict's size was unchanged. So after clear() and a refill to the same size that leaves live entries below the iterator's position, they kept yielding entries for the new keys. They could yield more items than the dict had when the iterator was created, and __length_hint__() then wrapped around to 2**64-1.

This adds the check the forward iterators already have (e.g. dictiter_iternextkey_lock_held): finding an entry after the expected number of items raises RuntimeError: dictionary keys changed during iteration.

The new test fails without the change ("RuntimeError not raised") and passes with it. The full test suite passes on a macOS arm64 debug build (not free-threaded).

This change was written with the help of Claude Code, an AI assistant. I have reviewed it.

🤖 Generated with Claude Code

A reverse dict iterator only checked that the dict's size was unchanged,
so after the keys were replaced with the same number of keys it kept
yielding entries for the new keys. It could yield more items than the
dict had when it was created, and __length_hint__() then wrapped around.

Add the check the forward iterators already have: finding an entry after
the expected number of items raises "dictionary keys changed during
iteration".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ZeroIntensity ZeroIntensity added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels Sep 27, 2026

@ZeroIntensity ZeroIntensity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ZeroIntensity
ZeroIntensity merged commit 042a7c1 into python:main Sep 27, 2026
67 checks passed
@miss-islington-app

Copy link
Copy Markdown

Thanks @jab for the PR, and @ZeroIntensity for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14, 3.15.
🐍🍒⛏🤖

@bedevere-app

bedevere-app Bot commented Sep 27, 2026

Copy link
Copy Markdown

GH-158267 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Sep 27, 2026
@bedevere-app

bedevere-app Bot commented Sep 27, 2026

Copy link
Copy Markdown

GH-158268 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Sep 27, 2026
@bedevere-app

bedevere-app Bot commented Sep 27, 2026

Copy link
Copy Markdown

GH-158269 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Sep 27, 2026
ZeroIntensity pushed a commit that referenced this pull request Sep 27, 2026
…H-158255) (GH-158269)

A reverse dict iterator only checked that the dict's size was unchanged,
so after the keys were replaced with the same number of keys it kept
yielding entries for the new keys. It could yield more items than the
dict had when it was created, and __length_hint__() then wrapped around.

Add the check the forward iterators already have: finding an entry after
the expected number of items raises "dictionary keys changed during
iteration".
(cherry picked from commit 042a7c1)

Co-authored-by: Joshua Bronson <jabronson@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
ZeroIntensity pushed a commit that referenced this pull request Sep 27, 2026
…H-158255) (GH-158268)

A reverse dict iterator only checked that the dict's size was unchanged,
so after the keys were replaced with the same number of keys it kept
yielding entries for the new keys. It could yield more items than the
dict had when it was created, and __length_hint__() then wrapped around.

Add the check the forward iterators already have: finding an entry after
the expected number of items raises "dictionary keys changed during
iteration".
(cherry picked from commit 042a7c1)

Co-authored-by: Joshua Bronson <jabronson@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@bedevere-bot

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Windows Server 2022 NoGIL 3.14 (tier-1) has failed when building commit f085176.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1717/builds/1783) and take a look at the build logs.
  4. Check if the failure is related to this commit (f085176) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1717/builds/1783

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "C:\bbarea\3.14.itamaro-win64-srv-22-aws.x64.nogil\build\Lib\threading.py", line 1082, in _bootstrap_inner
    self._context.run(self.run)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^
  File "C:\bbarea\3.14.itamaro-win64-srv-22-aws.x64.nogil\build\Lib\threading.py", line 1024, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\bbarea\3.14.itamaro-win64-srv-22-aws.x64.nogil\build\Lib\multiprocessing\pool.py", line 595, in _handle_results
    cache[job]._set(i, obj)
    ~~~~~~~~~~~~~~~^^^^^^^^
  File "C:\bbarea\3.14.itamaro-win64-srv-22-aws.x64.nogil\build\Lib\multiprocessing\pool.py", line 783, in _set
    del self._cache[self._job]
        ~~~~~~~~~~~^^^^^^^^^^^
  File "C:\bbarea\3.14.itamaro-win64-srv-22-aws.x64.nogil\build\Lib\multiprocessing\pool.py", line 171, in __delitem__
    self.notifier.put(None)
    ~~~~~~~~~~~~~~~~~^^^^^^
  File "C:\bbarea\3.14.itamaro-win64-srv-22-aws.x64.nogil\build\Lib\multiprocessing\queues.py", line 394, in put
    self._writer.send_bytes(obj)
    ~~~~~~~~~~~~~~~~~~~~~~~^^^^^
  File "C:\bbarea\3.14.itamaro-win64-srv-22-aws.x64.nogil\build\Lib\multiprocessing\connection.py", line 210, in send_bytes
    self._send_bytes(m[offset:offset + size])
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\bbarea\3.14.itamaro-win64-srv-22-aws.x64.nogil\build\Lib\multiprocessing\connection.py", line 297, in _send_bytes
    raise ValueError("concurrent send_bytes() calls "
                     "are not supported")
ValueError: concurrent send_bytes() calls are not supported
k

@jab
jab deleted the reversed-dict-keys-changed branch September 27, 2026 12:16
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 this pull request may close these issues.

4 participants