Repository navigation
[subinterpreter] _PyUnicode_EqualToASCIIId() issue with subinterpreters #90164
Description
Activity
_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>
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)
ba3d67cSerhiy: 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 +0200Issue 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.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 ...
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.
Should
_PyUnicode_EqualToASCIIId()support comparing two unicode from different interpreter??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:
14 remaining items
I've created a PR for moving the interned strings and identifiers to _PyRuntimeState until we are ready to move them back to the interpreter.
If that seems okay, I'll work on a backport PR for 3.10.
are there any objections to my PR?
Personally, I'd rather we not revert the original change.
Even in 3.10? Your PR looks pretty heavy for a bugfix release, and won't apply cleanly to 3.10.
Moving the data to _PyRuntimeState would save me some effort with related work I'm doing right now.
I guess that's what makes the PR hard to review. What's the plan (or are there more conflicting plans), and what's the state in 3.10 vs. main?
IMO writing a complete rationale for running multiple interpreters in parallel which require a whole PEP. I didn't write such PEP yet since there are still non-trivial technical issues, especially the problem of static types: bpo-40601.
I don't have time right now to measure the performance overhead of PR 30123 on _PyUnicode_EqualToASCIIId() and on changing a type attribute (like overriding the __init__() method). I expect a minor overhead on micro-benchmark and no significant change on pyperformance macrobenchmarks. But again, right I don't have time to go through such analysis.
The priority for now is to unblock the Python 3.11 release and repair the Python 3.10 regression, so I'm fine with reverting my commit ea25180 which introduced the regression.
PR 30131 makes more changes than just reverting the commit, it changes the _PyRuntimeState structure and it also reverts the identifiers change, whereas so far, there is no known relationship between this issue and identifiers. IMO it's ok to leave identifiers unchanged.
+1 on just reverting in both branches. I can deal with my stuff separately.
IMO writing a complete rationale for running multiple interpreters in parallel which require a whole PEP.
FYI, I'm planning on having such a PEP published in the next few days.
_PyUnicode_EqualToASCIIId() and type update_slot() functions are fixed in 3.10 and main branches. The regression is now fixed.
But the revert reintroduces the issue on subinterpreters, so I created bpo-46283: "[subinterpreters] Unicode interned strings must not be shared between interpreters".
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:
bugs.python.org fields: