PyInterpreterState.config.int_max_str_digits Should Not Be Modified

[ericsnowcurrently]

PyConfig is for configuring the runtime during initialization, not for storing runtime state. Currently in sys.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 PyInterpreterState and that is what should be modified by sys (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

[gpshead]

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->config and interp itself. The resolved conversation #96944 (comment) alludes to that at least.

Do we document the semantics of PyConfig being read-only anywhere?

[ericsnowcurrently]

@gpshead

Do 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 PyConfig declaration (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]

To be clear, I'm operating under the assumption that the cached PyConfig copy 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 runtime

The 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]

The 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.

[gpshead]

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.

[vstinner]

What 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_argv that I added recently (Python 3.10), since Python makes changes to build sys.argv. Duplicating values between PyConfig and PyInterpreterState can lead to inconsistencies and wasting memory, no?

[zooba]

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.

[vstinner]

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.py if I followed correctly.

Yeah, PyConfig API is weird and surprising, but they are reasons for its special design :-)

[zooba]

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 sys itself. 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.

[vstinner]

It needs to be treated as read-only, because it's not our memory.

I don't get it: PyInterpreterState.config is a PyConfig structure, not a pointer to somewhere else. What do you call "our memory"?

And really, getpath shouldn't modify it either, but should go directly to setting up sys itself.

Well. Python has a long history and the initialization is complicated. You need many things to compute the path configuration, and the sys module is created "in the middle". There is always room for enhancement :-) On my side, I gave up at some point since I already sent like 1 or 2 years to deeply reorganize the code to make it "better" ;-)

---

I'm not strongly against duplicating variables to keep some PyConfig members as read-only, or even try to make the whole PyConfig structure read-only. I'm just trying to get the advantages compared to the current state where some members are copied (ex: to the sys module), and some PyConfig members are read and modified in-place.

Anyway, I'm glad that getpath is now implemented in pure Python. In that it was the main motivation for @ncoghlan and @ericsnowcurrently, at least at the beginning, to reorganize the Python initialization. My goal was just to fix my implementation of the -X utf8 command line option which has to parse the command line twice if the encoding change... but then I fall into a giant trap, and I rewrote most of the initialization code to try to make it more consistent :-)

Maybe PyConfig should really and only belong to Python initialization.

[zooba]

I see, it gets deep copied into every interpreter.

Even so, I don't think we should blur the line between interpreter state and interpreter config. Right now, that's what the issue is, and the fix is to move int_max_str_digits into interpreter state.

Whether we keep a copy of the config around is a separate discussion, probably to be driven by perf or memory needs rather than anything else.