[ViderStar]

Description

Fixes #5637.

Passing a NumPy array to a function that takes std::optional<std::reference_wrapper<Eigen::MatrixXd>> segfaults. The same happens for any T that is converted by value (Eigen types, STL containers, strings, custom casters), not only Eigen.

optional_caster::load() loads into a local value_conv inner_caster and then stores cast_op<Value &&>(std::move(inner_caster)) in the optional. For a reference_wrapper<T> that result points at the T owned by the inner caster, which is destroyed when load() returns. The bound function then reads a freed object.

With master, the reproducer from the issue under AddressSanitizer:

ERROR: AddressSanitizer: heap-use-after-free ...
    #4 ... Eigen::DenseBase<Eigen::Matrix<double, -1, -1, 0, -1, -1>>::redux<...>(...) const Redux.h:418

The fix makes the inner caster a member of optional_caster, so it lives as long as the outer caster, i.e. for the whole call, the same way type_caster<std::reference_wrapper<T>> keeps its subcaster. Behaviour for other std::optional<T> arguments does not change.

Testing:

• New test_optional_reference_wrapper_to_converted_value in test_stl. It uses a small by-value caster whose type records which instances are alive, so the check is deterministic and does not rely on reading freed memory. It fails on master ('dangling' == 7) and passes with the fix.
• test_stl.py (C++17): 41 passed, 2 skipped.
• The Eigen reproducer from the issue, built with -fsanitize=address: heap-use-after-free on master, clean with the fix.

variant_caster::load_alternative uses the same local-caster pattern, so std::variant<std::reference_wrapper<T>, ...> probably has the same issue. I left it out to keep this PR small.

Suggested changelog entry:

• Fixed a use-after-free when a std::optional<std::reference_wrapper<T>> argument refers to a type converted by value (e.g. Eigen matrices).

[rwgk]

codex GPT-6.1-Sol ultra:

Found two regressions at stl.h:565:

1. [P2] Preserve lazy construction for None. The member constructs the inner caster before checking for None. For std::optional<py::array_t<double>>, this imports NumPy and allocates an array. Without NumPy installed, f(None) now raises ModuleNotFoundError; the baseline accepts it. Reproducer: repro.cpp below.

2. [P2] Preserve support for nonmovable custom casters. If the inner caster contains nonmovable state, such as std::mutex, the optional caster now becomes nonmovable too. Previously valid obj.cast<std::optional<Item>>() fails compiling at load_type()’s return conv, even when Item is movable. Confirmed against baseline and PR headers. Reproducer: nonmovable_full.cpp below.

Lazy initialization through movable ownership could address both while preserving the lifetime fix.

repro.cpp

nonmovable_full.cpp

[ViderStar]

Thanks @rwgk, both regressions reproduced with the attached files (f(None) raising ModuleNotFoundError without NumPy, and cast<std::optional<Item>>() failing to compile with a nonmovable inner caster).

Fixed in f116885: the inner caster is now held by std::unique_ptr and constructed only when the value is not None, so the optional caster stays movable and the referenced value still lives as long as the caster (#5637 regression test and the ASan reproducer still pass). Added a test with a nonmovable, construction-counting custom caster that covers both the None case and cast<std::optional<T>>().

[henryiii]

Here's a Fable 5.1 review too:

🤖 AI text below 🤖

1. Fix is optional-only. The same local-inner-caster-then-discard pattern exists in variant_caster::load_alternative, list_caster, array_caster, map_caster and set_caster. A std::vector<std::reference_wrapper<std::string>> or a std::variant<std::reference_wrapper<Eigen::MatrixXd>, int> argument still dangles. Either apply the held-subcaster treatment to all container casters, or static_assert against reference_wrapper over non-type_caster_base casters inside containers.

2. Heap allocation on every optional load. inner_caster.reset(new value_conv()) adds malloc/free per call for every non-None std::optional<T>, including std::optional<int>. Suggested: construct the caster in place when it is movable, or only hold the subcaster when Value is a std::reference_wrapper.

3. optional_caster is no longer copyable. The std::unique_ptr member makes type_caster<std::optional<T>> and derived user casters, such as the documented boost::optional one, non-copy-constructible. The changelog entry does not mention this.

4. The py::cast path still dangles. py::cast<std::optional<std::reference_wrapper<T>>>(obj) returns the caster as a temporary, so the result now reads freed heap memory. cast_is_temporary_value_reference does not cover reference_wrapper, so no static_assert fires. The new tests only cover the argument-loader path.

5. Second load with None keeps old state. Loading an int then None on the same caster reports success but leaves value engaged and inner_caster allocated. Pre-existing for value, but the hunk now also retains the stale inner caster. Reset both on the None path.

6. Test has an ABA hole. LiveTracked::is_alive() checks address membership in a static set, so a later object at the same address masks a dangling reference. A generation id would close it. The static set is also unsynchronized.

Items 1, 2 and 4 are the ones that affect the design of the fix.