Skip to content

Conversation

rwgk
Copy link
Collaborator

@rwgk rwgk commented Mar 14, 2021

Enables safely passing unique_ptr from Python to C++, even for trampolines.

A disowned trampoline Python object is kept it alive until the unique_ptr pointee is destroyed. The critical small trick needed was found in PyCLIF (probably invented by @mrovner):

google/clif@acacae6

The critical part of the trick is: the Py_INCREF is triggered only when the trampoline Python object is in the process of being disowned.

This PR goes beyond PyCLIF, in that the disowned Python object still maintains a pointer that is only reset and deregistered when the unique_ptr pointee is deleted, which means methods of the pointee can still be safely accessed from Python. This could be viewed as "lazy disowning".

@rwgk rwgk marked this pull request as draft March 14, 2021 18:14
@rwgk rwgk changed the title Initial version of virtual_overrider_self_life_support. Adding virtual_overrider_self_life_support. Mar 14, 2021
@rwgk rwgk mentioned this pull request Mar 14, 2021
@rwgk
Copy link
Collaborator Author

rwgk commented Mar 16, 2021

The CI is green, also with PYBIND11_USE_SMART_HOLDER_AS_DEFAULT (#2879). Google-internal testing is also green. Merging here to unblock follow-on PyCLIF work. This change will get reviewed internally. I'll open a new PR if there are requests for changes.

@rwgk rwgk marked this pull request as ready for review March 16, 2021 13:31
@rwgk rwgk merged commit 4697920 into pybind:smart_holder Mar 16, 2021
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Mar 16, 2021
@rwgk rwgk removed the needs changelog Possibly needs a changelog entry label Mar 16, 2021
@rwgk rwgk deleted the sh_virtual_overrider_self_life_support branch March 16, 2021 14:34
@EricCousineau-TRI EricCousineau-TRI added the smart holder See: https://github.com/pybind/pybind11/blob/smart_holder/README_smart_holder.rst label Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
smart holder See: https://github.com/pybind/pybind11/blob/smart_holder/README_smart_holder.rst
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants