Repository navigation
PyInterpreterState.config.int_max_str_digits Should Not Be Modified #98417
Description
Activity
- addedtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error3.12only security fixesonly security fixes
on Oct 18, 2022 I'm pretty sure I had a version of the code doing exactly this originally and was guided towards making it simpler by not duplicating the field between
interp->configandinterpitself. The resolved conversation #96944 (comment) alludes to that at least.Do we document the semantics of PyConfig being read-only anywhere?
ericsnowcurrently commented
on Oct 19, 2022 MemberAuthorMore actionsDo we document the semantics of PyConfig being read-only anywhere?
I agree about documenting the read-only-after-init constraint somewhere. I was thinking of a comment right above the
PyConfigdeclaration (in initconfig.h). However, you just ran into the mutability situation. What would have been the best way to alert you to the constraint?ericsnowcurrently commented
on Oct 19, 2022 MemberAuthorMore actionsTo be clear, I'm operating under the assumption that the cached
PyConfigcopy should be treated as read-only after init. That is what PEP 432 implies, it's what makes sense to me (config is config and state is state), and @zooba agreed in a private conversation. From #96944 (comment), it sounds like that might not be a universal opinion though.FWIW, PEP 587 doesn't say anything about read-only-after-init, but PEP 432 (from which 587 was partially derived) says:
These are snapshots of the initial configuration settings. They are not modified by the interpreter during runtimeThe C-API docs don't say anything about config mutability-after-init. That may be fine though, since the fact that we cache a copy of the original config on each interpreter state is an internal implementation detail.
ericsnowcurrently commented
on Oct 19, 2022 MemberAuthorMore actionsThe resolved conversation #96944 (comment) alludes to that at least.
That comment is more about preserving the original config value vs. updating during init, isn't it? The question of the source of the information during runtime (and where mutable state lives) seems like a separate one.
Updating the PyConfig struct definition comment to say it should be treated read-only after init makes sense to me.
I'd have seen that when making changes. I'm much less likely to go back and read PEPs as they're more often initial design ideas rather than policies and actual implementation.
Reacted by Eric SnowWhat are advantages of having a read-only PyConfig? For me, it's fine to modify it. But if its value is copied somewhere (ex: sys module), it's good to attempt to keep both consistent (whenever possible). Currently, _testinternalcapi.set_config() always copies PyConfig.module_search_paths to sys.path, erasing sys.path change done after early Python init, which makes the function annoying in practice and that's why I don't want to expose it in public.
My main motivation to design PyConfig was to put all inputs at the same place. My ideal would be that PyConfig would be the only input to initialize Python.
But once Python is initialized, for me, it's convenient to reuse this structure for a few variables. The only case I'm aware where the "initial" value matters is
sys.orig_argvthat I added recently (Python 3.10), since Python makes changes to buildsys.argv. Duplicating values between PyConfig and PyInterpreterState can lead to inconsistencies and wasting memory, no?We should've (and still can) allow PyConfig to be fully provided by the caller, rather than making them go via our memory management functions. This would be very convenient for some situations, but relies on us not modifying the structure at all.
Honestly, we shouldn't even copy it around the way we do. It should exist once, and if we need to refer back to it we can get to it through a pointer (bearing in mind that the host app owns it, not us, and so they might have modified it themselves). But really, it just looks like a set of parameters to
Py_Initialize(and friends), rather than a runtime structure. If it doesn't survive beyond the initialize call, we should be able to handle that.Reacted by Eric Snow- added a commit that references this issue
on Oct 20, 2022 Currently, PyConfig sits between two chairs: it's used to initialized Python (as expected), but it also stays during the whole Python lifecycle.
I hesitated to build a separated structure to store this configuration in PyInterpreterState: it would avoid storing things which become useless once Python is initialized. The problem is that it caused a lot of code to be duplicated, since we would have two similar but different structures. I chose to put PyConfig simply to keep the code as simple as possible. So yeah, we waste a few bytes in memory, but a single structure has to be handled in the C code.
Keeping PyConfig around also makes Py_NewInterpreter() simpler: it just copies the PyConfig from the current interpreter. By the way, _Py_NewInterpreterFromConfig() allows to run an interpreter with a different configuration.
Honestly, we shouldn't even copy it around the way we do.
Py_InitializeFromConfig() is a wild beast: it changes the memory allocator with implicit Python pre-initialization. If we do everything in a single PyConfig, the ownership of memory allocated on the stack is not well defined, and it's also unclear which memory allocator should be used to release the memory.
Moreover, PyConfig design is to collect all data to initialize Python without modifying Python, and then "write" the configuration: _PyConfig_Write(). This design also allows to override/replace PyConfig at runtime, currently implemented as _testinternalcapi.set_config(). This feature is also used by
Modules/getpath.pyif I followed correctly.Yeah, PyConfig API is weird and surprising, but they are reasons for its special design :-)
Keeping it around is great! I love it! Modifying it after we've initialised is the issue. It needs to be treated as read-only, because it's not our memory.
And really, getpath shouldn't modify it either, but should go directly to setting up
sysitself. We still keep the config around, but would need to re-run getpath if we're going to base new interpreters off the original config rather than the current setup of the interpreter creating them. However, we're now committed to modifying the original config for users (well, for our own tests), and I didn't want to block the getpath rewrite on a deprecation cycle.6 remaining items
If we're still treating config variables as live sources of truth, then this issue is still relevant.
If there's a value that can change at runtime (not just because the user/creator set it), then it should be directly in the interpreter state, not in the struct used to create it.
PyConfig_Setwas added to apply settings into a struct that may change over releases (as you well know, since it's your API 😉 ). It was never intended for changing runtime settings at runtime, because that's not what the config structs are for.This issue can be closed when the field for the runtime-modifiable setting is moved to the interpreter state directly, and that location is the one that gets updated by the
sysfunctions.Reacted by Gregory P. Smith- added 9 commits that reference this issue
on Jul 2, 2026 PR #152869 basically amounts to a one line change plus a unittest for this. do we want it? i do not consider this a bugfix as it is an observable behavior change so i wouldn't backport it past 3.15.
As I wrote previously,
PyConfig_Set()now modifies directlyPyInterpreterState.config. Example settingPyInterpreterState.config.cpu_count(int):$ python3.16 >>> import _testcapi, os >>> import _testcapi, os >>> os.cpu_count() 12 >>> _testcapi.config_set("cpu_count", 123) >>> os.cpu_count() 123
PyInterpreterState.configis no longer read-only. But I don't consider that it's a configuration thing, it's just part of the interpreter state. The fact that it usesPyConfigis just an implementation detail.I understand that some people are unhappy about that since
PyConfigwas designed first to configure the Python initialization. IMO it's ok to reusePyConfiginPyInterpreterState, because it's just convenient.If you consider that it's a design issue, someone should step in and propose a patch to remove
PyInterpreterState.config: move variables toPyInterpreterStatedirectly.Until we actually remove
PyInterpreterState.config, I would prefer to keepPyInterpreterState.configconsistent, even if the value is stored elsewhere. For example, even if we do have aPyInterpreterState.long_state.max_str_digitsvariable, there is still aPyInterpreterState.config.int_max_str_digitscopy. IMO it's better to keepPyInterpreterState.config.int_max_str_digitsupdated when the value is modified bysys.set_int_max_str_digits()orPyConfig_Set("int_max_str_digits", value).But I don't consider that it's a configuration thing, it's just part of the interpreter state.
Then your consideration is wrong.
There's value in keeping the original configuration around even as state changes, so "remove the field" is too drastic. Treating the field as a read-only copy of the configuration used to create the interpreter is the right way here.
So we consider that the design issue is exactly what this issue says, and the only pushback is that you've misunderstood the feature.
i do not consider this a bugfix as it is an observable behavior change so i wouldn't backport it past 3.15.
Bugfixes generally are, since bugs are observable behaviours that we don't want 😉 But I'm okay with not backporting further than 3.15, as long as we've got the main branch behaving properly so that we don't add more of these, and having it in 3.15 makes it a bit safer for embedders who might not expect their config structs to be changing.
PyConfigis for configuring the runtime during initialization, not for storing runtime state. Currently insys.set_int_max_str_digits()we are modifying the interpreter's PyConfig directly.That field should never be modified outside runtime init. Instead, the config value should be copied to a field on
PyInterpreterStateand that is what should be modified bysys(and used in longobject.c).In fact, this is mostly what we were doing until a couple weeks ago (and what we still are doing in 3.11 and earlier). This was changed on main (3.12) in gh-96944. The fix is relatively trivial.
CC @gpshead
Linked PRs