Skip to content

gh-153908: Fix datarace in itertools.count().repr()#153917

Merged
ZeroIntensity merged 4 commits into
python:mainfrom
johng:tsan-0006-itertools-count-repr-ft
Jul 18, 2026
Merged

gh-153908: Fix datarace in itertools.count().repr()#153917
ZeroIntensity merged 4 commits into
python:mainfrom
johng:tsan-0006-itertools-count-repr-ft

Conversation

@johng

@johng johng commented Jul 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Uses atomic load to ensure protected read to internal cnt variable that can be incremented on next()

When using repro_tsan_0006_count.py as a temporary script to expose the issue we not longer have TSAN flags

➜  cpython git:(tsan-0006-itertools-count-repr-ft) ✗ PYTHON_GIL=0 ./python.exe repro_tsan_0006_count.py
done (no race detected)

@johng
Fix datarace in itertools.count().repr()
c35185b 
Uses atomic load to ensure protected read to internal
`cnt` variable that can be incremented on `next()`
@johng
johng requested a review from rhettinger as a code owner July 18, 2026 10:43
@bedevere-app

bedevere-app Bot commented Jul 18, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@python-cla-bot

python-cla-bot Bot commented Jul 18, 2026 •
edited
Loading

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app bedevere-app Bot mentioned this pull request Jul 18, 2026
Closed
@picnixz picnixz changed the title [GH-153908] Fix datarace in itertools.count().repr() gh-153908: Fix datarace in itertools.count().repr() Jul 18, 2026
johng
johng commented Jul 18, 2026
View reviewed changes
Comment thread Modules/itertoolsmodule.c Outdated
Comment on lines +3678 to +3679
if (lz->long_cnt == NULL) {
/* Match the atomic access in count_next(); a concurrent next() may

@johng johng Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment likely too verbose?

ByteFlowing1337
ByteFlowing1337 reviewed Jul 18, 2026
View reviewed changes

@ByteFlowing1337 ByteFlowing1337 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug fixes deserve a news entry, please add one :)

Comment thread Modules/itertoolsmodule.c Outdated
if (lz->long_cnt == NULL) {
/* Match the atomic access in count_next(); a concurrent next() may
be advancing lz->cnt via _Py_atomic_compare_exchange_ssize(). */
Py_ssize_t cnt = _Py_atomic_load_ssize_relaxed(&lz->cnt);

@ByteFlowing1337 ByteFlowing1337 Jul 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should use FT_ATOMIC_LOAD_SSIZE_RELAXED macro to avoid the possible perf overhead under the default build.

@bedevere-app

bedevere-app Bot commented Jul 18, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@johng
johng requested a review from ByteFlowing1337 July 18, 2026 11:17
ByteFlowing1337
ByteFlowing1337 reviewed Jul 18, 2026
View reviewed changes

@ByteFlowing1337 ByteFlowing1337 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The news entry should be in the Library dir, not Core and Builtins.

Comment thread Misc/NEWS.d/next/Core_and_Builtins/2026-07-18-11-16-41.gh-issue-153908.82FiGk.rst Outdated
@@ -0,0 +1 @@
Fix data race in repr in itertools.counter() for freethreading builds

@ByteFlowing1337 ByteFlowing1337 Jul 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
Fix data race in repr in itertools.counter() for freethreading builds
Fix data race when calling :func:`repr` on :class:`itertools.count` under the :term:`free-threaded build`.

@johng
johng requested a review from ByteFlowing1337 July 18, 2026 12:13
@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 and removed needs backport to 3.13 bugs and security fixes labels Jul 18, 2026
@ZeroIntensity
ZeroIntensity merged commit 5200f11 into python:main Jul 18, 2026
66 checks passed
@miss-islington-app

miss-islington-app Bot commented Jul 18, 2026

Copy link
Copy Markdown

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

@bedevere-app

bedevere-app Bot commented Jul 18, 2026

Copy link
Copy Markdown

GH-153951 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 Jul 18, 2026
@bedevere-app

bedevere-app Bot commented Jul 18, 2026

Copy link
Copy Markdown

GH-153952 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 Jul 18, 2026
ZeroIntensity pushed a commit that referenced this pull request Jul 18, 2026
@miss-islington @johng
[3.14] gh-153908: Fix data race in itertools.count.__repr__ (GH-153917
950e7fb 
) (GH-153952)

(cherry picked from commit 5200f11)

Co-authored-by: John <me@joh.ng>
ZeroIntensity pushed a commit that referenced this pull request Jul 18, 2026
@miss-islington @johng
[3.15] gh-153908: Fix data race in itertools.count.__repr__ (GH-153917
536d18b 
) (GH-153951)

(cherry picked from commit 5200f11)

Co-authored-by: John <me@joh.ng>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ZeroIntensity ZeroIntensity ZeroIntensity approved these changes
@rhettinger rhettinger Awaiting requested review from rhettinger rhettinger is a code owner
@ByteFlowing1337 ByteFlowing1337 Awaiting requested review from ByteFlowing1337

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@johng @ZeroIntensity @ByteFlowing1337