Adding tp_reachable by mjp41 · Pull Request #65 · fxpl/cpython

mjp41 · 2026-01-16T22:10:48Z

📚 Documentation preview 📚: https://cpython-previews--65.org.readthedocs.build/

xFrednet

The addition itself looks good, but it doesn't set the tp_reachable for most built-in types. Once that is done it should be good to go.

mjp41 · 2026-01-19T09:47:11Z

The addition itself looks good, but it doesn't set the tp_reachable for most built-in types. Once that is done it should be good to go.

Sounds good. I mostly wanted to get someone else's eyes on it, and then we can decide how broad a blast radius we want.

Modules/_immutablemodule.c

mjp41 · 2026-02-26T16:46:50Z

Objects/codeobject.c

+    if (co->_co_cached != NULL) {
+        Py_VISIT(co->_co_cached->_co_code);
+        Py_VISIT(co->_co_cached->_co_varnames);
+        Py_VISIT(co->_co_cached->_co_cellvars);
+        Py_VISIT(co->_co_cached->_co_freevars);
+    }


This makes me worry about write barriers on the cache. There is a lot of stuff around modifying this for NoGIL

mjp41 · 2026-02-27T09:40:53Z

Objects/typeobject.c

+/*
+ * TODO: We should discuss this.  I am a little confused about the
+ * correct way to do this.  This gives a good level of compatibility,
+ * but is perhaps too much.
+ *


Can we discuss this?

xFrednet · 2026-03-02T10:54:55Z

Objects/dictobject.c

+static int
+dict_reachable(PyObject *op, visitproc visit, void *arg)


NIT: When I implemented this, I used a dict_visit function with a flag to indicate it the keys should be used or not:

https://github.com/mjp41/cpython/blob/e6fe674b476b2609e291f41812de9a235e191fea/Objects/dictobject.c#L4975-L5025

I think that's slightly cleaner than having two mostly identical visit functions, but this is a style thing and this is good to go as is.

xFrednet · 2026-03-02T10:56:22Z

Objects/exceptions.c

+    .tp_reachable = BaseException_reachable,
+    0,                          /* tp_weaklistoffset */
+    0,                          /* tp_iter */
+    0,                          /* tp_iternext */
+    BaseException_methods,      /* tp_methods */
+    BaseException_members,      /* tp_members */
+    BaseException_getset,       /* tp_getset */
+    0,                          /* tp_base */
+    0,                          /* tp_dict */
+    0,                          /* tp_descr_get */
+    0,                          /* tp_descr_set */
+    offsetof(PyBaseExceptionObject, dict), /* tp_dictoffset */
+    BaseException_init,         /* tp_init */
+    0,                          /* tp_alloc */
+    BaseException_new,          /* tp_new */
+    .tp_vectorcall = BaseException_vectorcall,


This seems weird, why does the change add all of these? Shouldn't it just be

.tp_reachable = BaseException_reachable,

xFrednet

Good changes, mostly high level comments

xFrednet · 2026-03-02T10:59:34Z

Objects/typeobject.c

+    Py_VISIT(lookup_tp_bases(type));
+    Py_VISIT(type->tp_base);
+
+    /* Do NOT visit tp_subclasses or tp_weaklist here.


Funny how we talked about tp_weaklist earlier today ^^

Yeah, my memory of what I think we do, and what we do is not consistent anymore

xFrednet · 2026-03-02T12:03:00Z

Objects/typevarobject.c

+static int
+paramspecattr_reachable(PyObject *self, visitproc visit, void *arg)
+{
+    Py_VISIT(_PyObject_CAST(Py_TYPE(self)));
+    return paramspecattr_traverse(self, visit, arg);
+}


Why do types created via typeslots need a custom tp_reachable implementation? Shouldn't these types visit the type anyways?

I hoped that we could just reuse the tp_traverse for these.

Not sure. Should the typeslots have to have it? There are a lot of things already created by typeslots, and we don't want them to magically get a tp_reachable without some thought?

Sorry, I mean that most types that already implement tp_traverse should be able to say .tp_reachable = my_traverse and be good to go. AFAIK objects with heap types are supposed to visit them in tp_traverse

Python/immutability.c

xFrednet · 2026-03-02T12:08:26Z

Python/immutability.c

+                    PySys_FormatStderr(
+                        "freeze: type '%.100s' has no tp_traverse and no tp_reachable\n",
+                        tp->tp_name);


This feels like it should only be printed in debug builds?

Thinking about it, each freezable type should have a tp_reachable since it at least needs to reference the type. So maybe add an explanation comment and then leave the message.

Guess it depends on whether we are just trying to run stuff and see what happens?

For the current state of the implementation, it's probably correct to log this, so let's keep it as is for now.

Python/immutability.c

xFrednet · 2026-03-03T08:40:54Z

Modules/_test_reachable.c

+static PyModuleDef_Slot _test_reachable_slots[] = {
+    {Py_mod_exec, _test_reachable_exec},
+    {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+    {Py_mod_gil, Py_MOD_GIL_NOT_USED},


We probably want a Py_mod_reachable as well. But I think this should be done in a follow-up PR. I also need to hack on modules again, so I can take over the addition of that.

xFrednet · 2026-03-03T09:36:40Z

The test.test_inspect.test_inspect CI error comes from me. The other ones seem to be from the changes in this PR (Just based on a quick skim)

The ASAN may be also on immutable main. At least I also got an ASan error:

ASAN error

==15233==ERROR: AddressSanitizer: heap-use-after-free on address 0x6130001b9b30 at pc 0x000101255a0c bp 0x00016efef6d0 sp 0x00016efef6c8
READ of size 8 at 0x6130001b9b30 thread T0
    #0 0x000101255a08 in clear_lock_held dictobject.c:2969
    #1 0x000101255b40 in PyDict_Clear dictobject.c:3000
    #2 0x000101781884 in gc_collect_region gc.c:1817
    #3 0x000101777f18 in _PyGC_Collect gc.c:2099
    #4 0x000101943ca8 in gc_collect gcmodule.c.h:143
    #5 0x0001016028e4 in _PyEval_EvalFrameDefault generated_cases.c.h:2361
    #6 0x0001015da440 in PyEval_EvalCode ceval.c:906
    #7 0x0001015ce574 in builtin_exec bltinmodule.c.h:571
    #8 0x000101130ed0 in _PyObject_VectorcallTstate pycore_call.h:169
    #9 0x00010161f50c in _PyEval_EvalFrameDefault generated_cases.c.h:1620
    #10 0x0001015da884 in _PyEval_Vector ceval.c:2023
    #11 0x000101133af0 in _PyVectorcall_Call call.c:272
    #12 0x00010194113c in pymain_run_module main.c:353
    #13 0x00010193f700 in Py_RunMain main.c:772
    #14 0x000101940948 in pymain_main main.c:802
    #15 0x000101940c24 in Py_BytesMain main.c:826
    #16 0x00018d759d50  (<unknown module>)

0x6130001b9b30 is located 112 bytes inside of 352-byte region [0x6130001b9ac0,0x6130001b9c20)
freed by thread T0 here:
    #0 0x000103451424 in free+0x7c (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3d424)
    #1 0x00010137aaec in subtype_dealloc typeobject.c:2896
    #2 0x0001012d5948 in _Py_Dealloc object.c:3228
    #3 0x00010126f670 in dictkeys_decref dictobject.c:462
    #4 0x00010125eaf8 in dict_dealloc dictobject.c
    #5 0x0001012d5948 in _Py_Dealloc object.c:3228
    #6 0x00010125ea38 in dict_dealloc dictobject.c:3379
    #7 0x0001012d5948 in _Py_Dealloc object.c:3228
    #8 0x00010137aa3c in subtype_dealloc typeobject.c:2867
    #9 0x0001012d5948 in _Py_Dealloc object.c:3228
    #10 0x0001012554fc in clear_lock_held dictobject.c:2969
    #11 0x000101255b40 in PyDict_Clear dictobject.c:3000
    #12 0x000101781884 in gc_collect_region gc.c:1817
    #13 0x000101777f18 in _PyGC_Collect gc.c:2099
    #14 0x000101943ca8 in gc_collect gcmodule.c.h:143
    #15 0x0001016028e4 in _PyEval_EvalFrameDefault generated_cases.c.h:2361
    #16 0x0001015da440 in PyEval_EvalCode ceval.c:906
    #17 0x0001015ce574 in builtin_exec bltinmodule.c.h:571
    #18 0x000101130ed0 in _PyObject_VectorcallTstate pycore_call.h:169
    #19 0x00010161f50c in _PyEval_EvalFrameDefault generated_cases.c.h:1620
    #20 0x0001015da884 in _PyEval_Vector ceval.c:2023
    #21 0x000101133af0 in _PyVectorcall_Call call.c:272
    #22 0x00010194113c in pymain_run_module main.c:353
    #23 0x00010193f700 in Py_RunMain main.c:772
    #24 0x000101940948 in pymain_main main.c:802
    #25 0x000101940c24 in Py_BytesMain main.c:826
    #26 0x00018d759d50  (<unknown module>)

previously allocated by thread T0 here:
    #0 0x000103451330 in malloc+0x78 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3d330)
    #1 0x000101315b30 in _PyMem_DebugRawAlloc obmalloc.c:2887
    #2 0x000101371140 in _PyType_AllocNoTrack typeobject.c:2548
    #3 0x000101370efc in PyType_GenericAlloc typeobject.c:2579
    #4 0x000101388478 in object_new typeobject.c:7221
    #5 0x0001013c10e4 in tp_new_wrapper typeobject.c:10196
    #6 0x0001012b1264 in cfunction_call methodobject.c:564
    #7 0x0001011316c8 in _PyObject_MakeTpCall call.c:241
    #8 0x0001015ed5b8 in _PyEval_EvalFrameDefault generated_cases.c.h:4021
    #9 0x0001015da884 in _PyEval_Vector ceval.c:2023
    #10 0x0001011311a0 in _PyObject_VectorcallDictTstate call.c:145
    #11 0x000101134fa4 in _PyObject_Call_Prepend call.c:503
    #12 0x0001013c1bfc in slot_tp_new typeobject.c:10907
    #13 0x000101383d7c in type_call typeobject.c:2492
    #14 0x000101133f28 in _PyObject_Call call.c:360
    #15 0x000101606e18 in _PyEval_EvalFrameDefault generated_cases.c.h:2616
    #16 0x0001015da884 in _PyEval_Vector ceval.c:2023
    #17 0x000101130ed0 in _PyObject_VectorcallTstate pycore_call.h:169
    #18 0x00010138fb50 in call_attribute typeobject.c:10681
    #19 0x00010138f830 in _Py_slot_tp_getattr_hook typeobject.c
    #20 0x0001012cace4 in PyObject_GetAttr object.c:1319
    #21 0x00010161dfc0 in _PyEval_EvalFrameDefault generated_cases.c.h:7872
    #22 0x0001015da884 in _PyEval_Vector ceval.c:2023
    #23 0x0001011416ec in _PyObject_VectorcallTstate pycore_call.h:169
    #24 0x00010113da4c in method_vectorcall classobject.c:95
    #25 0x000101133af0 in _PyVectorcall_Call call.c:272
    #26 0x000101606e18 in _PyEval_EvalFrameDefault generated_cases.c.h:2616
    #27 0x0001015da884 in _PyEval_Vector ceval.c:2023
    #28 0x00010113122c in _PyObject_VectorcallDictTstate call.c:134
    #29 0x000101134fa4 in _PyObject_Call_Prepend call.c:503

Co-authored-by: Fridtjof Stoldt <xFrednet@gmail.com>

xFrednet reviewed Jan 19, 2026

View reviewed changes

mjp41 force-pushed the tp_reaches branch from 40d1fce to 35c470c Compare February 20, 2026 16:59

mjp41 commented Feb 27, 2026

View reviewed changes

mjp41 requested a review from xFrednet February 27, 2026 09:41

mjp41 marked this pull request as ready for review February 27, 2026 09:41

mjp41 force-pushed the tp_reaches branch from 01fc9f9 to 1d8ae54 Compare February 27, 2026 16:00

xFrednet reviewed Mar 2, 2026

View reviewed changes

xFrednet reviewed Mar 3, 2026

View reviewed changes

mjp41 force-pushed the tp_reaches branch from 4039862 to 9e9ee08 Compare March 3, 2026 11:38

mjp41 and others added 13 commits March 3, 2026 12:42

WIP

93e486b

More types tp_reachable

8577826

More tp_reachable

302d96f

Message on using tp_traverse

771ebd4

Pre-Code review comments.

2b27e79

Add a few more types.

1974cc9

Add missing file

6edc6b8

Addressing code review comments.

a7c0bf9

Adding missing parts of configure script.

c4008bc

Update Python/immutability.c

ce625c2

Co-authored-by: Fridtjof Stoldt <xFrednet@gmail.com>

Fix

c09d120

Trying to get it through CI.

e7a7b44

Fix unused from CI warnings.

7845168

mjp41 force-pushed the tp_reaches branch from e6e94eb to 7845168 Compare March 3, 2026 12:43

mjp41 added 3 commits March 3, 2026 12:47

Configureorder.

4b64e12

REmove unused.

33c2bb0

Fix to CI

770c9c3

mjp41 added 6 commits March 3, 2026 14:48

Fix test of Type size.

73d1cba

Some changes

0d1a470

Some changes

00c5a27

Fix one of the CI tests

93e02e1

Fix windows build

8cf4b11

Fix warning.

591b7ae

mjp41 merged commit 3f5bf95 into immutable-main Mar 4, 2026
28 of 43 checks passed

		static int
		dict_reachable(PyObject op, visitproc visit, void arg)

Conversation

mjp41 commented Jan 16, 2026 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xFrednet left a comment

Choose a reason for hiding this comment

Uh oh!

mjp41 commented Jan 19, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xFrednet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xFrednet Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xFrednet commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mjp41 commented Jan 16, 2026 •

edited by github-actions bot

Loading

xFrednet Mar 3, 2026 •

edited

Loading

xFrednet commented Mar 3, 2026 •

edited

Loading