Improve Python 3.11 support #3694

Skylion007 · 2022-02-07T16:38:12Z

Description

Unfortunately, 3.11a uncovered a bug in our multi-inheritance code that may cause memory leaks with dynamic_attrs, will deal with a follow up PR.
Fixes a compilation error caused by 3.11 requiring the use of the helper to get the f_back. This PR also backports the reference counting behavior of newer pythons (f_back being a strong reference that needs to be decremented).
Closes [BUG]: Remove Python 3.11/PyPy dispatch workaround #3530 by removing the workaround

Suggested changelog entry:

* Better support for Python 3.11 alphas.

include/pybind11/detail/class.h

…-311-frame-changes

Skylion007 · 2022-02-12T17:46:46Z

Okay, I have narrowed down the problem. It causes an MRO issue under these conditions:

A type has multiple inheritance
One of the bases has dynamic_attrs enabled
The first base does NOT have dynamic_attrs enabled.

This causes the multiple inheritance code complain similar to issue #597 . We fixed that one by ensuring all classes have the same metaclass. Something breaks this fix in 3.11.

Skylion007 · 2022-02-14T18:04:01Z

tests/test_multiple_inheritance.cpp

@@ -204,7 +204,9 @@ TEST_SUBMODULE(multiple_inheritance, m) {
    m.def("i801e_b2", []() -> I801B2 * { return new I801E(); });

    // test_mi_static_properties
-    py::class_<Vanilla>(m, "Vanilla").def(py::init<>()).def("vanilla", &Vanilla::vanilla);
+    py::class_<Vanilla>(m, "Vanilla", py::dynamic_attr())


Making all bases in the hierarchy either static or dynamic_attrs works, but is not a tenable solution.

…test-311-frame-changes

Skylion007 · 2022-02-28T18:14:58Z

include/pybind11/detail/type_caster_base.h

@@ -514,7 +515,14 @@ PYBIND11_NOINLINE std::string error_string() {
            errorString += "  " + handle(f_code->co_filename).cast<std::string>() + "("
                           + std::to_string(lineno)
                           + "): " + handle(f_code->co_name).cast<std::string>() + "\n";
-            frame = frame->f_back;
+#    if PY_VERSION_HEX >= 0x030900B1


Nit: Maybe we should add a backport section details/common.h . I just wasn't sure if the recommend static inline stuff would cause any nasty ODR / LTO issues so I left it as is.

tacaswell · 2022-02-28T20:42:11Z

This fixes compiling scipy with py311.

rwgk · 2022-02-28T21:12:27Z

I looked a bit but have questions for my understanding:

Is it true that

the change in pybind11.h is purely a 3.11 compatibility fix?
the change in type_caster_base.h is for 3.11 compatibility, but also an actual fix for a bug that exists already pre 3.11? (Does the fix involve the Py_XINCREF(frame); and matching Py_DECREF(frame);?)

It would be useful to explain in a couple sentences in the PR description.

Skylion007 · 2022-03-01T02:38:20Z

@rwgk This fixes 3.11 compatability. The issue is the new API returns a strong reference (instead of a weak reference) which means we need to decref it. The easiest way to handle this was to have the old code path make a strong reference too. I'll edit the PR description.

rwgk

The changes LGTM, too, but I'm wondering about

@Skylion007 I didn't see edits to the PR description, would be nice to have them.
the valgrind error looks like it needs attention
is it OK to ignore the upstream failure?

Skylion007 · 2022-03-01T18:51:14Z

@rwgk Good catch on the Valgrind error, seems I am not doing the incref / decrefs properly. Thoughts @henryiii ?

include/pybind11/detail/type_caster_base.h

include/pybind11/pybind11.h

for more information, see https://pre-commit.ci

Skylion007 · 2022-03-01T20:22:14Z

Thanks for pointing out that issue @henryiii. Looks good to me now.

rwgk · 2022-03-01T20:28:45Z

I'll run some quick interactive tests in with the Google toolchain.

…-311-frame-changes

include/pybind11/pybind11.h

rwgk · 2022-03-01T21:44:13Z

I did a manual & embarrassingly simple leak check (diff below + top command), with Python 3.7 and 3.9. I'm happy to report I didn't see a leak. (I identified that test by inserting an assert in the middle of the changed code in type_caster_base.h)

ASAN testing was happy, too.

+++ b/tests/test_exceptions.py
@@ -246,6 +246,7 @@ def test_throw_nested_exception():

 # This can often happen if you wrap a pybind11 class in a Python wrapper
 def test_invalid_repr():
+  while True:
     class MyRepr:
         def __repr__(self):
             raise AttributeError("Example error")

* Test out Python 3.11 migration * Clean up a bit * Remove todo * Test workaround * Fix potential bug uncovered in 3.11 * Try to fix it more * last ditch fix * Revert. Tp-traverse isn't the problem * Test workaround * Try this hack * Revert MRO changes * Use f_back properly * Qualify auto * Update include/pybind11/pybind11.h * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Simplify code slightly * Ensure co_varnames decref if dict_getitem throws * Eager decref f_code Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Backport of pybind#3694

Skylion007 added 2 commits February 7, 2022 11:20

Test out Python 3.11 migration

a0eb05b

Clean up a bit

6a0a4d0

Skylion007 added the python dev Working on development versions of Python label Feb 7, 2022

Skylion007 closed this Feb 7, 2022

Skylion007 reopened this Feb 7, 2022

Skylion007 requested review from henryiii and rwgk February 7, 2022 16:41

Skylion007 added 5 commits February 7, 2022 11:42

Remove todo

e3f3644

Test workaround

c0a1306

Fix potential bug uncovered in 3.11

a731769

Try to fix it more

be95611

last ditch fix

5864a2a

Skylion007 changed the title ~~Test 311 frame changes~~ Python 3.11 support Feb 7, 2022

Skylion007 commented Feb 10, 2022

View reviewed changes

include/pybind11/detail/class.h Outdated Show resolved Hide resolved

Skylion007 added 4 commits February 12, 2022 11:59

Merge branch 'master' of https://github.com/pybind/pybind11 into test…

4428748

…-311-frame-changes

Revert. Tp-traverse isn't the problem

b292dd8

Test workaround

729bb4f

Try this hack

8bd9214

Skylion007 commented Feb 14, 2022

View reviewed changes

Merge branch 'master' of https://github.com/skylion007/pybind11 into …

1457185

…test-311-frame-changes

Skylion007 mentioned this pull request Feb 27, 2022

FIX: support python311 #3762

Closed

Skylion007 added 2 commits February 27, 2022 13:02

Revert MRO changes

b375591

Use f_back properly

6fa3fc5

Skylion007 changed the title ~~Python 3.11 support~~ Improve Python 3.11 support Feb 28, 2022

Skylion007 commented Feb 28, 2022

View reviewed changes

Qualify auto

38f9096

henryiii approved these changes Mar 1, 2022

View reviewed changes

rwgk approved these changes Mar 1, 2022

View reviewed changes

henryiii reviewed Mar 1, 2022

View reviewed changes

include/pybind11/detail/type_caster_base.h Show resolved Hide resolved

henryiii reviewed Mar 1, 2022

View reviewed changes

include/pybind11/pybind11.h Outdated Show resolved Hide resolved

henryiii and others added 2 commits March 1, 2022 14:39

Update include/pybind11/pybind11.h

9b9df3d

[pre-commit.ci] auto fixes from pre-commit.com hooks

6868334

for more information, see https://pre-commit.ci

Skylion007 added 2 commits March 1, 2022 15:46

Simplify code slightly

1eccd5d

Merge branch 'master' of https://github.com/pybind/pybind11 into test…

b87c337

…-311-frame-changes

rwgk approved these changes Mar 1, 2022

View reviewed changes

include/pybind11/pybind11.h Outdated Show resolved Hide resolved

Skylion007 added 2 commits March 2, 2022 12:09

Ensure co_varnames decref if dict_getitem throws

eadcb87

Eager decref f_code

4add4c2

Skylion007 merged commit 42a8e31 into pybind:master Mar 2, 2022

Skylion007 deleted the test-311-frame-changes branch March 2, 2022 18:18

github-actions bot added the needs changelog Possibly needs a changelog entry label Mar 2, 2022

rwgk mentioned this pull request Mar 2, 2022

[v2.9] Backport some recent commits #3773

Merged

rwgk mentioned this pull request Mar 10, 2022

[smart_holder] git merge master #3794

Merged

henryiii removed the needs changelog Possibly needs a changelog entry label Mar 29, 2022

svenevs added a commit to svenevs/pybind11 that referenced this pull request Nov 4, 2022

Improve Python 3.11 support

3d90c7d

Backport of pybind#3694

svenevs mentioned this pull request Nov 4, 2022

Improve Python 3.11 support RobotLocomotion/pybind11#60

Merged

Lantern-r mentioned this pull request Nov 26, 2022

更新VTIL-Python的pybind11以支持Python3.11 wallds/NoVmpy#10

Open

rwgk mentioned this pull request Feb 10, 2023

FWD pybind11 google/pybind11clif#3694

Closed

kevinsmia1939 mentioned this pull request Jun 28, 2024

update pybind11 to 2.13.1 OpenChemistry/tomviz#2298

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Python 3.11 support #3694

Improve Python 3.11 support #3694

Skylion007 commented Feb 7, 2022 •

edited by henryiii

Loading

Skylion007 commented Feb 12, 2022

Skylion007 Feb 14, 2022

Skylion007 Feb 28, 2022

tacaswell commented Feb 28, 2022

rwgk commented Feb 28, 2022

Skylion007 commented Mar 1, 2022

rwgk left a comment

Skylion007 commented Mar 1, 2022

Skylion007 commented Mar 1, 2022

rwgk commented Mar 1, 2022

rwgk commented Mar 1, 2022

Improve Python 3.11 support #3694

Improve Python 3.11 support #3694

Conversation

Skylion007 commented Feb 7, 2022 • edited by henryiii Loading

Description

Suggested changelog entry:

Skylion007 commented Feb 12, 2022

Skylion007 Feb 14, 2022

Choose a reason for hiding this comment

Skylion007 Feb 28, 2022

Choose a reason for hiding this comment

tacaswell commented Feb 28, 2022

rwgk commented Feb 28, 2022

Skylion007 commented Mar 1, 2022

rwgk left a comment

Choose a reason for hiding this comment

Skylion007 commented Mar 1, 2022

Skylion007 commented Mar 1, 2022

rwgk commented Mar 1, 2022

rwgk commented Mar 1, 2022

Skylion007 commented Feb 7, 2022 •

edited by henryiii

Loading