gh-117482: Cleaning up tp_dict for static builtin types in subinterpreters

[mliezun]

Solves #117482

Problem description

When initializing base types in a subinterpreters extra items are added to tp_dict. For example, in the linked issue we use the following script to identify the problem:

import _xxsubinterpreters as interpreters

script = """print(int.__str__)"""


exec(script)
# Output: <slot wrapper '__str__' of 'object' objects>

interp_id = interpreters.create()
interpreters.run_string(interp_id, script)
# Output: <slot wrapper '__str__' of 'int' objects>

While running on a subinterpreter int.__str__ points to a slot in int itself instead of pointing to the one defined in object as it should be.

Tracking down the root cause

As can be seen in #117482 (comment), the problem was introduced in commit de64e75.

By further debugging the specific parts modified in that commit I found that the problem lies in the type_ready function.

In previous versions of Python tp_dict was shared between subinterpreters. Now each interpreter has their own copy for static builtin types.

Let's take int.__str__ as an example to see what's happening:

1. The type_ready function gets executed as part of the main interpreter initialization process.
2. The function starts "readying" the int type: tp_dict gets filled by type_ready_fill_dict, one of the thing this function does is lookup which slots are not empty and add them to the dict, for int the tp_str slot is empty.
3. After that, type_ready_inherit get called, and copies the tp_str slot from object to int.
4. The init process for the main interpreter finishes and we're ready to start with the second interpreter.
5. In this case, the tp_str slot is not empty for int, it was filled by step 3. So it gets added to the dict by type_ready_fill_dict.
6. The program continues and we see different behavior depending on which interpreter we run.

The problem here is that the code expects type_ready_fill_dict to be called before type_ready_inherit. Which is true for the main interpreter but not for the subinterpreter. Also, this is caused because all the slots in int are shared except for tp_dict which is stored in each interpreter.

Tracking down all affected types

I ran a little script to track down all affected types and the differences in their base dicts:

import sys
import subprocess

type_names = [b for b in dir(__builtins__) if getattr(eval(b), '__dict__', None)]
for tp in type_names:
    script = f"print(set({tp}.__dict__.keys()))"

    process = subprocess.Popen([sys.executable, '-c', script], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
    out, _ = process.communicate()
    original_keys = eval(out)

    script_interpreters = f"""
import _xxsubinterpreters as interpreters

interp_id = interpreters.create()
interpreters.run_string(interp_id, '{script}')
interpreters.destroy(interp_id)
    """
    process = subprocess.Popen([sys.executable, '-c', script_interpreters], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
    out, _ = process.communicate()
    new_keys = eval(out)


    print(f"{tp}:", (original_keys-new_keys), (new_keys-original_keys))

This was used to compare outputs with previous python versions (<3.12) and to check if the proposed solution worked correctly.

Proposed Solution

The current proposed solution is to add a function called cleanup_tp_dict that checks if we're running in a subinterpreter, then identifies what extra keys were added to the static type tp_dict and removes them.

This function is called at the end of type_ready_fill_dict.

Alternative solutions

• Introduce new sentinel values similar PyObject_HashNotImplemented to indicate that certain slots should not be filled, like tp_str for int.
• Only call type_ready_fill_dict on the main interpreter and just make a copy of the original one in subinterpreters.

Conclusion

The proposed solution feels quite hacky, but gets the work done. This is my first contribution to CPython and I don't have much experience with it so there surely might be a better way to do it, there's probably some internal knowledge that I'm missing.

I'm happy to work on a different approach if pointed in the right direction.

Thank you for your time and all your effort building Python!

• Issue: Weird int.__str__ behaviour inside sub-interpreters #117482

[cpython-cla-bot]

All commit authors signed the Contributor License Agreement.

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[ericsnowcurrently]

Thanks for the detailed analysis and solution! It makes sense. I'll take a closer look at the PR in the next day or two.

[mliezun]

Thank you for taking the time to look into this!

[mliezun]

Hi @ericsnowcurrently, just checking in on the PR. No worries if you're busy, just making sure it didn’t slip through the cracks!

[ericsnowcurrently]

Yeah, sorry it's taking so long to get to this.

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[kunkku]

This bug has a severe impact on WSGI applications running on mod_wsgi, which uses subinterpreters to isolate the applications.

[ericsnowcurrently]

I've left a few recommendations.

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

[ericsnowcurrently]

Is the main issue the "tp" slots getting set during type_ready() for the main interpreter? If so, shouldn't we be able to constrain the changes to add_operators() instead of type_ready_fill_dict()?

[mliezun]

Is the main issue the "tp" slots getting set during type_ready() for the main interpreter? If so, shouldn't we be able to constrain the changes to add_operators() instead of type_ready_fill_dict()?

Moved cleanup to add_operators() in commit 0b12944.

[ericsnowcurrently]

FYI, I tried out an alternative approach that is a bit more direct: gh-121602.

I think the approach I took is more focused and probably less fragile. It also addresses a potential problem related to reinitializing the runtime.

[mliezun]

FYI, I tried out an alternative approach that is a bit more direct: gh-121602.

I think the approach I took is more focused and probably less fragile. It also addresses a potential problem related to reinitializing the runtime.

Looks great!

[mliezun]

Closing in favor of #121602

[ericsnowcurrently]

Thanks for working on this, @mliezun! The analysis you did made it a lot easier to find a solution for this. While we didn't end up using your fix, it was helpful for identifying what needed to happen.

[mliezun]

Thanks for working on this, @mliezun! The analysis you did made it a lot easier to find a solution for this. While we didn't end up using your fix, it was helpful for identifying what needed to happen.

Thank you for looking into it!

[AraHaan]

I think an alternative option is for tp_dict to be computed once and for those members of the type objects to remain untouched between main and sub-interpreters. As in once the main interpreter computes it and the inherited tp slots, it then uses those slots in any sub-interpreter and permanently solves this issue I think (I think this way it provides a performance boost for free in sub-interpreters on top of fixing this issue). Basically the entire PyTypeObject structure after the main interpreter properly loads and prepares it, the sub interpreters would be able to make use of the same pointers the main one used (by first copying the pointers to a new one for the subinterpreter using memcpy).

[ericsnowcurrently]

@AraHaan, that's worth looking into. It would be best to post your comment to gh-117482.

[AraHaan]

@AraHaan, that's worth looking into. It would be best to post your comment to gh-117482.

Alright done that just now. #117482 (comment)