Skip to content

gh-150865: Allow immortalizing objects as long as thread owns it #150956

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.

Sign up for GitHub

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

Open
seberg wants to merge 1 commit into
base: main
Choose a base branch
from
+32 −8
Open

gh-150865: Allow immortalizing objects as long as thread owns it #150956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions Doc/c-api/object.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,17 +812,21 @@ Object Protocol

.. c:function:: int PyUnstable_SetImmortal(PyObject *op)

Marks the object *op* :term:`immortal`. The argument should be uniquely referenced by
the calling thread. This is intended to be used for reducing reference counting contention
in the :term:`free-threaded build` for objects which are shared across threads.
Marks the object *op* :term:`immortal`. To successfully immortalize an object on a
:term:`free-threaded build` it must have been created by the calling thread.
Unicode objects are not immortalized and :c:func:`PyUnicode_InternInPlace` should
be used instead.
Comment on lines +817 to +818
Copy link
Copy Markdown
Member

@ZeroIntensity ZeroIntensity Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should document that some objects may not support immortalization in the future and that some objects will break when immortalized. (The second part should be in a .. warning note.)

This function is intended to be used for reducing reference counting contention
in free-threaded builds for objects which are shared across threads.
Comment on lines +819 to +820
Copy link
Copy Markdown
Member

@ZeroIntensity ZeroIntensity Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should note that it's intended for highly-specific cases where either:

  1. The object is not tracked by the GC (so DRC doesn't work).
  2. The contention occurs in native code calling Py_INCREF, where DRC does not apply.


This is a one-way process: objects can only be made immortal; they cannot be
made mortal once again. Immortal objects do not participate in reference counting
and will never be garbage collected. If the object is GC-tracked, it is untracked.

This function is intended to be used soon after *op* is created, by the code that
creates it, such as in the object's :c:member:`~PyTypeObject.tp_new` slot.
Returns 1 if the object was made immortal and returns 0 if it was not.
Returns 1 if the object was made immortal or is already immortal and returns 0
if it was not.
This function cannot fail.

.. versionadded:: 3.15
16 changes: 13 additions & 3 deletions Modules/_testcapi/object.c
View file
Open in desktop
Copy link
Copy Markdown
Member

@ZeroIntensity ZeroIntensity Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's expose PyUnstable_SetImmortal as a helper and add tests using some more exotic objects. I'm still not very keen on the idea that "owned by current thread" implies "safe to immortalize".

Here's an outline:

static PyObject *
pyobject_set_immortal(PyObject *self, PyObject *op)
{
    return PyLong_FromLong(PyUnstable_SetImmortal(op));
}
import _testcapi

class TestWhatever(TestCase):
    def test_set_immortal(self):
        for obj, expected_result in [(module_object, 1), (shared_module_object, 0), ...]:
            self.assertEqual(_testcapi.pyobject_enable_deferred_refcount(module), expected_result)

Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,10 @@ test_py_set_immortal(PyObject *self, PyObject *unused)
#ifdef Py_GIL_DISABLED
object.ob_tid = _Py_ThreadId();
object.ob_gc_bits = 0;
object.ob_ref_local = 1;
object.ob_ref_shared = 0;
object.ob_ref_local = 3;
Copy link
Copy Markdown
Member

@ZeroIntensity ZeroIntensity Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems kind of arbitrary, can you add a comment explaining why 3 is used?

object.ob_ref_shared = 128;
Copy link
Copy Markdown
Member

@ZeroIntensity ZeroIntensity Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the shared refcount being changed?

#else
object.ob_refcnt = 1;
object.ob_refcnt = 3;
#endif
object.ob_type = &PyBaseObject_Type;

Expand All @@ -227,8 +227,18 @@ test_py_set_immortal(PyObject *self, PyObject *unused)
assert(PyUnstable_IsImmortal(&object));

// Check already immortal object
rc = PyUnstable_SetImmortal(&object);
assert(rc == 1);

// If not owned by the current thread cannot immortalize
#if Py_GIL_DISABLED
object.ob_tid = _Py_ThreadId() + 1;
object.ob_ref_local = 1; // reset refcount

rc = PyUnstable_SetImmortal(&object);
assert(rc == 0);
assert(!PyUnstable_IsImmortal(&object));
#endif

// Check unicode objects
PyObject *unicode = PyUnicode_FromString("test");
Expand Down
12 changes: 11 additions & 1 deletion Objects/object.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2885,7 +2885,17 @@ int
PyUnstable_SetImmortal(PyObject *op)
Copy link
Copy Markdown
Member

@ZeroIntensity ZeroIntensity Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, looking at my version, we need to do a few other things here:

  1. The interpreter stack might have some live references to the object, and it currently thinks that the object is mortal. We need to undo that (see this function).
  2. I think we need to disable deferred reference counting on the object in case there are some queued references that will break later. I achieved this by exposing this function from gc_free_threading.c and calling _PyObject_MergePerThreadRefcounts.
  3. It's probably a good idea to lower the reftotal so -Xshowrefcount doesn't break. (See this loop.)
  4. If you want to support calling PyUnstable_SetImmortal on objects that have a non-zero ob_ref_shared as well, then you also need to make a stop-the-world pause (_PyEval_StopTheWorld/_PyEval_StartTheWorld) to ensure that the object is in a consistent state when immortalizing it.

This also furthers my suspicion that accidental immortalization does not work correctly.

Note that my implementation was also designed to eventually deallocate the immortal object, so there are some things in my PyUnstable_Immortalize that we don't need to do in PyUnstable_SetImmortal (namely the _Py_immortal stuff), but the above list makes sense to me even if the object isn't ever deallocated.

Copy link
Copy Markdown
Contributor Author

@seberg seberg Jun 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 4: Well, I'll assume stop the world likely not worth the effort. And I expect without that, there could be arbitrary increfs/decrefs in-flight on ob_ref_shared.
    So, unless we assume it's OK if (maybe except the flags space, I'll guess that only gets mutated from the owning thread/during stop-the-world) ob_ref_shared must be considered undefined for immortal objects. Otherwise, I expect it is best to just close this PR.

  • 3: Not sure I follow why it matters that immortalizing leaves the total refcount untouched. Immortalizing seems like "arbitrary code" and arbitrary code may modify it?

  • 1-2: This is maybe just more complicated then I think, I can't follow that deeply unfortunately. Part of me keeps wondering if there are two competing concepts of "immortal" here, but I just don't know (the best I can do is find this comment in PyStackRef_CheckValid right now:)

    /* Can be immortal if object was made immortal after reference came into existence */
assert(!_Py_IsStaticImmortal(obj));

    (and it isn't even clear to me that the current check would be enough to protect if that wasn't.)

Copy link
Copy Markdown
Member

@ZeroIntensity ZeroIntensity Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not sure what the difference between a "static immortal" and "immortal" is.

For 3, it would just be to improve debugging. Lowering the global refcount would make it so -Xshowrefcount shows 0 leaked references at the end.

{
assert(op != NULL);
if (!_PyObject_IsUniquelyReferenced(op) || PyUnicode_Check(op)) {
if (_Py_IsImmortal(op)) {
// If the object is immortal, be idempotent.
return 1;
}
#if Py_GIL_DISABLED
// Only the owning thread can immortalize the object safely
if (!_Py_IsOwnedByCurrentThread(op)) {
Comment on lines +2893 to +2894
Copy link
Copy Markdown
Member

@ZeroIntensity ZeroIntensity Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need to also check that ob_ref_shared == 0?

return 0;
}
#endif
if (PyUnicode_Check(op)) {
return 0;
}
_Py_SetImmortal(op);
Expand Down
Loading