fix: valgrind-detected after-freeing access of PyMethodDef (macOS Python 3.9.0 segfaults) by YannickJadoul · Pull Request #2576 · pybind/pybind11 · GitHub

Python 3.9 added this line to meth_dealloc, unfortunately for us, 2 lines too low.

Short summary of what goes wrong:

pybind11 stores the function_record in a capsule, and passes this as the self argument to PyCFunction_NewEx
The PyCFunctionObject object nicely keeps track of that, but decrefs m->m_self 2 lines before decrefing PyCFunction_GET_CLASS(m). The capsule with the function_record with the PyModuleDef gets freed.
PyCFunction_GET_CLASS accesses m_ml -> ml_flags, where m_ml now points to something that was destructed.
BOOM

I guess this being so close explains why we don't often get a crash here. Maybe macOS protects its memory better?

61e40f6 demonstrates that leaking the PyModuleDef resolves the Python 3.9 crash on macOS.
d10f154 would be a (the?) solution, if it weren't calling cpp_function during the construction of a cpp_function during the construction of a cpp_function during the construction of a cpp_function during ... and thus blowing up the stack.

Related issue on bugs.python.org: https://bugs.python.org/issue41237
PR on CPython adding the line to meth_dealloc that makes things crash: python/cpython#19936

… macOS Python 3.9 segfaults

Replacing delete rec->def; with

                static int counter = 0;
                std::cout << ++counter << std::endl;

and running pybind11's tests, gives me a final count of 1808, of which 49 before cleaning up (so things that would be cleaned up during, by pybind11, that are the true leaks, here):

======================================== test session starts ========================================
platform linux -- Python 3.8.6, pytest-5.3.2, py-1.8.0, pluggy-0.13.1
rootdir: /home/yannick/pybind11/tests, inifile: pytest.ini
plugins: lazy-fixture-0.6.2
collected 430 items                                                                                 

../../tests/test_async.py ..                                                                  [  0%]
../../tests/test_buffers.py .........                                                         [  2%]
../../tests/test_builtin_casters.py ....s...........                                          [  6%]
../../tests/test_call_policies.py ....1
.2
...                                                    [  8%]
../../tests/test_callbacks.py 3
4
...5
.6
.....                                                       [ 10%]
../../tests/test_chrono.py ...........................................                        [ 20%]
../../tests/test_class.py .............7
8
......9
10
.........11
12
13
.                                       [ 26%]
../../tests/test_constants_and_functions.py ....                                              [ 27%]
../../tests/test_copy_move.py ..14
15
16
17
..s..                                                         [ 29%]
../../tests/test_custom_type_casters.py ..                                                    [ 30%]
../../tests/test_docstring_options.py .                                                       [ 30%]
../../tests/test_eigen.py .........................                                           [ 36%]
../../tests/test_enum.py ......                                                               [ 37%]
../../tests/test_eval.py 18
19
.20
.                                                                   [ 37%]
../../tests/test_exceptions.py .........                                                      [ 40%]
../../tests/test_factory_constructors.py ....21
.22
..23
........                                      [ 43%]
../../tests/test_gil_scoped.py 24
25
26
27
.....                                                          [ 44%]
../../tests/test_iostream.py ............                                                     [ 47%]
../../tests/test_kwargs_and_defaults.py ........                                              [ 49%]
../../tests/test_local_bindings.py ..........                                                 [ 51%]
../../tests/test_methods_and_attributes.py .....28
........29
.......                               [ 56%]
../../tests/test_modules.py .....                                                             [ 57%]
../../tests/test_multiple_inheritance.py ........30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
...                                          [ 60%]
../../tests/test_numpy_array.py ................................................              [ 71%]
../../tests/test_numpy_dtypes.py ...............                                              [ 74%]
../../tests/test_numpy_vectorize.py ........                                                  [ 76%]
../../tests/test_opaque_types.py ...                                                          [ 77%]
../../tests/test_operator_overloading.py ....                                                 [ 78%]
../../tests/test_pickling.py .....                                                            [ 79%]
../../tests/test_pytypes.py ...............................                                   [ 86%]
../../tests/test_sequences_and_iterators.py ........                                          [ 88%]
../../tests/test_smart_ptr.py .............                                                   [ 91%]
../../tests/test_stl.py ........ss........                                                    [ 95%]
../../tests/test_stl_binders.py .........                                                     [ 97%]
../../tests/test_tagbased_polymorphic.py .                                                    [ 97%]
../../tests/test_union.py .                                                                   [ 98%]
../../tests/test_virtual_functions.py .46
47
.48
.49
.....                                                [100%]

====================================== short test summary info ======================================
SKIPPED [1] test_builtin_casters.py:135: no <string_view>
SKIPPED [1] test_copy_move.py:69: no <optional>
SKIPPED [1] test_stl.py:109: no <optional>
SKIPPED [1] test_stl.py:137: no <experimental/optional>
================================== 426 passed, 4 skipped in 5.81s ===================================

Check if valgrind-detected after-freeing access of PyMethodDef causes…

61e40f6

… macOS Python 3.9 segfaults

This was referenced Oct 11, 2020

Race condition in test_python_to_cpp_to_python_from_process test on 3.9? #2391

Open

bpo-42015: Reorder dereferencing calls in meth_dealloc, to make sure m_self is kept alive long enough python/cpython#22670

Merged

henryiii added this to the v2.6.0 milestone Oct 12, 2020

YannickJadoul force-pushed the py39-methoddef-crash branch from d10f154 to 61e40f6 Compare October 12, 2020 20:17

github-actions Bot added the ci related to the CI system label Oct 12, 2020

henryiii force-pushed the py39-methoddef-crash branch 2 times, most recently from 9661789 to bebdbd2 Compare October 12, 2020 20:58

fix: only apply leak on 3.9.0

47403b9

henryiii force-pushed the py39-methoddef-crash branch from bebdbd2 to 47403b9 Compare October 12, 2020 21:03

henryiii reviewed Oct 12, 2020

View reviewed changes

Comment thread include/pybind11/pybind11.h

YannickJadoul commented Oct 12, 2020

View reviewed changes

Comment thread include/pybind11/pybind11.h Outdated

YannickJadoul mentioned this pull request Oct 13, 2020

[bug] Python 3.9 macOS segfault #2558

Closed

This was referenced Oct 13, 2020

Rebuild python39 0 2 he4191d conda-forge/pybind11-feedstock#55

Merged

Rebuild for python39 conda-forge/python-goosefem-feedstock#7

Merged

henryiii changed the title Check if valgrind-detected after-freeing access of PyMethodDef causes macOS Python 3.9 segfaults fix: valgrind-detected after-freeing access of PyMethodDef (macOS Python 3.9.0 segfaults) Oct 13, 2020

fix: faster check

350beac

YannickJadoul commented Oct 13, 2020

View reviewed changes

Comment thread include/pybind11/pybind11.h

bstaletic reviewed Oct 13, 2020

View reviewed changes

Comment thread include/pybind11/pybind11.h Outdated

fix: better naming thanks to @bstaletic

c08d9d4

YannickJadoul commented Oct 13, 2020

View reviewed changes

YannickJadoul marked this pull request as ready for review October 13, 2020 20:12

henryiii closed this Oct 13, 2020

henryiii reopened this Oct 13, 2020

mbargull mentioned this pull request Oct 14, 2020

Add patch for https://bugs.python.org/issue42015 (which affects pybind11) conda-forge/python-feedstock#402

Merged

5 tasks

henryiii merged commit 493649f into pybind:master Oct 14, 2020

henryiii mentioned this pull request Oct 15, 2020

[chore] Test pybind11 2.6.0 (RC 2?) pytorch/pytorch#46367

Open

QuLogic referenced this pull request in pikepdf/pikepdf Nov 9, 2020

Block support for Python 3.9

4a6c779

hyanwong mentioned this pull request Nov 30, 2020

Segfault in multiple tests on OS X, Python 3.9 tskit-dev/tsinfer#376

Closed

YannickJadoul mentioned this pull request Dec 31, 2020

Fix various minor memory leaks in the tests (found by Valgrind in #2746) #2758

Merged

YannickJadoul deleted the py39-methoddef-crash branch January 20, 2021 23:15

YannickJadoul mentioned this pull request Jan 20, 2021

feat: requires-python pypa/cibuildwheel#536

Merged

rwgk mentioned this pull request Feb 10, 2023

FWD pybind11 google/pybind11clif#2576

Closed

fix: valgrind-detected after-freeing access of PyMethodDef (macOS Python 3.9.0 segfaults) by YannickJadoul · Pull Request #2576 · pybind/pybind11 · GitHub

fix: valgrind-detected after-freeing access of PyMethodDef (macOS Python 3.9.0 segfaults) - #2576

fix: valgrind-detected after-freeing access of PyMethodDef (macOS Python 3.9.0 segfaults)#2576
henryiii merged 4 commits into
pybind:masterfrom
YannickJadoul:py39-methoddef-crash

YannickJadoul commented Oct 11, 2020 •
edited by henryiii
Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

YannickJadoul left a comment •
edited
Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fix: valgrind-detected after-freeing access of PyMethodDef (macOS Python 3.9.0 segfaults) by YannickJadoul · Pull Request #2576 · pybind/pybind11 · GitHub

fix: valgrind-detected after-freeing access of PyMethodDef (macOS Python 3.9.0 segfaults) - #2576

fix: valgrind-detected after-freeing access of PyMethodDef (macOS Python 3.9.0 segfaults)#2576henryiii merged 4 commits intopybind:masterpybind/pybind11:masterfrom YannickJadoul:py39-methoddef-crashYannickJadoul/pybind11:py39-methoddef-crashCopy head branch name to clipboard

Conversation

YannickJadoul commented Oct 11, 2020 • edited by henryiii Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

YannickJadoul left a comment • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fix: valgrind-detected after-freeing access of PyMethodDef (macOS Python 3.9.0 segfaults)#2576
henryiii merged 4 commits into
pybind:masterfrom
YannickJadoul:py39-methoddef-crash

YannickJadoul commented Oct 11, 2020 •
edited by henryiii
Loading

YannickJadoul left a comment •
edited
Loading