feat: add release_gil_before_calling_cpp_dtor annotation for class_#5522
feat: add release_gil_before_calling_cpp_dtor annotation for class_#5522rwgk merged 2 commits intopybind:masterfrom
release_gil_before_calling_cpp_dtor annotation for class_#5522Conversation
…if#30092 (minor fixes). Note for completeness: These are identical to the current versions on the pybind11clif main branch (@ commit 4841661df5daf26ecdedaace54e64d0782e63f64): * test_class_release_gil_before_calling_cpp_dtor.cpp * test_class_release_gil_before_calling_cpp_dtor.py
|
@gentlegiantJGC could you please review (and formally approve if it looks good to you)? (This PR is pretty much exactly as reviewed previously by @rainwoodman.) |
|
Looking at this with a fresh eye, I got suspicious about a potential data race in the test, and ChatGPT agrees that there is a problem: It looks like a very easy fix. I'll add a commit in a minute. |
…dtor.cpp The original intent was to let the singleton leak, but making that tread-safe is slightly more involved than this solution. It's totally fine in this case if the RegistryType destructor runs on process teardown. See also: pybind#5522 (comment)
gentlegiantJGC
left a comment
gentlegiantJGC
left a comment
There was a problem hiding this comment.
The implementation looks fine to me.
I got a bit lost in the test code. You might want someone with more knowledge of the internals of pybind11 and python to look over this.
Thanks @gentlegiantJGC! Since the test was reviewed already (almost as-is), I'll go ahead with merging. |
henryiii
changed the title
release_gil_before_calling_cpp_dtor annotation for class_release_gil_before_calling_cpp_dtor annotation for class_
3 participants
Description
Closes #1446.
This PR is a backport of:
release_gil_before_calling_cpp_dtorannotation forclass_google/pybind11clif#30088 (main PR)It introduces the
py::release_gil_before_calling_cpp_dtoroption (py::class_argument). For background, see the description of google/pybind11clif#30088 and #1446.The core change (in pybind11.h
class_::dealloc()) is to add ReleaseGIL-try-catch-ReaquireGIL-rethrow logic around the existing code calling the C++ destructor of wrapped objects.All the rest is more-or-less boilerplate code to add the
py:: release_gil_before_calling_cpp_dtorannotation, then use it to enable the new feature if that annotation is provided by the user.Note for completeness:
These are identical to the current versions on the pybind11clif main branch @ commit 4841661:
@rainwoodman (original reviewer) for visibility.
Suggested changelog entry: