gh-151218: Fix data race in sys_set_flag for free-threading

[bhuvi27]

Fixes #151218

Concurrent calls to sys.set_int_max_str_digits() in free-threaded builds
could double-free the same sys.flags tuple item because sys_set_flag()
updated the slot without synchronization.

Protect sys.flags updates with a mutex in free-threaded builds, and hold
the same lock across flag and interpreter int_max_str_digits state updates
so sys.get_int_max_str_digits() stays consistent with sys.flags. Add a
concurrent stress regression test in test_sys.py.

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

[picnixz]

Please do not force push. To contribute cleanly, please read https://devguide.python.org/getting-started/pull-request-lifecycle/#pullrequest.

[da-woods]

This doesn't guard the read though. And the read through sys.flags is probably difficult to guard because it's an "immutable" object and so shouldn't be changing once it's visible

[bhuvi27]

This doesn't guard the read though. And the read through sys.flags is probably difficult to guard because it's an "immutable" object and so shouldn't be changing once it's visible

Good point — you're right that the read path through sys.flags isn't
guarded by the new mutex, so a reader can still observe the old slot
while a writer replaces and decref's it (use-after-free).

A few options I see:

1. Stop updating sys.flags.int_max_str_digits at runtime entirely —
   treat sys.flags as startup-only (its documented immutability),
   and let sys.get_int_max_str_digits() be the single source of truth
   for the current value. This removes the racy shared object.

2. Atomic store/load of the slot pointer plus deferred reclamation
   (QSBR) for the old value — keeps the current API but is more
   machinery.

3. Replace sys.flags with a fresh struct sequence on each update.

Option 1 looks cleanest and matches the "immutable" intent, but it's
a behaviour change. Which direction would you prefer? Happy to redo
the PR around option 1 if that's the right call.

[picnixz]

This conversation seems to go against our AI policy. Human interaction is necessary, don't let your agents in automated mode. Please read https://devguide.python.org/getting-started/ai-tools/#guidelines-for-using-ai-tools.

[bhuvi27]

This conversation seems to go against our AI policy. Human interaction is necessary, don't let your agents in automated mode. Please read https://devguide.python.org/getting-started/ai-tools/#guidelines-for-using-ai-tools.

Sorry — I used an AI assistant to draft PR replies and posted them
without reviewing properly (the first one was off-topic). I'll handle
discussion myself from here.

On the technical side: I understand the mutex doesn't protect reads
through sys.flags. I'm happy to rework the approach once there's
guidance on whether runtime updates to sys.flags should stop entirely.

[skirpichev]

I think that exposing int_max_str_digits in the sys.flags was a mistake. This is an immutable named tuple for command-line flags.

But sys.flags.int_max_str_digits does not fit in this picture: it's for runtime settings, which can be changed by sys.set_int_max_str_digits().

I think this bug should be fixed by changing meaning of the sys.flags.int_max_str_digits. It will keep immutable value, provided as command-line option. We have a different API to access runtime settings, i.e. sys.get_int_max_str_digits(). CC @gpshead

[gpshead]

PyConfig_Set() per PEP 741 in 3.14 is a public API that mutates various other sys.flags values at runtime. It is no longer correct to claim that it is a tuple of command line flags that never changes after the interpreter starts. I do not think changing what happens with int_max_str_digits would be right at this point.

[gpshead]

overall I don't think this PR is the right way to accomplish this. we don't need a static global mutex. use Py_BEGIN_CRITICAL_SECTION on the flags object instead.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[da-woods]

we don't need a static global mutex. use Py_BEGIN_CRITICAL_SECTION on the flags object instead.

I don't think that works because there's no thread-safety guards on any of the reads from the flags object (because it's a tuple-like object so considered immutable)

overall I don't think this PR is the right way to accomplish this

I do agree with this though

[skirpichev]

PyConfig_Set() per PEP 741 in 3.14 is a public API that mutates various other sys.flags values at runtime. It is no longer correct to claim that it is a tuple of command line flags that never changes after the interpreter starts. I do not think changing what happens with int_max_str_digits would be right at this point.

Indeed. Then struct sequence doesn't look to be the right data structure for this sys attribute anymore. Or, I suspect, we will reinvent the wheel (dict?).

I don't think that works because there's no thread-safety guards on any of the reads from the flags object (because it's a tuple-like object so considered immutable)

Maybe use a dict subclass (to make backward-compatible __getattr__) instead of the named tuple? Mutating of the struct sequence breaks the contract in docs:

void PyStructSequence_SetItem(PyObject *p, Py_ssize_t pos, PyObject *o)
[...]
Sets the field at index pos of the struct sequence p to value o. Like PyTuple_SET_ITEM(), this should only be used to fill in brand new instances.

CC @vstinner (as PEP author)

Edit: So far, option (3) from proposed above seems to be an alternative to this.

[gpshead]

its perfectly fine for it to be a structsequence. we should probably just do as tuples do and replace the instance with a new structsequence instance upon each mutation (a copy with one element changed) instead of trying to treat that as mutable or piling even more odd "some elements can change via getitem/getattr implementations returning new info" shapes on top of it.

the design mistake was ever using tuples for anything in the 1990s followed by recognizing that namedtuples weren't a solution to their ills in the 2000s. IIRC that's how structsequence came into being? an odd duck shaped thing that floats to preserve legacy API expectations while letting us add new .field_names.

the joys of the standard library and specifically old things like sys.