-
-
Notifications
You must be signed in to change notification settings - Fork 30.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[subinterpreters] Static types incorrectly share some objects between interpreters #94673
Comments
The current Footnotes |
Python currently has a common GIL. |
I have the changes ready for this:
|
This is covered by PEP 684. The issue exists for implementation discussion and to anchor PRs. |
This has been discussed elsewhere. Also see #29228. FYI, for this PR (and in most cases) the performance penalty is insignificant. |
This is the first of several precursors to storing tp_subclasses (and tp_weaklist) on the interpreter state for static builtin types. We do the following: * add `_PyStaticType_InitBuiltin()` * add `_Py_TPFLAGS_STATIC_BUILTIN` * set it on all static builtin types in `_PyStaticType_InitBuiltin()` * shuffle some code around to be able to use _PyStaticType_InitBuiltin() * rename `_PyStructSequence_InitType()` to `_PyStructSequence_InitBuiltinWithFlags()` * add `_PyStructSequence_InitBuiltin()`.
Static builtin types are finalized by calling _PyStaticType_Dealloc(). Before this change, we were skipping finalizing such a type if it still had subtypes (i.e. its tp_subclasses hadn't been cleared yet). The problem is that types hold several heap objects, which leak if we skip the type's finalization. This change addresses that. For context, there's an old comment (from e9e3eab) that says the following: // If a type still has subtypes, it cannot be deallocated. // A subtype can inherit attributes and methods of its parent type, // and a type must no longer be used once it's deallocated. However, it isn't clear that is actually still true. Clearing tp_dict should mean it isn't a problem. Furthermore, the only subtypes that might still be around come from extension modules that didn't clean them up when unloaded (i.e. extensions that do not implement multi-phase initialization, AKA PEP 489). Those objects are already leaking, so this change doesn't change anything in that regard. Instead, this change means more objects gets cleaned up that before.
This is the last precursor to storing tp_subclasses (and tp_weaklist) on the interpreter state for static builtin types. Here we add per-type storage on PyInterpreterState, but only for the static builtin types. This involves the following: * add PyInterpreterState.types * move PyInterpreterState.type_cache to it * add a "num_builtins_initialized" field * add a "builtins" field (a static array big enough for all the static builtin types) * add _PyStaticType_GetState() to look up a static builtin type's state * (temporarily) add PyTypeObject.tp_static_builtin_index (to hold the type's index into PyInterpreterState.types.builtins) We will be eliminating tp_static_builtin_index in a later change.
Please get the PEP accepted before merging anything you can't easily revert. |
…102663) Aside from sys and builtins, _io is the only core builtin module that hasn't been ported to multi-phase init. We may do so later (e.g. pythongh-101948), but in the meantime we must at least take care of the module's static types properly. (This came up while working on pythongh-101660.) python#94673
There were cases where we do unnecessary work for builtin static types. This also simplifies some work necessary for a per-interpreter GIL.
…ach Interpreter (gh-104072) Until now, we haven't been initializing nor finalizing the per-interpreter state properly.
This makes it much cleaner to move more PyTypeObject fields to PyInterpreterState.
* main: pythongh-103822: [Calendar] change return value to enum for day and month APIs (pythonGH-103827) pythongh-65022: Fix description of tuple return value in copyreg (python#103892) pythonGH-103525: Improve exception message from `pathlib.PurePath()` (pythonGH-103526) pythongh-84436: Add integration C API tests for immortal objects (pythongh-103962) pythongh-103743: Add PyUnstable_Object_GC_NewWithExtraData (pythonGH-103744) pythongh-102997: Update Windows installer to SQLite 3.41.2. (python#102999) pythonGH-103484: Fix redirected permanently URLs (python#104001) Improve assert_type phrasing (python#104081) pythongh-102997: Update macOS installer to SQLite 3.41.2. (pythonGH-102998) pythonGH-103472: close response in HTTPConnection._tunnel (python#103473) pythongh-88496: IDLE - fix another test on macOS (python#104075) pythongh-94673: Hide Objects in PyTypeObject Behind Accessors (pythongh-104074) pythongh-94673: Properly Initialize and Finalize Static Builtin Types for Each Interpreter (pythongh-104072) pythongh-104016: Skip test for deeply neste f-strings on wasi (python#104071)
* main: (760 commits) pythonGH-104102: Optimize `pathlib.Path.glob()` handling of `../` pattern segments (pythonGH-104103) pythonGH-104104: Optimize `pathlib.Path.glob()` by avoiding repeated calls to `os.path.normcase()` (pythonGH-104105) pythongh-103822: [Calendar] change return value to enum for day and month APIs (pythonGH-103827) pythongh-65022: Fix description of tuple return value in copyreg (python#103892) pythonGH-103525: Improve exception message from `pathlib.PurePath()` (pythonGH-103526) pythongh-84436: Add integration C API tests for immortal objects (pythongh-103962) pythongh-103743: Add PyUnstable_Object_GC_NewWithExtraData (pythonGH-103744) pythongh-102997: Update Windows installer to SQLite 3.41.2. (python#102999) pythonGH-103484: Fix redirected permanently URLs (python#104001) Improve assert_type phrasing (python#104081) pythongh-102997: Update macOS installer to SQLite 3.41.2. (pythonGH-102998) pythonGH-103472: close response in HTTPConnection._tunnel (python#103473) pythongh-88496: IDLE - fix another test on macOS (python#104075) pythongh-94673: Hide Objects in PyTypeObject Behind Accessors (pythongh-104074) pythongh-94673: Properly Initialize and Finalize Static Builtin Types for Each Interpreter (pythongh-104072) pythongh-104016: Skip test for deeply neste f-strings on wasi (python#104071) pythongh-104057: Fix direct invocation of test_super (python#104064) pythongh-87092: Expose assembler to unit tests (python#103988) pythongh-97696: asyncio eager tasks factory (python#102853) pythongh-84436: Immortalize in _PyStructSequence_InitBuiltinWithFlags() (pythongh-104054) ...
…3912) his involves moving tp_dict, tp_bases, and tp_mro to PyInterpreterState, in the same way we did for tp_subclasses. Those three fields are effectively const for builtin static types (unlike tp_subclasses). In theory we only need to make their values immortal, along with their contents. However, that isn't such a simple proposition. (See gh-103823.) In the meantime the simplest solution is to move the fields into the interpreter. One alternative is to statically allocate the values, but that's its own can of worms.
…pythongh-105465) Fixes a typo in d2e2e53. (cherry picked from commit 5394bf9) Co-authored-by: neonene <53406459+neonene@users.noreply.github.com>
…gh-117761) Guido pointed out to me that some details about the per-interpreter state for the builtin types aren't especially clear. I'm addressing that by: * adding a comment explaining that state * adding some asserts to point out the relationship between each index and the interp/global runtime state
We now get a warning when building without pydebug:
|
I'll take a look. |
… Types (pythongh-117761) Guido pointed out to me that some details about the per-interpreter state for the builtin types aren't especially clear. I'm addressing that by: * adding a comment explaining that state * adding some asserts to point out the relationship between each index and the interp/global runtime state
static_builtin_index_is_set() is only used in asserts; inline it to avoid compiler warnings.
While static types (
PyTypeObject
values) don't themselves ever change (oncePyType_Ready()
has run), they do hold mutable data. This means they cannot be safely shared between multiple interpreters without a common GIL (and without state leaking between).Mutable data:
__class__
)__base__
) - set byPyType_Ready()
if not set__bases__
) - always set byPyType_Ready()
__mro__
) - always set byPyType_Ready()
__dict__
) - set byPyType_Ready()
if not set__subclasses__
)(See https://docs.python.org/3/c-api/typeobj.html#tp-slots.)
(Note that
tp_cache
is no longer used.)For the object header, if PEP 683 (immortal objects) is accepted then we can make static types immortal.
For the otherwise immutable objects, we can make sure each is immortal and then we're good. Even
tp_dict
is fine since it gets hidden behindtypes.MappingProxyType
. We'd also need to either make sure each contained item is immortal.For
tp_subclasses
we will need a per-interpreter copy, and do the proper lookup in the__subclasses__
getter. The cache could be stored onPyInterpreterState
or even as a dict intp_subclasses
.For
tp_weaklist
it's a similar story as fortp_subclasses
. Note thattp_weaklist
isn't very important for static types since they are never deallocated.(The above is also discussed in PEP 684.)
CC @kumaraditya303
Linked PRs
The text was updated successfully, but these errors were encountered: