Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 14, 2020
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
61e40f6
Check if valgrind-detected after-freeing access of PyMethodDef causes…
YannickJadoul Oct 11, 2020
47403b9
fix: only apply leak on 3.9.0
henryiii Oct 12, 2020
350beac
fix: faster check
henryiii Oct 13, 2020
c08d9d4
fix: better naming thanks to @bstaletic
henryiii Oct 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,6 @@ jobs:
python: pypy3
arch: x64

# TODO: renable
# Currently segfaults on macOS Python 3.9
- runs-on: macos-latest
python: 3.9
arch: x64

name: "🐍 ${{ matrix.python }} • ${{ matrix.runs-on }} • ${{ matrix.arch }} ${{ matrix.args }}"
runs-on: ${{ matrix.runs-on }}

Expand Down
16 changes: 15 additions & 1 deletion include/pybind11/pybind11.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,12 @@ class cpp_function : public function {

/// When a cpp_function is GCed, release any memory allocated by pybind11
static void destruct(detail::function_record *rec) {
// If on Python 3.9, check the interpreter "MICRO" (patch) version.
// If this is running on 3.9.0, we have to work around a bug.
#if !defined(PYPY_VERSION) && PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION == 9
static bool is_zero = Py_GetVersion()[4] == '0';
#endif

while (rec) {
detail::function_record *next = rec->next;
if (rec->free_data)
Expand All @@ -466,7 +472,15 @@ class cpp_function : public function {
}
if (rec->def) {
std::free(const_cast<char *>(rec->def->ml_doc));
delete rec->def;
// Python 3.9.0 decref's these in the wrong order; rec->def
// If loaded on 3.9.0, let these leak (use Python 3.9.1 at runtime to fix)
// See https://github.com/python/cpython/pull/22670
#if !defined(PYPY_VERSION) && PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION == 9
if (!is_zero)
delete rec->def;
#else
delete rec->def;
#endif
}
delete rec;
rec = next;
Expand Down