bpo-43503: Make limited API objects effectively immutable. #24828

ericsnowcurrently · 2021-03-11T22:18:01Z

This is a slight alternative to @eduardo-elizondo's GH-19474, though our main motivations are different. Eddie is looking to optionally reduce the impact of refcounts on the copy-on-write semantics of forking.

My interest lies in supporting (immutable) objects that can be safely shared between interpreters without a shared GIL. In the short term this will help solve the problem of what to do about PyObjects that are part of the public C-API (and especially the limited API): the singletons and many static types (including the exception types).

The problem of those public objects is something I was thinking through last December. This approach using refcounts is one of several I tried. A couple weeks later I found Eddie's PR and adopted a few of his changes (e.g. the high positive bit, whereas I was previously using a negative value).

Notable differences from GH-19474:

CPU performance isn't impacted (no changes to _Py_INCREF() or Py_DECREF())
set the initial refcount for immortal objects to a value above the magic bit, rather than at it
- the actual value is halfway between the magic bit and the next bit up, to preserve the magic bit through normal incr/decr
the API is opt-in for the public API (but never the limited API), if _Py_IMMORTAL_OBJECTS is defined
all the builtin types are made immortal during runtime initialization instead of statically

Also note that if Py_IMMORTAL_CONST_REFCOUNTS is defined then the refcount of immortal objects essentially becomes immutable, as in GH-19474. That should meet Eddie's needs without being invasive.

UPDATE: I've dropped the "const refcounts" and "opt-in API" parts.

See #24828 (comment) for a more in-depth explanation of the motivation (and rationale) for this PR.

https://bugs.python.org/issue43503

pablogsal · 2021-03-11T22:39:30Z

[Removed my previous comment as I have misread the name of the macros]

nascheme · 2021-03-11T22:46:00Z

I was a bit confused as well. The PR doesn't work for the copy-on-write behavior until Py_IMMORTAL_CONST_REFCOUNTS is turned on. If so, there is a performance impact because of the _py_is_immortal() branch. If Py_IMMORTAL_CONST_REFCOUNTS is off, it doesn't work for the multi-thread usage (non-thread safe to update refcnts from different threads) and it doesn't help for the copy-on-write behavior. Is that correct Eric?

ericsnowcurrently · 2021-03-11T23:05:40Z

The PR doesn't work for the copy-on-write behavior until Py_IMMORTAL_CONST_REFCOUNTS is turned on.

Correct. And if it is not defined then you get immortal objects with refcounts that change like normal, just at a very large value.

If so, there is a performance impact because of the _py_is_immortal() branch.

I'm unclear on what performance impact you mean. If Py_IMMORTAL_CONST_REFCOUNTS is defined then you get the hit discussed in GH-19474. Otherwise (the default) there should be 0 performance impact since _Py_DECREF() and _Py_INCREF() are unchanged.

If Py_IMMORTAL_CONST_REFCOUNTS is off, it doesn't work for the multi-thread usage (non-thread safe to update refcnts from different threads)

That should be fine. The initial refcount for immortal objects in that case should make it resilient to any races on incr/decr on the refcount.

markshannon · 2021-03-12T10:37:11Z

Objects/object.c

@@ -1817,7 +1835,9 @@ _Py_NewReference(PyObject *op)
 #ifdef Py_REF_DEBUG
    _Py_RefTotal++;
 #endif
-    Py_SET_REFCNT(op, 1);
+    /* Do not use Py_SET_REFCNT to skip the Immortal Instance check. This


I think this is dangerous. An immortal object may have any number of pending decrefs on it. Setting its refcnt to 1 would result in crashes.

Yeah, I'd meant to address this. Thanks for reminding me.

markshannon · 2021-03-12T10:51:34Z

This, by itself, won't allow sharing objects between parallel sub-interpreters.
For an object to be shared it must be immutable (and the entire graph of objects reachable from it must be immutable).
Adding the ability to make objects immortal to the C-API means that mutable objects will be made immortal, so that immortality will not imply shareability, which will make this feature much less useful.

Also, I don't like the addition of compile-time flags, as they don't get tested, and will just start rotting.

vstinner

I dislike the _Py_IMMORTAL_OBJECTS macro idea.

the API is opt-in for the public API (but never the limited API), if _Py_IMMORTAL_OBJECTS is defined

It sounds like something different than Python. It would mean that by default, Python would crash if an extension is built with _Py_IMMORTAL_OBJECTS and use immortal object. You should use a Python built with _Py_IMMORTAL_OBJECTS and rebuild all your extension with _Py_IMMORTAL_OBJECTS to fully support immortable objects.

CPU performance isn't impacted (no changes to _Py_INCREF() or Py_DECREF())

You PR modify _Py_INCREF() and _Py_DECREF() ... Please run pyperformance benchmark suite to measure the _Py_IMMORTAL_OBJECTS overhead. I expect something like 1.1x slower.

bedevere-bot · 2021-03-12T11:18:57Z

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

vstinner · 2021-03-12T11:21:52Z

My interest lies in supporting (immutable) objects that can be safely shared between interpreters without a shared GIL.

I dislike the idea of hacking Py_INCREF/Py_DECREF for this specific use case. I would prefer to have shared data, and have one "proxy" object per interpreter. The proxy would protect against data race and has its own reference count.

gvanrossum · 2021-03-12T16:58:16Z

I'd like to take a step back. Let's forget (for now) about copy-on-write. Then we can leave INCREF/DECREF unchanged.

IIUC, the goal is to be able to share objects like PyExc_Exception (all exceptions are part of the C API) between multiple interpreters (PEP 554). (I think it also applies to built-in types like int and list, right?)

Since at the Python level such type objects are immutable, and assuming that C extensions won't break this, it would be sufficient to set the ob_refcnt field of such type objects to some very high value, and they would never be deleted. It's clever to choose some high bit and then multiply that bit by 1.5 to give the actual initial refcount value, so that we can test for that high bit as long as the INCREF or DECREF deficit for that object is no more than 0.5 times the selected bit's value.

As long as the GIL is shared, the refcount of immortal objects should always be higher than the initial immortal refcount value. If the GIL is not shared, we could see drift in the refcount due to races, but the drift (esp. on a 64-bit machine) would never be so much that it would push the refcount all the way to zero or past the immortal bit.

The only concern I have at this point is that class objects, including exceptions and builtin classes, maintain a list of weak references to all subclasses. This list is exposed via t.__subclasses__() and unless we change this arrangement, this would allow one interpreter to access subclasses defined in another interpreter. That would be bad, so before we rely on this I think we should figure out what to do about that. (Singletons like Py_None, Py_True and Py_False don't suffer from this problem.)

If we don't do something like this, how are we going to preserve backwards compatibility the existing C API where PyExc_Exception and others are part of the public C API? That is necessary if we ever want to obtain the goals of PEP 554 (even with a shared GIL).

FWIW, I agree that for communication between interpreters we should use an entirely different mechanism. The current PR is only under consideration as a solution for the problem of how to share immutable objects between interpreters that are part of the public C API. Eric's solution is a good one IMO.

ericsnowcurrently · 2021-03-12T17:21:16Z

I'd like to take a step back. Let's forget (for now) about copy-on-write. Then we can leave INCREF/DECREF unchanged.

Yeah, that's been a source of confusion. I was already thinking about pulling that part out to keep things more focused here.

IIUC, the goal is to be able to share objects like PyExc_Exception (all exceptions are part of the C API) between multiple interpreters (PEP 554). (I think it also applies to built-in types like int and list, right?)

All of that is correct.

Since at the Python level such type objects are immutable, and assuming that C extensions won't break this, it would be sufficient to set the ob_refcnt field of such type objects to some very high value, and they would never be deleted. It's clever to choose some high bit and then multiply that bit by 1.5 to give the actual initial refcount value, so that we can test for that high bit as long as the INCREF or DECREF deficit for that object is no more than 0.5 times the selected bit's value.

As long as the GIL is shared, the refcount of immortal objects should always be higher than the initial immortal refcount value. If the GIL is not shared, we could see drift in the refcount due to races, but the drift (esp. on a 64-bit machine) would never be so much that it would push the refcount all the way to zero or past the immortal bit.

Exactly! I'm proud of that idea. It's one of the few novel things I can claim in the PR. 🙂 We'll see if it holds up.

The only concern I have at this point is that class objects, including exceptions and builtin classes, maintain a list of weak references to all subclasses. This list is exposed via t.__subclasses__() and unless we change this arrangement, this would allow one interpreter to access subclasses defined in another interpreter.

Yeah. We would have to make __subclasses__ (and any other similar mutable state) a proxy around a per-interpreter copy. This is something @zooba and I had discussed last year.

That would be bad, so before we rely on this I think we should figure out what to do about that.

Agreed.

(Singletons like Py_None, Py_True and Py_False don't suffer from this problem.)

Correct.

If we don't do something like this, how are we going to preserve backwards compatibility the existing C API where PyExc_Exception and others are part of the public C API? That is necessary if we ever want to obtain the goals of PEP 554 (even with a shared GIL).

Right. AFAICS, the objects that are part of the public C-API are the biggest obstacle to eliminating all the unshareable C globals. That's why it's the thing I've been focused on lately. There are other approaches to tackle this, but none of them are as straightforward as the refcount solution.

FWIW, I agree that for communication between interpreters we should use an entirely different mechanism.

Same here.

The current PR is only under consideration as a solution for the problem of how to share immutable objects between interpreters that are part of the public C API.

Correct.

Eric's solution is a good one IMO.

Thanks! FWIW, I borrowed the main idea from a variety of old python-dev threads and past in-person discussions at sprints, as well as one or two details from Eddie's PR.

ericsnowcurrently · 2021-03-12T19:22:34Z

For the sake of better clarity, I've dropped the "const refcount" parts (what Eddie wants), as well as the "opt-in API" part. I also plan on taking a look at making static types properly immutable in this PR.

ericsnowcurrently · 2021-03-12T20:33:23Z

This, by itself, won't allow sharing objects between parallel sub-interpreters.
For an object to be shared it must be immutable (and the entire graph of objects reachable from it must be immutable).
Adding the ability to make objects immortal to the C-API means that mutable objects will be made immortal, so that immortality will not imply shareability, which will make this feature much less useful.

Good point. So even if immortality is part of the overall solution, a dedicated public API for immortal objects probably wouldn't be that helpful. A "public API for shareable" objects would be the right level. (The question of meeting Eddie's needs, relative to a public API, is a different matter.)

Regardless, the problem is more specific than shareability. We will probably end up with a per-interpreter copy of every builtin type and singleton. The real problem at hand is how to deal with all the PyObjects created as static variables in C. In the runtime that is all the static types and singletons, regardless of public or internal. In extensions it will be similar.

I expect that the answer will mostly be "stop creating static objects", and maybe provide some helpers to bridge the gap. However, that does not help with any PyObject already exposed by the public C-API. Thankfully that is a small set of cases (each of 5 singletons plus a portion of the static types), so we should be able to find a way to deal with it. That is what motivated this PR.

Ideally we would stop exposing those objects in the public API. If we did so, there are things we can do to make that less painful for extension authors and maybe even transparent.

With that in mind, here are some alternatives to having the actual objects in the public API (pretty much none require changes in extensions):

per-interpreter replacements + macros
- add an API to get the per-interpreter object for a given identity value (flag, global, etc.)
- change the existing names to macros that wrap the per-interpreter lookup
- responsibility: caller (mitigated by macros)
- upside: extensions can stop using the variables directly; a fairly straightforward change for us
- downside: breaks the stable ABI
per-interpreter replacements + placeholders
- keep the existing variables (as PyObject *?) but make them placeholder values (maybe tagged pointers?)
- add an API like in #1
- any API to which the existing variable might get passed must be fixed to look up the per-interpreter object
- responsibility: callee
- upside: same as #1
- downside: breaks the stable ABI; many touches all over the C-API
like #2, but keep them as PyObject *
- upside: same as #1; does not break the stable ABI(?)
- downside: many touches all over the C-API
make all the existing variables effectively const
- this would include making them immortal
- the singletons are not a problem
- requires dealing with the few mutable parts of static types (e.g. __subclasses__)
- upside: not a lot of changes for anyone; does not break the stable ABI; shareable between interpreters
- downside: we can't get rid of those globals; encourages continued use of globals?

My preference would be the first one. However, the tricky part is how we deal with the limited API. As far as I understand it, it means the first and second options aren't viable. I suppose we could do the first one for the public API and one of the others for the limited API, but that seems like a headache. (Perhaps it's the cost of having a stable ABI?)

So, this PR is in pursuit of that last option. I think we'll end up with at least the API from the first option, regardless of what we do, but that doesn't help fix the problem of the existing objects for us, relative to the limited API. Hence the introduction of immortal objects here.

(Sorry, I should have been more clear about all this in the PR text above.)

ericsnowcurrently · 2021-03-12T20:38:47Z

Include/object.h

+    if (_py_is_immortal((PyObject *)ob)) {
+        return;
+    }


Would it make sense to emit a warning for this case?

ericsnowcurrently · 2021-03-12T20:40:25Z

@vstinner, do my (and Guido's) other comments address your concerns?

vstinner · 2021-03-12T21:04:57Z

IIUC, the goal is to be able to share objects like PyExc_Exception (all exceptions are part of the C API) between multiple interpreters (PEP 554). (I think it also applies to built-in types like int and list, right?). Since at the Python level such type objects are immutable, and assuming that C extensions won't break this, (...)

The plan is to get rid of static types to replace them with heap types: see https://bugs.python.org/issue40077 The Exception class will become a heap type and so be mutable, as array.array for example:

$ python3.9
>>> import array; array.array.x=1
TypeError: can't set attributes of built-in/extension type 'array.array'

$ python3.10
>>> import array; array.array.x=1
# no error

At the C level, the str type is mutable (the cached hash is set when it's computed for the first time). It's hard to ensure that a Python cannot be modified by a subinterpreter :-/

vstinner · 2021-03-12T21:11:22Z

The only concern I have at this point is that class objects, including exceptions and builtin classes, maintain a list of weak references to all subclasses. This list is exposed via t.subclasses() and unless we change this arrangement, this would allow one interpreter to access subclasses defined in another interpreter.

This type member (subclasses) causes a very concrete issue which lead to crash in add_subclass() which is called by PyType_Ready(). I had the issue when I experimented running pip in parallel (with on GIL per interpreter): https://bugs.python.org/issue40512#msg383830

It's just yet another example why we should not share Python objects between interpreters.

IMO we must never share any Python object between two interpreters, mutable or immutable. There are too many corner cases leading to inconsistent objects or crashes if a single object is shared.

Sharing data is fine. Just put one Python proxy object on top of it per interpreter.

vstinner · 2021-03-12T21:18:19Z

If we don't do something like this, how are we going to preserve backwards compatibility the existing C API where PyExc_Exception and others are part of the public C API? That is necessary if we ever want to obtain the goals of PEP 554 (even with a shared GIL).

There is a similar problem with static types which should become heap types but are currently directly exposed as static type in the public C API: https://bugs.python.org/issue40601

I propose to replace &PyLong_Type with Py_GetLongType(). It would be a backward incompatible changes impacting a few extension modules.

I can add a pattern to my https://github.com/pythoncapi/pythoncapi_compat project to automate this change without losing support for old Python versions. I did something similar for other changes. For example, tstate->interp pattern is replaced with PyThreadState_GetInterpreter(tstate) and a pythoncapi_compat.h header file provides PyThreadState_GetInterpreter() to Python 3.8 and older (function introduced in Python 3.9).

I plan to propose a PEP to replace static types with heap types and the incompatible C API change.

Yeah, it's painful. I prefer to "break some eggs" (C API compat) to be able to move the "good" design with best performances.

vstinner · 2021-03-13T13:43:51Z

Eric:

Doesn't that break the stable ABI? All 5 singletons are exposed in the limited API, as are almost 70 exception types and roughly 70 other static types.

That change would break things, that's why a PEP is needed ;-)

vstinner

An "immortal" object is one for which Py_DECREF() will never try to deallocate it.

This change goes against https://bugs.python.org/issue1635741 which tries to delete all Python objects at exit.

I understand that right now, some Python objects are defined statically and don't need to be released at exit. There are many changes being done to get rid of statically allocated objects (like static types) to use dynamically allocated objects.

vstinner · 2021-03-13T13:50:49Z

Include/object.h

 static inline void _Py_SET_REFCNT(PyObject *ob, Py_ssize_t refcnt) {
+    // We don't have clean access to pycore_object.h at this point.
+    PyAPI_FUNC(int) _PyObject_IsImmortal(PyObject *);


It's weird to define a function inside a static inline function. Why not defining it outside the function?

If you don't want to expose _PyObject_IsImmortal() in the public C API, a better option would be to convert Py_SET_REFCNT() static inline function to a regular function. So you can do whatever you want in the implementation. Calling Py_SET_REFCNT() is uncommon and so doesn't need to be optimized, no?

Hey, it's good to see that my backward incompatible C API change on Py_REFCNT() being useful :-) In Python 3.9, it was allowed to write Py_REFCNT(obj) = refcnt;. I fixed many C extensions to use Py_REFCNT(): see bpo-39573 if you are curious.

If making _Py_SET_REFCNT() a regular function isn't a problem then I'll definitely take the cleaner approach.

vstinner · 2021-03-13T13:53:52Z

Objects/typeobject.c

@@ -146,7 +146,9 @@ _PyType_CheckConsistency(PyTypeObject *type)
        return 1;
    }

-    CHECK(Py_REFCNT(type) >= 1);
+    if (!_PyObject_IsImmortal((PyObject *)type)) {
+        CHECK(Py_REFCNT(type) >= 1);


I don't understand this change. Does it mean that refcount of immortal objects are negative and so Py_DECREF() on this type will call its deallocator?

Hmm, I thought I had removed this. It is a holdover from when I used a negative refcount to indicate immortal and no longer applies. I'll fix it.

vstinner · 2021-03-13T13:55:40Z

Doc/c-api/refcounting.rst

+   .. versionadded:: 3.10
+
+Also see :c:macro:`_PyObject_HEAD_IMMORTAL_INIT` and
+:c:macro:`_PyVarObject_HEAD_IMMORTAL_INIT`.


I don't think that we document API which are parts of the internal C API. If you want to document it, mention that it's only part of the internal C API.

We do document some internal API, for the sake of core development. However, the docs I added don't add much value in that regard. I was thinking more of them as eventually public, which I pulled out of this PR, so I'll remove the docs.

vstinner · 2021-03-13T13:56:26Z

Include/internal/pycore_object.h

+ * At the moment this API is strictly internal.  However, if it proves
+ * helpful for extension authors we may move it to the public API. */
+
+#define _Py_IMMORTAL_OBJECTS 1


I don't get the purpose of this macro since it's always defined.

It's there only as a marker relative to the public API. However, it is not necessary at this point and I'll drop it.

vstinner · 2021-03-13T13:56:54Z

Doc/c-api/structures.rst

+      _PyObject_IMMORTAL_INIT_REFCNT, type,
+
+   For now you must opt in to use this by defining
+   ``_Py_IMMORTAL_OBJECTS``.


It's always defined, no?

vstinner · 2021-03-13T14:03:29Z

Objects/object.c

 #define INIT_TYPE(TYPE, NAME) \
    do { \
        if (PyType_Ready(TYPE) < 0) { \
            return _PyStatus_ERR("Can't initialize " NAME " type"); \
        } \
+        _PyObject_SetImmortal((PyObject *)TYPE); \


Don't do that: using static types from multiple interpreters in parallel is unsafe. See for example the PyTypeObject.tp_subclasses issue.

Yuck, but yes. This detail seems to be a particular problem for the lofty idea of sub-interpreters one day eventually having their own GIL...

Right. This is something that has to be addressed before this PR can proceed.

vstinner · 2021-03-13T14:13:58Z

Guido:

There's no need to absolutely prevent it (nor would it be possible). It's clear that C extensions shouldn't mutate builtin types. C extensions or apps that do so anyway should be considered buggy. subclasses() is a method so we can change the representation of the data without breaking the API.

When I tested to run pip in parallel, what I see is that calling PyType_Ready() in parallel from different subinterpreters (running in different threads) is causing crashes since it calls add_subclass() without any kind of locking, whereas PyTypeObject.tp_subclasses dict is not thread safe.

Declaring builtin types such as object, int or str as immortal is likely to lead to a similar crash, so the overall approach sounds like a bad idea. The PR description explicitly lists static types:

My interest lies in supporting (immutable) objects that can be safely shared between interpreters without a shared GIL. In the short term this will help solve the problem of what to do about PyObjects that are part of the public C-API (and especially the limited API): the singletons and many static types (including the exception types).

Eric wrote:

Yeah. We would have to make subclasses (and any other similar mutable state) a proxy around a per-interpreter copy. This is something @zooba and I had discussed last year.

But the PR still makes builtin types as immortals.

subinterpreters issues are really complex and surprising. See for example this bug in the xml.parsers package caused by a change in the implementation of interned strings: https://bugs.python.org/issue40521#msg383829

The problem was that a C extension (pyexpat) wasn't ported to the new multiphase initialization API. Or said differently: a single shared Python object shared by multiple interpreters caused very surprising bad leading to a crash.

I know that it's appealing to hack the refcount because it looks simple and safe, but be aware that sharing any Python object between interpreters cause many complex and surprising bugs.

Also, I'm not comfortable with INCREF/REFCNT which modify the ref count in parallel without any synchronization. The PR makes the assumption that the _PyObject_IMMORTAL_BIT bit will not be cleared by DECREF. On 32-bit, _PyObject_IMMORTAL_BIT is 268_435_456. If you mark the None singleton as immortal, I'm not sure that the risk fo clearing the immortal bit is so unlikely.

gvanrossum · 2021-03-14T18:15:38Z

Victor, let's wait until Eric has submitted a new version of the PR. I agree that the issue with __subclasses__ should be fixed before we use this (except perhaps for NoneType and bool, which cannot be subclassed).

gpshead · 2021-03-14T19:46:59Z

Doc/c-api/refcounting.rst

+Immortal Objects
+================
+
+"Immortal" objects are those that are expected to never be deallocated


never be deallocated? even on main interpreter Py_Finalize()? It seems like we should kill the immortals at that point.

As I noted to Victor, it shouldn't be a problem for us to address immortal objects directly in Py_Finalize(). The question of dealing with static objects during runtime finalization is a separately question; one worth addressing.

gpshead · 2021-03-14T19:57:46Z

Objects/object.c

 #define INIT_TYPE(TYPE, NAME) \
    do { \
        if (PyType_Ready(TYPE) < 0) { \
            return _PyStatus_ERR("Can't initialize " NAME " type"); \
        } \
+        _PyObject_SetImmortal((PyObject *)TYPE); \


Yuck, but yes. This detail seems to be a particular problem for the lofty idea of sub-interpreters one day eventually having their own GIL...

ericsnowcurrently · 2021-03-15T16:08:32Z

An "immortal" object is one for which Py_DECREF() will never try to deallocate it.

This change goes against https://bugs.python.org/issue1635741 which tries to delete all Python objects at exit.

Cleaning up immortal objects at runtime finalization shouldn't be a problem. If it's a question of dealing with static objects then that is not a problem this PR introduces. 🙂

markshannon · 2021-03-15T16:35:32Z

We need to be very careful about sharing objects between interpreters.
It only takes one race condition on a mutable object anywhere in the entire code base, and 💥
Making an object immortal, does not make it immutable.

In other words, there is no point in making objects immortal until we have a robust way to ensure that they are immutable.

nascheme · 2021-03-15T17:55:37Z

I have concerns similar to Mark's with this approach. I can't picture the end state and how it might work. It would help to have a sketch of what the final design is going to be. This PR is just doing the easy part, IMHO.

One the key problems with removing the GIL (or implementing something like sub-interpreters) is that the reference counting it not thread safe. It (apparently) can only be made thread safe with large overhead. E.g. atomic operations for incref/decref or using something like buffered reference counting.

The PR is an attempt to skirt that issue by disabling automatic memory management for the objects that are going to be shared between interpreters. However, as Mark points out, you have to do that to all the state that gets shared. So, not just the immediate objects that are shared but all the objects reachable from those objects. Also, we have automatic memory management for a reason and just turning it off creates new problems. Finalizing objects during shutdown is one of the problems but likely there will be others.

IMHO, we need to devise a thread safe automatic memory management scheme that can be used for these shared objects and that can integrate with reference counting. We cannot remove reference counting because the C API depends on it. It seems to me that the key is to make the reference info something that is thread-local (e.g. ref counts or roots that a M&S collector follows) . Or, if not thread local, at least something you don't need locking to update.

Could we use per-interpreter proxy objects that wrap the real objects? That way, updating it's reference count is thread safe since it's only changed by one thread. The wrapping would have to apply to objects reachable from the proxy. I remember something like this was done for security purposes years ago (bastion, rexec). It is hard to get right though. If used for security proposes, you get security holes if you don't properly wrap things. For multi-thread usage, you could get mysterious crashes or mis-behavior that would be almost impossible to debug since it could be a result of race conditions.

ericsnowcurrently · 2021-03-15T18:25:13Z

For the moment let's set aside any objective of sharing objects between interpreters. It's not actually what I'm looking for right now. The more pressing issue for me is that we have a bunch of static PyObject (5 singletons and ~160 types) that are exposed in the limited API. The question is how to make them safe in a world of subinterpreters that do not share the GIL.

This PR is one possible approach and arguably the simplest. (Plus, the fixed list of objects makes the solution easier to maintain.) See my explanation in #24828 (comment).

(FWIW, I'd rather make each of those objects per-interpreter and expect we probably will regardless.)

ericsnowcurrently · 2021-03-15T19:29:34Z

FYI, I've created https://bugs.python.org/issue43503 to address that underlying problem, of which this PR is only one solution.

vstinner · 2021-03-15T22:41:34Z

For the moment let's set aside any objective of sharing objects between interpreters. It's not actually what I'm looking for right now. The more pressing issue for me is that we have a bunch of static PyObject (5 singletons and ~160 types) that are exposed in the limited API.

For singletons (None, True, False, ...), I proposed to make them per-interpreter, I have a draft PR: bpo-39511.

For static types, I'm working on a PEP to remove them from the C API and to convert them to heap types.

ericsnowcurrently · 2021-03-15T23:09:53Z

Hmm, what about saying that the limited API is only available for the main interpreter? Then pretty much all the problems go away.

(This feels like something someone already suggested at some point and I forgot about it.)

vstinner · 2021-03-16T09:38:24Z

Hmm, what about saying that the limited API is only available for the main interpreter? Then pretty much all the problems go away.

It has been proposed to mark extensions modules as being compatible with subinterpreters. For example, require to use the multiphase initialization API (PEP 489) and only define types as heap types. Trying to import an extension which is not marked this way would fail with an ImportError. The idea is to prevent random crashes if an extension access a Python object shared by multiple interpreters. The extension maintainer declares to be aware of subinterpreters issues and announces to not access shared Python objects.

About the specific case of static types, I understand that you propose to continue using &PyLong_Type in the main interpreter, but require to use Py_GetLongType() in subinterpreters. I don't know if that works. We should try :-p

ericsnowcurrently · 2021-03-16T15:20:32Z

Hmm, what about saying that the limited API is only available for the main interpreter? Then pretty much all the problems go away.

It has been proposed to mark extensions modules as being compatible with subinterpreters. For example, require to use the multiphase initialization API (PEP 489) and only define types as heap types. Trying to import an extension which is not marked this way would fail with an ImportError. The idea is to prevent random crashes if an extension access a Python object shared by multiple interpreters. The extension maintainer declares to be aware of subinterpreters issues and announces to not access shared Python objects.

Exactly. And the multi-phase init API isn't part of the limited API, thus extensions using it wouldn't be able to use multi-phase init, so no subinterpreters.

About the specific case of static types, I understand that you propose to continue using &PyLong_Type in the main interpreter, but require to use Py_GetLongType() in subinterpreters. I don't know if that works. We should try :-p

It's worth trying it out since it reduces the impact of the change. Macros would help as a transition aid (though it might be tricky since all the Py*_Type symbols are values rather than pointers).

Regardless, if we can take the limited-api-does-not-support-subinterpreters approach then a lot of options open up for us. I'm going to start a ~~python-dev~~ capi-sig thread about this.

ericsnowcurrently · 2021-03-16T16:43:50Z

https://mail.python.org/archives/list/capi-sig@python.org/thread/INLCGPMTYFLRTWQL7RB4MUQZ37JAFRAU/

ericsnowcurrently · 2021-03-17T14:09:59Z

I'm dropping this in favor of avoiding the limited API.

ericsnowcurrently requested a review from markshannon as a code owner March 11, 2021 22:18

the-knights-who-say-ni added the CLA signed label Mar 11, 2021

bedevere-bot added the awaiting core review label Mar 11, 2021

ericsnowcurrently requested a review from gvanrossum March 11, 2021 22:19

This was referenced Mar 11, 2021

Globals immortal objects flags ericsnowcurrently/cpython#9

Closed

"immortal" objects faster-cpython/ideas#14

Closed

pablogsal requested review from vstinner, methane and serhiy-storchaka March 11, 2021 22:32

ericsnowcurrently force-pushed the globals-immortal-objects-flags branch from e80c8cb to 5a00f82 Compare March 11, 2021 23:57

markshannon reviewed Mar 12, 2021

View reviewed changes

vstinner requested changes Mar 12, 2021

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting core review labels Mar 12, 2021

ericsnowcurrently force-pushed the globals-immortal-objects-flags branch from 091cca1 to 88dd6fa Compare March 12, 2021 18:56

ericsnowcurrently commented Mar 12, 2021

View reviewed changes

vstinner reviewed Mar 13, 2021

View reviewed changes

gpshead reviewed Mar 14, 2021

View reviewed changes

Drop a holdover from immortal objects with negative refcounts.

00948a0

ericsnowcurrently added 3 commits March 15, 2021 10:44

Move _Py_SET_REFCNT() out of the header file.

00504d9

Define _py_is_immortal() only for Py_IMMORTAL_CONST_REFCOUNTS.

e6fc84f

Drop Py_IMMORTAL_CONST_REFCOUNTS and make immortal objects internal.

f103804

ericsnowcurrently force-pushed the globals-immortal-objects-flags branch from 547240b to f103804 Compare March 15, 2021 16:56

ericsnowcurrently changed the title ~~bpo-40255: Support "immortal" objects.~~ bpo-43503: Support "immortal" objects. Mar 15, 2021

ericsnowcurrently changed the title ~~bpo-43503: Support "immortal" objects.~~ bpo-43503: Make limited API objects effectively immutable. Mar 15, 2021

ericsnowcurrently closed this Mar 17, 2021

gvanrossum mentioned this pull request Sep 15, 2021

Faster startup -- Experiment B -- streamline and tweak faster-cpython/ideas#65

Closed

ericsnowcurrently mentioned this pull request Dec 16, 2021

gh-84436: Implement Immortal Objects #19474

Merged

vstinner mentioned this pull request Apr 10, 2022

[subinterpreters] Per-interpreter singletons (None, True, False, etc.) #83692

Closed

ericsnowcurrently mentioned this pull request Apr 10, 2022

[subinterpreters] PyObject statics exposed in the limited API break isolation. #87669

Closed

eduardo-elizondo mannequin mentioned this pull request Jun 6, 2022

Fixing Copy on Writes from reference counting and immortal objects #84436

Closed

bpo-43503: Make limited API objects effectively immutable. #24828

bpo-43503: Make limited API objects effectively immutable. #24828

Conversation

ericsnowcurrently commented Mar 11, 2021 • edited Loading

pablogsal commented Mar 11, 2021

nascheme commented Mar 11, 2021

ericsnowcurrently commented Mar 11, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markshannon commented Mar 12, 2021

vstinner left a comment

Choose a reason for hiding this comment

bedevere-bot commented Mar 12, 2021

vstinner commented Mar 12, 2021

gvanrossum commented Mar 12, 2021

ericsnowcurrently commented Mar 12, 2021

ericsnowcurrently commented Mar 12, 2021

ericsnowcurrently commented Mar 12, 2021

Choose a reason for hiding this comment

ericsnowcurrently commented Mar 12, 2021

vstinner commented Mar 12, 2021

vstinner commented Mar 12, 2021

vstinner commented Mar 12, 2021

vstinner commented Mar 13, 2021

vstinner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vstinner commented Mar 13, 2021

gvanrossum commented Mar 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericsnowcurrently commented Mar 15, 2021

markshannon commented Mar 15, 2021

nascheme commented Mar 15, 2021

ericsnowcurrently commented Mar 15, 2021

ericsnowcurrently commented Mar 15, 2021

vstinner commented Mar 15, 2021 • edited by bedevere-bot Loading

ericsnowcurrently commented Mar 15, 2021 • edited Loading

vstinner commented Mar 16, 2021

ericsnowcurrently commented Mar 16, 2021 • edited Loading

ericsnowcurrently commented Mar 16, 2021

ericsnowcurrently commented Mar 17, 2021

ericsnowcurrently commented Mar 11, 2021 •

edited

Loading

vstinner commented Mar 15, 2021 •

edited by bedevere-bot

Loading

ericsnowcurrently commented Mar 15, 2021 •

edited

Loading

ericsnowcurrently commented Mar 16, 2021 •

edited

Loading