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.

[vstinner]

I hacked locally PyType_Ready() to initialize ht_cached_keys. With this change, building pikepdf documentation with Sphinx no longer crashes.

[rwgk]

You should be able to replace the pybind11 requirement here:

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

Thanks a lot @hroncok! It didn't work straightaway, but with your hint and looking at the docs I could figure it out quickly: I just had to insert pbyind11 @ before the URL.

Good news: the pybind11 PR fixes the crash.

diff --git a/pyproject.toml b/pyproject.toml
index 554f848..371a704 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -6,8 +6,7 @@ requires = [
   "setuptools >= 52",
   "setuptools-scm[toml] >= 7.0.5",
   "wheel >= 0.35",
-  "pybind11 >= 2.9.1",
-  "pybind11 < 2.10.0; python_implementation == 'PyPy'"
+  "pybind11 @ https://github.com/rwgk/pybind11/archive/refs/heads/undo_Py_TPFLAGS_MANAGED_DICT.zip"
 ]
 build-backend = "setuptools.build_meta"

[rwgk]

I hacked locally PyType_Ready() to initialize ht_cached_keys. With this change, building pikepdf documentation with Sphinx no longer crashes.

We seem to have two ways of fixing the crash now:

• IIUC the above is a change in Python 3.11
• Undo attr.h, detail/class.h changes made under PR #3923 pybind/pybind11#4142

Should I merge the pybind11 PR? — I'm guessing yes, because that enables testing with Python 3.11rc1. Could you please confirm?

[vstinner]

PR #96047 is going to be merged soon: PyType_Ready() will initialize ht_cached_keys in the next Python 3.11 release.

I'm not sure that pybind11 should be changed. I don't know well the effect of Py_TPFLAGS_MANAGED_DICT.

[Fidget-Spinner]

@pablogsal this issue should be fixed. I think it shouldn't block the 3.11 release.

[vstinner]

@pablogsal this issue should be fixed. I think it shouldn't block the 3.11 release.

The issue has the "release blocker" label.

I understood that #96047 fixed pybind11.

[pablogsal]

That was my understanding but nobody closed the issue so it was unclear if something else was missing. We should close it or remove the release blocker label if nothing else is missing here.

[vstinner]

I can no longer reproduce the crash. I confirm that pybind11 was fixed and works around in the 3.11 branch. I close the issue.

I ran commands of the first issue comment, and it no longer crashs. Tested versions:

(env) vstinner@mona$ python -m pip freeze
alabaster==0.7.12
asttokens==2.0.8
Babel==2.10.3
backcall==0.2.0
certifi==2022.6.15
charset-normalizer==2.1.1
decorator==5.1.1
deprecation==2.1.0
docutils==0.17.1
executing==1.0.0
idna==3.3
imagesize==1.4.1
ipython==8.5.0
jedi==0.18.1
Jinja2==3.1.2
lxml==4.9.1
MarkupSafe==2.1.1
matplotlib-inline==0.1.6
packaging==21.3
parso==0.8.3
pexpect==4.8.0
pickleshare==0.7.5
pikepdf @ file:///home/vstinner/python/3.11/env/pikepdf
Pillow==9.2.0
prompt-toolkit==3.0.31
ptyprocess==0.7.0
pure-eval==0.2.2
Pygments==2.13.0
pyparsing==3.0.9
pytz==2022.2.1
requests==2.28.1
six==1.16.0
snowballstemmer==2.2.0
Sphinx==5.1.1
sphinx-issues==3.0.1
sphinx-rtd-theme==1.0.0
sphinx_design==0.3.0
sphinxcontrib-applehelp==1.0.2
sphinxcontrib-devhelp==1.0.2
sphinxcontrib-htmlhelp==2.0.0
sphinxcontrib-jsmath==1.0.1
sphinxcontrib-qthelp==1.0.3
sphinxcontrib-serializinghtml==1.1.5
stack-data==0.5.0
traitlets==5.3.0
urllib3==1.26.12
wcwidth==0.2.5