Title: bpo-42015: Reorder dereferencing calls in meth_dealloc, to make sure m_self is kept alive long enough by YannickJadoul · Pull Request #22670 · python/cpython · GitHub
Open Graph Title: bpo-42015: Reorder dereferencing calls in meth_dealloc, to make sure m_self is kept alive long enough by YannickJadoul · Pull Request #22670 · python/cpython
X Title: bpo-42015: Reorder dereferencing calls in meth_dealloc, to make sure m_self is kept alive long enough by YannickJadoul · Pull Request #22670 · python/cpython
Description: In Python 3.9, the line Py_XDECREF(PyCFunction_GET_CLASS(m)); was added to meth_dealloc (in methodobject.c). Unfortunately for pybind11, it's inserted exactly two lines too low, since it accesses the PyMethodDef and we store the PyMethodDef instance in the capsule that's used as self-argument of the PyCFunction. Result: UB, since Py_XDECREF(m->m_self); brings down the refcount of the capsule to 0 and (indirectly) frees the PyMethodDef, while its contents are now still accessed. From the pybind11 perspective, it would be optimal if this could be fixed in CPython itself, by moving up this one Py_XDECREF 2 lines. This would a) be more efficient than creating a workaround, and b) allow old, existing versions of pybind11 to work with Python 3.9 (well, 3.9.1, then, hopefully); the user base of pybind11 has grown quite a bit and now includes giants like scipy or some Google libraries. This PR reorders those lines. If there's a different, recommended way of creating these PyCFunctionObjects dynamically and cleaning up the PyMethodDef, we'd be interested as well, to make sure these kinds of breakages are avoided in the future. Apologies for only figuring out now how to debug this, using valgrind. Up until yesterday, we only saw some failures in CI on macOS, but it was hard to reproduce and debug locally. Related issue: https://bugs.python.org/issue41237 pybind11 issue: pybind/pybind11#2558 pybind11 PR: pybind/pybind11#2576 https://bugs.python.org/issue42015
Open Graph Description: In Python 3.9, the line Py_XDECREF(PyCFunction_GET_CLASS(m)); was added to meth_dealloc (in methodobject.c). Unfortunately for pybind11, it's inserted exactly two lines too low, since it access...
X Description: In Python 3.9, the line Py_XDECREF(PyCFunction_GET_CLASS(m)); was added to meth_dealloc (in methodobject.c). Unfortunately for pybind11, it's inserted exactly two lines too low, since it ac...
Opengraph URL: https://github.com/python/cpython/pull/22670
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/files(.:format) |
| route-controller | pull_requests |
| route-action | files |
| fetch-nonce | v2:9266e709-28d6-8a96-5f8f-46120b62da81 |
| current-catalog-service-hash | ae870bc5e265a340912cde392f23dad3671a0a881730ffdadd82f2f57d81641b |
| request-id | E0D2:3C3DDE:37A3ECB:49E8C4F:6A5D354C |
| html-safe-nonce | aa5b983afbc240fa658191a0a3391fdc84ca420a54d192d90b6a9ee6e102b641 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJFMEQyOjNDM0RERTozN0EzRUNCOjQ5RThDNEY6NkE1RDM1NEMiLCJ2aXNpdG9yX2lkIjoiMjExMjM3ODQ1NTQxNzc2MzE0OCIsInJlZ2lvbl9lZGdlIjoiaWFkIiwicmVnaW9uX3JlbmRlciI6ImlhZCJ9 |
| visitor-hmac | a4ef3f38a5b6fc648886c169dc3e067c1317e512cf9b214ad6d6b7021f6b6269 |
| hovercard-subject-tag | pull_request:501673997 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/python/cpython/pull/22670/files |
| twitter:image | https://avatars.githubusercontent.com/u/8025744?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/8025744?s=400&v=4 |
| og:image:alt | In Python 3.9, the line Py_XDECREF(PyCFunction_GET_CLASS(m)); was added to meth_dealloc (in methodobject.c). Unfortunately for pybind11, it's inserted exactly two lines too low, since it access... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 5290d7e14309ad1e76106a9c4237bd1041517e83ea182c8ab756752cb0c6940b |
| turbo-cache-control | no-preview |
| diff-view | unified |
| go-import | github.com/python/cpython git https://github.com/python/cpython.git |
| octolytics-dimension-user_id | 1525981 |
| octolytics-dimension-user_login | python |
| octolytics-dimension-repository_id | 81598961 |
| octolytics-dimension-repository_nwo | python/cpython |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 81598961 |
| octolytics-dimension-repository_network_root_nwo | python/cpython |
| turbo-body-classes | logged-out env-production page-responsive full-width |
| disable-turbo | true |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | 9c975978430e9ad293956f2bbdaf153b1bd84a99 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width