python / cpython

The Python programming language
https://www.python.org
Other
62.75k stars 30.07k forks source link

Weird `int.__str__` behaviour inside sub-interpreters #117482

Open mliezun opened 6 months ago

mliezun commented 6 months ago

Bug report

Bug description:

Hi Python maintainers!

I noticed something weird when using subinterpreters, while converting an Enum to string I get an unexpected result. This occurs in Python 3.12 and 3.13.

Here's an script to reproduce it:

import _xxsubinterpreters as interpreters

script = """from enum import _simple_enum, IntEnum

@_simple_enum(IntEnum)
class MyEnum:
    DATA = 1

print(str(MyEnum.DATA))
"""

exec(script)
# Output: 1

interp_id = interpreters.create(isolated=False)
interpreters.run_string(interp_id, script)
# Output: <MyEnum.DATA: 1>, Expected: 1

In all python versions previous to 3.12 this prints "1" two times, on newer versions I get <MyEnum.DATA: 1> when running inside a subinterpreter. For some reason, the __str__ function being used is different on new Python versions.

I also noticed that the function pointed by __str__ is different inside and outside the subinterpreter.

Outside:

...
print(MyEnum.DATA.__str__)
# Output: <method-wrapper '__repr__' of MyEnum object at 0x7f9a09a2e910>

Inside:

...
print(MyEnum.DATA.__str__)
# Output: <method-wrapper '__str__' of MyEnum object at 0x7f9a099a5e90>

NOTE: I'm creating subinterpreters passing the isolated=False flag, which uses the Legacy init config. I first noticed the error on MacOS, then reproduced using Docker with various Python versions.

I hope this is enough to get to the source of the issue.

Appreciate all your work and effort on building Python, thank you!

CPython versions tested on:

3.8, 3.9, 3.10, 3.11, 3.12, 3.13

Operating systems tested on:

Linux, macOS

Linked PRs

Eclips4 commented 6 months ago

Bisected to de64e7561680fdc5358001e9488091e75d4174a3

Elchinchel commented 6 months ago

I think i've got a shorter MRE, i believe this issue is not related to enums.

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>
Eclips4 commented 6 months ago

I think i've got a shorter MRE, i believe this issue is not related to enums.

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>

This sheds light on what is going on. Thanks, @Elchinchel!

mliezun commented 6 months ago

Awesome! Thanks for looking into this @Eclips4 and @Elchinchel.

By running the simplified script, I noticed that other base types have the same issue: bool, float, complex.

The commit https://github.com/python/cpython/commit/de64e7561680fdc5358001e9488091e75d4174a3 you mentioned seems to change how the MRO works for base-types. I think that's probably related to the problem.

ericsnowcurrently commented 2 months ago

FTR, https://github.com/python/cpython/pull/117660#issue-2232410446 provides further analysis.

AraHaan commented 2 months ago

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 commented 2 months ago

@markshannon pointed out an alternate solution to the one I merged: in add_operators(), if *ptr isn't NULL then check if it is the same as the corresponding tp slot as the base (tp_base). If it is then act as though it were NULL.

There are good reasons to keep (and build on) the changes we've made already, for 3.13+. That isn't so much the case for 3.12 though. Thus, I'll probably update 3.12 to take the simpler approach.

encukou commented 3 weeks ago

Note that current PRs are closed, the issue should stay open. @ericsnowcurrently said in #122865:

I'll circle back to it in the future, particularly after I've improved the tests so no corner cases slip through the cracks.