GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-151218: Replace sys.flags in PyConfig_Set() by vstinner · Pull Request #151402 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Jun 12, 2026 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

PyConfig_Set() and sys.set_int_max_str_digits() now replace sys.flags, instead of modifying sys.flags in-place.

Modifying sys.flags in-place can lead to data races when multiple threads are reading or writing sys.flags in parallel.

Use _Py_atomic functions to get and set max_str_digits members.

  • Issue: Data race in sys_set_flag when sys.set_int_max_str_digits() is called concurrently with free-threaded build #151218

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
pythongh-151218: Replace sys.flags in PyConfig_Set()
425b022 
PyConfig_Set() and sys.set_int_max_str_digits() now replace
sys.flags, instead of modifying sys.flags in-place.

Modifying sys.flags in-place can lead to data races when multiple
threads are reading or writing sys.flags in parallel.

Use _Py_atomic functions to get and set max_str_digits members.
@vstinner
vstinner requested a review from ericsnowcurrently as a code owner June 12, 2026 13:40
@bedevere-app bedevere-app Bot added the awaiting core review label Jun 12, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Jun 12, 2026
Data race in sys_set_flag when sys.set_int_max_str_digits() is called concurrently with free-threaded build #151218
Closed
@vstinner vstinner mentioned this pull request Jun 12, 2026
gh-151218: Fix data race in sys_set_flag for free-threading #151220
Closed
@vstinner

vstinner commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

I built Python with ./configure --with-pydebug --disable-gil --with-thread-sanitizer CC=clang LD=clang and I ran the reproducer: TSan doesn't report data races anymore.

If we agree that this change is the right approach, I will add a non-regression test.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@skirpichev
skirpichev self-requested a review June 12, 2026 13:44
skirpichev
skirpichev reviewed Jun 12, 2026
View reviewed changes

@skirpichev skirpichev left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think it's a correct approach.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
vstinner added 3 commits June 15, 2026 17:04
@vstinner
Add unit tests
19e863c 
Add test_sys to "./python -m test --tsan" tests.
@vstinner
Move test to test_free_threading.test_sys
a66a331
@vstinner
Complete Changelog entry
4ca409d
@vstinner

vstinner commented Jun 15, 2026

Copy link
Copy Markdown
Member Author

I added a thread test to test_free_threading.test_sys which is run by ./python -m test --tsan.

I also added some simple tests on get_int_max_str_digits() / set_int_max_str_digits() (in test_sys).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented Jun 15, 2026

Copy link
Copy Markdown
Member Author

@picnixz @gpshead: Would you mind to review this fix?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner vstinner added needs backport to 3.15 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Jun 15, 2026
@vstinner

vstinner commented Jun 15, 2026

Copy link
Copy Markdown
Member Author

Python 3.13 is not affected: sys.set_int_max_str_digits() only sets tstate->interp->long_state.max_str_digits, it doesn't update sys.flags. And PyConfig_Set() was only added to Python 3.14.

I modified sys.set_int_max_str_digits() in Python 3.14 when I implemented PEP 741 (PyConfig_Set()).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
picnixz
picnixz reviewed Jun 15, 2026
View reviewed changes
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2026-06-12-15-30-25.gh-issue-151218.5M_nv8.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/sysmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/sysmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/sysmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
vstinner added 2 commits June 16, 2026 13:35
@vstinner
Address picnixz's review
f114294
@vstinner
Add get_int_max_str_digits() to the test
e5280f1
@vstinner
vstinner requested review from FFY00 and ZeroIntensity as code owners June 16, 2026 11:42
@read-the-docs-community

read-the-docs-community Bot commented Jun 16, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #33163314 | 📁 Comparing 44bc1b2 against main (8646385)

  🔍 Preview build  

2 files changed
± c-api/init_config.html
± whatsnew/changelog.html
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
picnixz
picnixz reviewed Jun 16, 2026
View reviewed changes
Comment thread Doc/c-api/init_config.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
Move versionadded
770d385
picnixz
picnixz reviewed Jun 16, 2026
View reviewed changes
Comment thread Python/sysmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
vstinner and others added 2 commits June 16, 2026 13:56
@vstinner
Use also _Py_ID() in _PySys_UpdateConfig()
df57cf2 
Update also outdated comment.
@vstinner @picnixz
Update Python/sysmodule.c
44bc1b2 
Co-authored-by: Bénédikt Tran <[email protected]>
@vstinner

vstinner commented Jun 16, 2026

Copy link
Copy Markdown
Member Author

@picnixz: I addressed your review. Please review the updated PR.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
picnixz
picnixz approved these changes Jun 16, 2026
View reviewed changes

@picnixz picnixz left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM. Btw, I tried to delete sys.flags and it made the REPL "crash" (well just exit with an exception). I wonder whether sys.flags should be made non-deletable as well.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/sysmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/sysmodule.c
sys_set_flag(flags, pos, value);
Py_DECREF(flags);
return 0;
new_flags = PyStructSequence_New(&FlagsType);

@picnixz picnixz Jun 16, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

All this dance makes me wonder whether we need a PyStructSequence_Copy. Would be nicer to just copy the structure and alter one field.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@vstinner vstinner Jun 16, 2026

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I searched for PyStructSequence_New() and the only other place which creates a copy of an existing structseq is structseq_replace(): structseq.__replace__() method. I'm not sure that it's worth it to write a generic PyStructSequence_Copy() function.

Example using structseq.__replace__() method:

$ python
>>> import sys, copy
>>> copy.replace(sys.flags, verbose=3)
sys.flags(..., verbose=3, ...)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot removed the awaiting core review label Jun 16, 2026
@bedevere-app bedevere-app Bot added the awaiting merge label Jun 16, 2026
@vstinner
PyStructSequence_GET_ITEM() cannot fail
1843290
@vstinner

vstinner commented Jun 16, 2026

Copy link
Copy Markdown
Member Author

Btw, I tried to delete sys.flags and it made the REPL "crash" (well just exit with an exception). I wonder whether sys.flags should be made non-deletable as well.

In https://peps.python.org/pep-0726/ discussion, I mentioned that protecting sys attributes would be a good motivation to implement module __setattr__() and __delattr__(). Sadly, the SC decided to reject this PEP.

It's not easy to modify how module attributes are set and deleted in a C extension.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
vstinner enabled auto-merge (squash) June 16, 2026 16:03
@vstinner
vstinner merged commit b16d23f into python:main Jun 16, 2026
54 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
vstinner deleted the replace_sys_flags branch June 16, 2026 16:17
@bedevere-app bedevere-app Bot removed the awaiting merge label Jun 16, 2026
@miss-islington-app

miss-islington-app Bot commented Jun 16, 2026

Copy link
Copy Markdown

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14, 3.15.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app

miss-islington-app Bot commented Jun 16, 2026

Copy link
Copy Markdown

Sorry, @vstinner, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker b16d23fc9fe9cb72fa15c8a3036753e5437b5b8c 3.14
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app miss-islington-app Bot assigned vstinner Jun 16, 2026
@bedevere-app

bedevere-app Bot commented Jun 16, 2026

Copy link
Copy Markdown

GH-151552 is a backport of this pull request to the 3.15 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.15 bugs and security fixes label Jun 16, 2026
@bedevere-app

bedevere-app Bot commented Jun 16, 2026

Copy link
Copy Markdown

GH-151553 is a backport of this pull request to the 3.14 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Jun 16, 2026
vstinner added a commit that referenced this pull request Jun 16, 2026
@miss-islington @vstinner @picnixz
[3.15] gh-151218: Replace sys.flags in PyConfig_Set() (GH-151402) (#1…
ff6e973 
…51552)

gh-151218: Replace sys.flags in PyConfig_Set() (GH-151402)

PyConfig_Set() and sys.set_int_max_str_digits() now replace
sys.flags (create a new object), instead of modifying sys.flags in-place.

Modifying sys.flags in-place can lead to data races when multiple
threads are reading or writing sys.flags in parallel.

Use _Py_atomic functions to get and set max_str_digits members.
(cherry picked from commit b16d23f)

Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
vstinner added a commit that referenced this pull request Jun 16, 2026
@vstinner @picnixz
[3.14] gh-151218: Replace sys.flags in PyConfig_Set() (#151402) (#151553
60132db 
)

gh-151218: Replace sys.flags in PyConfig_Set() (#151402)

PyConfig_Set() and sys.set_int_max_str_digits() now replace
sys.flags (create a new object), instead of modifying sys.flags in-place.

Modifying sys.flags in-place can lead to data races when multiple
threads are reading or writing sys.flags in parallel.

Use _Py_atomic functions to get and set max_str_digits members.


(cherry picked from commit b16d23f)

Co-authored-by: Bénédikt Tran <[email protected]>
philthompson10 pushed a commit to philthompson10/cpython that referenced this pull request Jun 17, 2026
@vstinner @picnixz @philthompson10
pythongh-151218: Replace sys.flags in PyConfig_Set() (python#151402)
0171ccc 
PyConfig_Set() and sys.set_int_max_str_digits() now replace
sys.flags (create a new object), instead of modifying sys.flags in-place.

Modifying sys.flags in-place can lead to data races when multiple
threads are reading or writing sys.flags in parallel.

Use _Py_atomic functions to get and set max_str_digits members.

Co-authored-by: Bénédikt Tran <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@skirpichev skirpichev skirpichev left review comments
@picnixz picnixz picnixz approved these changes
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@ZeroIntensity ZeroIntensity Awaiting requested review from ZeroIntensity ZeroIntensity is a code owner
@FFY00 FFY00 Awaiting requested review from FFY00 FFY00 is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@vstinner vstinner

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@vstinner @skirpichev @picnixz
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.