| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
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.
|
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. |
Sorry, something went wrong.
There was a problem hiding this comment.
I think it's a correct approach.
Sorry, something went wrong.
Add test_sys to "./python -m test --tsan" tests.
|
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). |
Sorry, something went wrong.
Sorry, something went wrong.
|
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()). |
Sorry, something went wrong.
Documentation build overview2 files changed ± c-api/init_config.html ± whatsnew/changelog.html |
Sorry, something went wrong.
Update also outdated comment.
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
@picnixz: I addressed your review. Please review the updated PR. |
Sorry, something went wrong.
There was a problem hiding this comment.
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.
| sys_set_flag(flags, pos, value); | ||
| Py_DECREF(flags); | ||
| return 0; | ||
| new_flags = PyStructSequence_New(&FlagsType); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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. |
Sorry, something went wrong.
|
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14, 3.15. |
Sorry, something went wrong.
|
Sorry, @vstinner, I could not cleanly backport this to 3.14 due to a conflict. cherry_picker b16d23fc9fe9cb72fa15c8a3036753e5437b5b8c 3.14 |
Sorry, something went wrong.
|
GH-151552 is a backport of this pull request to the 3.15 branch. |
Sorry, something went wrong.
|
GH-151553 is a backport of this pull request to the 3.14 branch. |
Sorry, something went wrong.
…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 <vstinner@python.org> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
) 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 <10796600+picnixz@users.noreply.github.com>
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 <10796600+picnixz@users.noreply.github.com>
| Back | FazBrowse Home | New Git URL |
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.