GitLab

Trying to import golang on py3.12 crashes with segmentation fault with
the following traceback:

    #0  0x00007f5579f6bfec in Py_INCREF (op=0x0) at /home/kirr/local/py3.12/include/python3.12/object.h:641
    #1  __pyx_f_6golang_7_golang__patch_slot (__pyx_v_typ=0x563b309bd6c0 <PyBytes_Type>, __pyx_v_name=0x563b30ac0e00 <_PyRuntime+32128>, __pyx_v_func_or_descr=0x7f557a4e42c0) at golang/_golang.cpp:43758
    #2  0x00007f5579f641b9 in __pyx_pf_6golang_7_golang_34_ (__pyx_self=0x0) at golang/_golang.cpp:40581
    #3  0x00007f5579f637ba in __pyx_pw_6golang_7_golang_35_ (__pyx_self=0x0, unused=0x0) at golang/_golang.cpp:40427
    ...

    43750     /* "golang/_golang_str.pyx":1562
    43751    * cdef DEL = object()
    43752    * cdef _patch_slot(PyTypeObject* typ, str name, object func_or_descr):
    43753    *     typdict = <dict>(typ.tp_dict)             # <<<<<<<<<<<<<<
    43754    *     #print("\npatching %s.%s  with  %r" % (typ.tp_name, name, func_or_descr))
    43755    *     #print("old:  %r" % typdict.get(name))
    43756    */
    43757     __pyx_t_1 = ((PyObject *)__pyx_v_typ->tp_dict);
    43758     __Pyx_INCREF(__pyx_t_1);                       <--- HERE
    43759     __pyx_v_typdict = ((PyObject*)__pyx_t_1);
    43760     __pyx_t_1 = 0;

Here we see that _patch_slots uses PyBytes_Type.tp_dict and hits NULL pointer
dereference on it.

Before Python3.12 typ.tp_dict was never NULL for any ready type and this
code was correct, but starting since py3.12, as Boxiang explains

    By checking [Python 3.12 type object document](https://docs.python.org/3.12/c-api/typeobj.html#c.PyTypeObject.tp_dict),
    it said "Changed in version 3.12: Internals detail: For static builtin types, this is always NULL"

What is going on here is that in py3.12 they continued the work on
subinterpreters (PEP 684) and introduced per-interpreter
GIL (https://docs.python.org/3.12/whatsnew/3.12.html#pep-684-a-per-interpreter-gil).
The subinterpreters all have their own set of imported modules and
objects, but for builtin types like e.g. bytes, str, dict and
exception types, those types are shared in between the interpreters
since the types are effectively immutable. The sharing is a bit delicate
though because there is .__subclasses__ attribute of a type and that
attribute can have different values in different subinterpreters.
https://peps.python.org/pep-0684/#global-objects explains:

    Builtin static types are a special case of global objects that will be
    shared. They are effectively immutable except for one part:
    __subclasses__ (AKA tp_subclasses). We expect that nothing else on a
    builtin type will change, even the content of __dict__ (AKA tp_dict).

So starting from py3.12 .tp_subclasses is handled specially: for heap
types (type objects that are allocated on the heap instead of being
static), .tp_subclasses continues to be a pointer. However for static
builtin types, .tp_subclasses is now an integer "pointing" to
per-interpreter data inside PyInterpreterState.types.builtins array
keeping mutable type state there (https://github.com/python/cpython/commit/47e75a00).

And even though they were initially saying that the __dict__ might be
left intact, later they also discovered that there are ways to modify
typ.__dict__ even from python code

    https://github.com/python/cpython/issues/94673#issuecomment-1210918283
    https://github.com/python/cpython/issues/88004

and thus that for static types typ.tp_dict should be also considered
mutable and kept in per-interpreter state inside PyInterpreterState
instead of typ.tp_dict (https://github.com/python/cpython/commit/de64e756).

Which caused a bit of backward compatibility pain for tools that were
previously accessing .tp_dict directly (https://github.com/python/cpython/issues/105227)
and was resolved via exposing PyType_GetDict utility (https://github.com/python/cpython/commit/a840806d).

In our crash above we were hitting such NULL .tp_dict for bytes type.

--------

The simplest fix is relatively straightforward: use PyType_GetDict
instead of directly accessing type's .tp_dict field.

A more correct fix when patching slots of a type is to go through all
created interpreters and adjust corresponding slot everywhere. This is
left as TODO since we do not currently use subinterpreters in practice
and with "free threaded" python builds coming, one can achieve GIL-less
threading without subinterpreters at all.

This patch takes into account original bug report and fix attempt by Boxiang:

    nexedi/pygolang!33 (comment 231278)
    Daetalus/pygolang@b6a09755

/cc @Daetalus
/reviewed-by @jerome, @tomo
/reviewed-on nexedi/pygolang!35