pybind11 icon indicating copy to clipboard operation
pybind11 copied to clipboard

[BUG]: keep_alive does not applied to properties

Open jnooree opened this issue 1 year ago • 0 comments

Required prerequisites

  • [X] Make sure you've read the documentation. Your issue may be addressed there.
  • [X] Search the issue tracker and Discussions to verify that this hasn't already been reported. +1 or comment there if it has.
  • [X] Consider asking first in the Gitter chat room or in a Discussion.

What version (or hash if on master) of pybind11 are you using?

2.10.4, but 8b48ff8 (the current master) still has the same problem.

Problem description

The problem

Passing keep_alive directly to def_property* family is ignored. The flow:

  1. The getter/setters are created without "extra" attributes. Related source locations: https://github.com/pybind/pybind11/blob/8b48ff878c168b51fe5ef7b8c728815b9e1a9857/include/pybind11/pybind11.h#L1750 https://github.com/pybind/pybind11/blob/8b48ff878c168b51fe5ef7b8c728815b9e1a9857/include/pybind11/pybind11.h#L1766 https://github.com/pybind/pybind11/blob/8b48ff878c168b51fe5ef7b8c728815b9e1a9857/include/pybind11/pybind11.h#L1782-L1783 https://github.com/pybind/pybind11/blob/8b48ff878c168b51fe5ef7b8c728815b9e1a9857/include/pybind11/pybind11.h#L1790-L1794 https://github.com/pybind/pybind11/blob/8b48ff878c168b51fe5ef7b8c728815b9e1a9857/include/pybind11/pybind11.h#L1812-L1813
  2. All the calls to def_properties* eventually forwarded to def_property_static, and "extra" attributes are processed there, with process_attributes<Extra...>::init: https://github.com/pybind/pybind11/blob/8b48ff878c168b51fe5ef7b8c728815b9e1a9857/include/pybind11/pybind11.h#L1816-L1848
  3. ... which in turn calls process_attribute<keep_alive<>>::init: https://github.com/pybind/pybind11/blob/8b48ff878c168b51fe5ef7b8c728815b9e1a9857/include/pybind11/attr.h#L644-L650
  4. Unfortunately, the process_attribute<keep_alive<>>::init is empty. https://github.com/pybind/pybind11/blob/8b48ff878c168b51fe5ef7b8c728815b9e1a9857/include/pybind11/attr.h#L619-L639

Possible solution

Just blindly pass "extra" attributes to the cpp_function constructor. I haven't yet analyzed possible side effects of this solution.

Workaround

keep_alive works when it is passed to the cpp_function constructor directly. If the maintainers choose not to update implementations, this (confusing) behavior should be documented IMO.

Related issues

#4236, #4124, #2618, ...

Reproducible example code

No response

Is this a regression? Put the last known working version here if it is.

Not a regression

jnooree avatar Mar 04 '24 05:03 jnooree