Crash in specialize_dict_access(): type->ht_cached_keys is NULL on a pybind11 type

[vstinner]

The crash occurs while building the pikepdf documentation with Sphinx. Reproduce on Fedora 36 with these commands:

sudo dnf install qpdf-devel

python3.11 -m venv env
cd env
source ./bin/activate

python -m pip install IPython
python -m pip install sphinx sphinx_issues sphinx_design sphinx_rtd_theme

git clone https://github.com/pikepdf/pikepdf
cd pikepdf/
python -m pip install .

cd docs/
~/env/bin/sphinx-build . ../html

gdb traceback:

(gdb) py-bt
Traceback (most recent call first):
  File "/home/vstinner/env/lib/python3.11/site-packages/pikepdf/_methods.py", line 798, in open
    pdf._tmp_stream = tmp_stream
  File "<ipython-input-5-851f84133ed8>", line 1, in <cell line: 0>
  File "/home/vstinner/env/lib/python3.11/site-packages/IPython/core/interactiveshell.py", line 3398, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "/home/vstinner/env/lib/python3.11/site-packages/IPython/core/interactiveshell.py", line 3338, in run_ast_nodes
    if await self.run_code(code, result, async_=asy):
(...)

(gdb) where
#0  0x00000000004a5af3 in _PyDictKeys_StringLookup (dk=0x0, key='_tmp_stream') at Objects/dictobject.c:1011
#1  0x00000000005805c1 in specialize_dict_access (owner=owner@entry=<pikepdf._qpdf.Pdf at remote 0x7fffdf2f35f0>, instr=instr@entry=0x1289664, 
    type=type@entry=0x11afa20, name=name@entry='_tmp_stream', values_op=values_op@entry=154, hint_op=hint_op@entry=159, base_op=95, kind=<optimized out>)
    at Python/specialize.c:625
#2  0x0000000000580a42 in _Py_Specialize_StoreAttr (owner=<pikepdf._qpdf.Pdf at remote 0x7fffdf2f35f0>, instr=0x1289664, name='_tmp_stream')
    at Python/specialize.c:813
#3  0x000000000041fbe7 in _PyEval_EvalFrameDefault (tstate=0x84d910 <_PyRuntime+166320>, frame=0x7ffff7fb51d0, throwflag=18545184) at Python/ceval.c:3597
#4  0x000000000053dc20 in _PyEval_EvalFrame (throwflag=0, frame=0x7ffff7fb5170, tstate=0x84d910 <_PyRuntime+166320>)
    at ./Include/internal/pycore_ceval.h:73
(...)

Frame 0: crash in _PyDictKeys_StringLookup() because dk=NULL.

(gdb) frame 0
#0  0x00000000004a5af3 in _PyDictKeys_StringLookup (dk=0x0, key='_tmp_stream') at Objects/dictobject.c:1011
1011	    if (!PyUnicode_CheckExact(key) || kind == DICT_KEYS_GENERAL) {
(gdb) l
1006	 */
1007	Py_ssize_t
1008	_PyDictKeys_StringLookup(PyDictKeysObject* dk, PyObject *key)
1009	{
1010	    DictKeysKind kind = dk->dk_kind;
1011	    if (!PyUnicode_CheckExact(key) || kind == DICT_KEYS_GENERAL) {
1012	        return DKIX_ERROR;
1013	    }
1014	    Py_hash_t hash = unicode_get_hash(key);
1015	    if (hash == -1) {

(gdb) p dk
$10 = (PyDictKeysObject *) 0x0

Frame 1, specialize_dict_access(): call _PyDictKeys_StringLookup(NULL, name), keys is NULL:

(gdb) frame 1
#1  0x00000000005805c1 in specialize_dict_access (owner=owner@entry=<pikepdf._qpdf.Pdf at remote 0x7fffdf2f35f0>, instr=instr@entry=0x1289664, 
    type=type@entry=0x11afa20, name=name@entry='_tmp_stream', values_op=values_op@entry=154, hint_op=hint_op@entry=159, base_op=95, kind=<optimized out>)
    at Python/specialize.c:625
(...)

622	        // Virtual dictionary
623	        PyDictKeysObject *keys = ((PyHeapTypeObject *)type)->ht_cached_keys;
624	        assert(PyUnicode_CheckExact(name));
625	        Py_ssize_t index = _PyDictKeys_StringLookup(keys, name);

(...)

(gdb) p type->tp_name
$7 = 0x11af850 "pikepdf._qpdf.Pdf"
(gdb) p type->tp_base->tp_name
$8 = 0x7fffe8614595 "pybind11_object"

(gdb) p ((PyHeapTypeObject *)type)->ht_cached_keys
$9 = (struct _dictkeysobject *) 0x0

Fedora bug report: https://bugzilla.redhat.com/show_bug.cgi?id=2118215

[vstinner]

The type tp_flags has Py_TPFLAGS_MANAGED_DICT flag set:

(gdb) p type->tp_name
$3 = 0x10fa560 "pikepdf._qpdf.Pdf"

(gdb) p type->tp_flags & (1 << 4)
$4 = 16

The issue #92678 may be related: the revert (commit 312dab2) is part of Python 3.11.0rc1 (the version that I'm testing).

[vstinner]

@rwgk: Would you mind to have a look?

[vstinner]

cd pikepdf/
python -m pip install .

If I understood correctly, this command installed pybind11-2.10.0 in a temporary virtual environment to build the pikepdf C extensions.

[rwgk]

@rwgk: Would you mind to have a look?

Will do. Hopefully later today.

[rwgk]

• I was able to reproduce the stack trace in a fedora:36 docker container.
• I created Undo attr.h, detail/class.h changes made under PR #3923 pybind/pybind11#4142 which I believe will fix the issue.

But I couldn't figure out how to have the pybind11 diff applied when the python -m pip install . runs. I know next to nothing about pip and pyproject.toml. Could someone please help with a hint? What is the best way to handle this situation?

E.g., is there a way to run a patch command after pybind11 2.10.0 landed in pip's temporary working directory?

Or can I get pip to use a given working directory that I manually patch, then run pip again?

[hroncok]

You should be able to replace the pybind11 requirement here:

https://github.com/pikepdf/pikepdf/blob/3ebb63a1d38c3a4d8a11b5b13b6106a245726f80/pyproject.toml#L9

With a link to https://github.com/rwgk/pybind11/archive/refs/heads/undo_Py_TPFLAGS_MANAGED_DICT.zip

[build-system]
requires = [
  "setuptools >= 52",
  "setuptools-scm[toml] >= 7.0.5",
  "wheel >= 0.35",
  "https://github.com/rwgk/pybind11/archive/refs/heads/undo_Py_TPFLAGS_MANAGED_DICT.zip",
]
build-backend = "setuptools.build_meta"

[vstinner]

pikepdf calls pybind11 make_new_python_type() function. Simplified code:

inline PyObject *make_new_python_type(const type_record &rec) {
    ...
    auto *heap_type = (PyHeapTypeObject *) metaclass->tp_alloc(metaclass, 0);
    ...
    type->tp_name = full_name;
    type->tp_doc = tp_doc;
    type->tp_base = type_incref((PyTypeObject *) base);
    type->tp_basicsize = static_cast<ssize_t>(sizeof(instance));
    ...
    type->tp_as_number = &heap_type->as_number;
    type->tp_as_sequence = &heap_type->as_sequence;
    type->tp_as_mapping = &heap_type->as_mapping;
    type->tp_as_async = &heap_type->as_async;
    ...
    type->tp_flags |= Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HEAPTYPE;
    ...
    if (PyType_Ready(type) < 0) {
        pybind11_fail(std::string(rec.name) + ": PyType_Ready failed: " + error_string());
    }
    ...
    return (PyObject *) type;
}

This function calls PyType_Ready() but not type_new().

Maybe type_new() initializing ht_cached_keys can be moved from type_new() to PyType_Ready(), so when a project only calls PyType_Type(), we can make sure that ht_cached_keys is initialized. By the way, when PyType_Ready() is called on a heap type which doesn't have Py_TPFLAGS_MANAGED_DICT, PyType_Ready() sets the flag if the parent class has the flag set. For me, it's surprising that the flag can be set, without initializing ht_cached_keys.

When PyType_Ready() is called, the caller is supposed to be done with initializing the type members, and so it's ok to set (or override) some members like ht_cached_keys.

[tiran]

You are not the only one who had the idea to move ht_cached_keys initialization into PyType_Ready. My PR #96047 already implements the proposal. Mark is in favor, too.