[subinterpreter] _PyUnicode_EqualToASCIIId() issue with subinterpreters

[vstinner]

BPO	46006
Nosy	@srittau, @vstinner, @encukou, @methane, @markshannon, @ericsnowcurrently, @serhiy-storchaka, @ndjensen, @hroncok, @corona10, @pablogsal, @erlend-aasland, @diabonas
PRs	bpo-46006: Fix _PyUnicode_EqualToASCIIId() for interned strings #30123 bpo-46006: Move the interned strings and identifiers to _PyRuntimeState. #30131 bpo-46006: Revert "bpo-40521: Per-interpreter interned strings (GH-20085)" #30422 [3.10] bpo-46006: Revert "bpo-40521: Per-interpreter interned strings (GH-20085)" (GH-30422) #30425 [3.10] bpo-46006: Revert "bpo-40521: Per-interpreter interned strings (GH-20085)" (GH-30422) #30433
Files	reproducer.c cmp_interned_strings.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2022-01-08.12:07:24.640>
created_at = <Date 2021-12-07.17:12:26.455>
labels = ['expert-subinterpreters', 'expert-C-API', '3.10', '3.11']
title = '[subinterpreter] _PyUnicode_EqualToASCIIId() issue with subinterpreters'
updated_at = <Date 2022-01-08.12:07:24.640>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2022-01-08.12:07:24.640>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2022-01-08.12:07:24.640>
closer = 'vstinner'
components = ['C API', 'Subinterpreters']
creation = <Date 2021-12-07.17:12:26.455>
creator = 'vstinner'
dependencies = []
files = ['50482', '50485']
hgrepos = []
issue_num = 46006
keywords = ['patch', '3.10regression']
message_count = 29.0
messages = ['407950', '407951', '407952', '407953', '407956', '408002', '408012', '408069', '408085', '408125', '408201', '408202', '408605', '408610', '408611', '408614', '408620', '408639', '408640', '408666', '408667', '409242', '409767', '409787', '409788', '409797', '409818', '409855', '409861']
nosy_count = 14.0
nosy_names = ['srittau', 'vstinner', 'craigh', 'petr.viktorin', 'methane', 'Mark.Shannon', 'eric.snow', 'serhiy.storchaka', 'ndjensen', 'hroncok', 'corona10', 'pablogsal', 'erlendaasland', 'diabonas']
pr_nums = ['30123', '30131', '30422', '30425', '30433']
priority = None
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue46006'
versions = ['Python 3.10', 'Python 3.11']

[vstinner]

_PyUnicode_EqualToASCIIId() seems to be incompatible with subinterpreter: it makes the assumption that if direct pointer comparison fails and the string is interned, the two strings are not equal.

--

super_init_without_args() of Objects/typeobject.c calls _PyUnicode_EqualToASCIIId(name, &PyId___class__) to test if the Unicode string 'name' is equal to "__class__".

int
_PyUnicode_EqualToASCIIId(PyObject *left, _Py_Identifier *right)
{
    right_uni = _PyUnicode_FromId(right);
    ...
    if (left == right_uni)
        return 1;
    if (PyUnicode_CHECK_INTERNED(left))
        return 0;
    ...
    return unicode_compare_eq(left, right_uni);
}

_PyUnicode_EqualToASCIIId() makes the assumption that left and right are not equal if left and _PyUnicode_FromId(right) pointers are not equal and left is an interned string.

In the reproducer, left object is abc.ABCMeta.__new__.__code__.co_freevars[0].

Depending on how the stdlib abc.py file was loaded (in the main interpreter and in the subinterpreter), __code__.co_freevars[0] may or may not be an interned string.

If __code__.co_freevars[0] is an interned string, _PyUnicode_EqualToASCIIId() fails in a subinterpreter if the direct pointer comparison fails (if left and right_uni pointers are not equal).

--

Reproducer from: ninia/jep#358 (comment)

• Build Python 3.10 with "./configure --enable-shared --prefix /opt/py310" and install it.
• Download attached reproducer.c.
• Build the reproducer with:
  gcc -o reproducer reproducer.c $(/opt/py310/bin/python3.10-config --embed --cflags --ldflags)
• Remove all stdlib .pyc files:
  find /opt/py310 -type d -name __pycache__|xargs rm -rf
• Run the reproducer with:
  LD_LIBRARY_PATH=/opt/py310/lib ./reproducer

Output:
---

Before creating sub interpreter
Traceback (most recent call last):
  File "/opt/py310/lib/python3.10/io.py", line 52, in <module>
  File "/opt/py310/lib/python3.10/abc.py", line 184, in <module>
  File "/opt/py310/lib/python3.10/abc.py", line 106, in __new__
RuntimeError: super(): __class__ cell not found
Fatal Python error: _PyThreadState_Delete: tstate 0x7f9f2001c710 is still current
Python runtime state: initialized

Current thread 0x00007f9f27c99640 (most recent call first):
<no Python frame>
Abandon (core dumped)
---

py-bt command in gdb:
---

(gdb) py-bt
Traceback (most recent call first):
  File "/opt/py310/lib/python3.10/abc.py", line 106, in __new__
    cls = super().__new__(mcls, name, bases, namespace, **kwargs)
  <built-in method __build_class__ of module object at remote 0x7fffea0b4cc0>
  File "/opt/py310/lib/python3.10/abc.py", line 184, in <module>
    class ABC(metaclass=ABCMeta):
  <built-in method exec of module object at remote 0x7fffea0b4cc0>
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "<frozen importlib._bootstrap_external>", line 883, in exec_module
  File "<frozen importlib._bootstrap>", line 688, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 1006, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "/opt/py310/lib/python3.10/io.py", line 52, in <module>
    import abc
  <built-in method exec of module object at remote 0x7fffea0b4cc0>
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "<frozen importlib._bootstrap_external>", line 883, in exec_module
  File "<frozen importlib._bootstrap>", line 688, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 1006, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  <built-in method __import__ of module object at remote 0x7fffea0b4cc0>

---

[vstinner]

In Python 3.9, the code works because the _Py_IDENTIFIER() API shares Python Unicode objects between all interpreters.

_PyUnicode_FromId() was modified to be per-interpreter in bpo-39465 by:

New changeset ba3d67c by Victor Stinner in branch 'master':
bpo-39465: Fix _PyUnicode_FromId() for subinterpreters (GH-20058)
ba3d67c

[vstinner]

Serhiy: Do you recall the idea of the PyUnicode_CHECK_INTERNED() optimization?

The PyUnicode_CHECK_INTERNED() test is as old as the _PyUnicode_EqualToASCIIId() function.

commit f5894dd
Author: Serhiy Storchaka <storchaka@gmail.com>
Date: Wed Nov 16 15:40:39 2016 +0200

Issue bpo-28701: Replace _PyUnicode_CompareWithId with _PyUnicode_EqualToASCIIId.

The latter function is more readable, faster and doesn't raise exceptions.

Based on patch by Xiang Zhang.

[vstinner]

Depending on how the stdlib abc.py file was loaded (in the main interpreter and in the subinterpreter), __code__.co_freevars[0] may or may not be an interned string.

When the bug occurs, I see that the Python stdlib abc.py file is loaded twice: the main interpreter builds a code object, and then subinterpreter builds its own code object: same content, but different Python object (at different memory addresses so inequal pointers!).

I modified reproducer.c to add "Py_VerboseFlag = 1;" before the Py_Initialize() call. Truncated output:
---

...
# code object from /opt/py310/lib/python3.10/abc.py
...
# code object from /opt/py310/lib/python3.10/abc.py
Traceback (most recent call last):
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
...
RuntimeError: super(): __class__ cell not found
...

---

[vstinner]

There are around 27 _PyUnicode_EqualToASCIIId() calls in the Python code base. I don't think that avoiding _PyUnicode_EqualToASCIIId() is a good solution :-)

Fixing _PyUnicode_EqualToASCIIId() to make it compatible with subinterpreters sound more reasonable: remove the PyUnicode_CHECK_INTERNED() test optimization.

[methane]

Should _PyUnicode_EqualToASCIIId() support comparing two unicode from different interpreter??

[vstinner]

Should _PyUnicode_EqualToASCIIId() support comparing two unicode from different interpreter?

Right now, there still many cases where objects are still shared between two interpreters:

• None, True, False singletons
• strings from code objects (according to what I saw when I reproduced the issue)
• objects from static types: type name (str), subtypes (tuple), MRO (tuple), etc.
• etc.

More details in the following issues:

• bpo-40533: [subinterpreters] Don't share Python objects between interpreters
• bpo-40512: [subinterpreters] Meta issue: per-interpreter GIL