diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst index 2954411d7b..c4c32f7c38 100644 --- a/docs/advanced/classes.rst +++ b/docs/advanced/classes.rst @@ -1427,4 +1427,12 @@ You can do that using ``py::custom_type_setup``: cls.def("size", &ContainerOwnsPythonObjects::size); cls.def("clear", &ContainerOwnsPythonObjects::clear); +.. note:: + + The ``py::detail::is_holder_constructed()`` guards above are required. During garbage + collection, ``tp_traverse`` and ``tp_clear`` may be handed an instance whose C++ value has + not been constructed yet -- for example one created with ``__new__`` before ``__init__`` + has run. Casting such an instance raises ``ValueError``, and an exception must not be + allowed to escape either of these slots. + .. versionadded:: 2.8 diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 1d857a0ed5..2c517cec69 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -2179,6 +2179,12 @@ class argument_loader { private: static bool load_impl_sequence(function_call &, index_sequence<>) { return true; } + template + bool load_one(function_call &call) { + loader_life_support::argument_load_guard guard(I == 0); + return std::get(argcasters).load(call.args[I], call.args_convert[I]); + } + template bool load_impl_sequence(function_call &call, index_sequence) { PYBIND11_WARNING_PUSH @@ -2187,11 +2193,11 @@ class argument_loader { PYBIND11_WARNING_DISABLE_GCC("-Warray-bounds") #endif #ifdef __cpp_fold_expressions - if ((... || !std::get(argcasters).load(call.args[Is], call.args_convert[Is]))) { + if ((... || !load_one(call))) { return false; } #else - for (bool r : {std::get(argcasters).load(call.args[Is], call.args_convert[Is])...}) { + for (bool r : {load_one(call)...}) { if (!r) { return false; } @@ -2203,6 +2209,7 @@ class argument_loader { template Return call_impl(Func &&f, index_sequence, Guard &&) && { + loader_life_support::old_style_init_call_guard old_style_init_guard; return std::forward(f)(cast_op(std::move(std::get(argcasters)))...); } diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index c8fff5c144..179f84e925 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -18,16 +18,16 @@ // See also: https://github.com/python/cpython/blob/HEAD/Include/patchlevel.h /* -- start version constants -- */ #define PYBIND11_VERSION_MAJOR 3 -#define PYBIND11_VERSION_MINOR 1 +#define PYBIND11_VERSION_MINOR 2 #define PYBIND11_VERSION_MICRO 0 // ALPHA = 0xA, BETA = 0xB, GAMMA = 0xC (release candidate), FINAL = 0xF (stable release) // - The release level is set to "alpha" for development versions. // Use 0xA0 (LEVEL=0xA, SERIAL=0) for development versions. // - For stable releases, set the serial to 0. -#define PYBIND11_VERSION_RELEASE_LEVEL PY_RELEASE_LEVEL_FINAL +#define PYBIND11_VERSION_RELEASE_LEVEL PY_RELEASE_LEVEL_ALPHA #define PYBIND11_VERSION_RELEASE_SERIAL 0 // String version of (micro, release level, release serial), e.g.: 0a0, 0b1, 0rc1, 0 -#define PYBIND11_VERSION_PATCH 0 +#define PYBIND11_VERSION_PATCH 0a0 /* -- end version constants -- */ #if !defined(Py_PACK_FULL_VERSION) @@ -632,7 +632,9 @@ struct nonsimple_values_and_holders { uint8_t *status; }; -/// The 'instance' type which needs to be standard layout (need to be able to use 'offsetof') +/// The 'instance' type which needs to be standard layout (need to be able to use 'offsetof'). +/// Changes to this struct or to the semantics of its members require incrementing +/// `PYBIND11_INTERNALS_VERSION`. struct instance { PyObject_HEAD /// Storage for pointers and holder; see simple_layout, below, for a description @@ -676,6 +678,8 @@ struct instance { bool has_patients : 1; /// If true, this Python object needs to be kept alive for the lifetime of the C++ value. bool is_alias : 1; + /// For simple layout, tracks whether a constructor is currently constructing the C++ value. + bool simple_value_constructing : 1; /// Initializes all of the above type/values/holders data (but not the instance values /// themselves) @@ -693,6 +697,7 @@ struct instance { /// Bit values for the non-simple status flags static constexpr uint8_t status_holder_constructed = 1; static constexpr uint8_t status_instance_registered = 2; + static constexpr uint8_t status_value_constructing = 4; }; static_assert(std::is_standard_layout::value, diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 295485ffab..83b05a7e05 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -39,11 +39,11 @@ /// further ABI-incompatible changes may be made before the ABI is officially /// changed to the new version. #ifndef PYBIND11_INTERNALS_VERSION -# define PYBIND11_INTERNALS_VERSION 12 +# define PYBIND11_INTERNALS_VERSION 13 #endif -#if PYBIND11_INTERNALS_VERSION < 12 -# error "PYBIND11_INTERNALS_VERSION 12 is the minimum for all platforms for pybind11 v3.1.0" +#if PYBIND11_INTERNALS_VERSION < 13 +# error "PYBIND11_INTERNALS_VERSION 13 is the minimum for all platforms for pybind11 v3.2.0" #endif PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 161b9884fa..bf6175162b 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -9,6 +9,7 @@ #pragma once +#include #include #include #include @@ -61,9 +62,37 @@ class loader_life_support { loader_life_support *parent = nullptr; std::unordered_set keep_alive; + // Old-style placement-new constructors need raw storage while loading their `self` + // argument. Keep it private to the exact overload candidate until its C++ callable returns. + value_and_holder *old_style_init_self = nullptr; + void *old_style_init_storage = nullptr; + bool old_style_init_self_load_allowed = false; + bool old_style_init_self_load_claimed = false; + + static bool is_same_value_and_holder(const value_and_holder &lhs, + const value_and_holder &rhs) { + return lhs.inst == rhs.inst && lhs.vh == rhs.vh; + } + + void cleanup_old_style_init_storage() { + if (old_style_init_storage == nullptr) { + return; + } + auto v_h = *old_style_init_self; + scoped_critical_section lock( + handle(reinterpret_cast(old_style_init_self->inst))); + if (v_h.value_ptr() != nullptr) { + pybind11_fail("loader_life_support: old-style constructor storage collision"); + } + v_h.value_ptr() = old_style_init_storage; + old_style_init_storage = nullptr; + v_h.type->dealloc(v_h); // Frees the storage and nulls the value pointer. + } + public: /// A new patient frame is created when a function is entered - loader_life_support() { + explicit loader_life_support(value_and_holder *old_style_init_self = nullptr) + : old_style_init_self(old_style_init_self) { auto &frame = tls_current_frame(); parent = frame; frame = this; @@ -76,11 +105,118 @@ class loader_life_support { pybind11_fail("loader_life_support: internal error"); } frame = parent; + cleanup_old_style_init_storage(); for (auto *item : keep_alive) { Py_DECREF(item); } } + /// Restricts the special old-style constructor permission to argument zero of the current + /// candidate. A nested bound call has its own loader frame and cannot inherit this permission. + class argument_load_guard { + public: + explicit argument_load_guard(bool is_first_argument) { + auto *current = tls_current_frame(); + if (is_first_argument && current != nullptr && current->old_style_init_self != nullptr + && !current->old_style_init_self_load_claimed) { + frame = current; + frame->old_style_init_self_load_allowed = true; + } + } + ~argument_load_guard() { + if (frame != nullptr) { + frame->old_style_init_self_load_allowed = false; + } + } + argument_load_guard(const argument_load_guard &) = delete; + argument_load_guard &operator=(const argument_load_guard &) = delete; + + private: + loader_life_support *frame = nullptr; + }; + + /// Some legacy `__setstate__` implementations accept `self` as `py::object` and perform the + /// typed cast inside the C++ callable. At that point all other arguments have finished + /// loading, so granting the same exact, one-shot permission is safe. Reentrant bound calls + /// still get a separate loader frame. + class old_style_init_call_guard { + public: + old_style_init_call_guard() { + auto *current = tls_current_frame(); + if (current != nullptr && current->old_style_init_self != nullptr + && !current->old_style_init_self_load_claimed) { + frame = current; + frame->old_style_init_self_load_allowed = true; + } + } + ~old_style_init_call_guard() { + if (frame != nullptr) { + frame->old_style_init_self_load_allowed = false; + } + } + old_style_init_call_guard(const old_style_init_call_guard &) = delete; + old_style_init_call_guard &operator=(const old_style_init_call_guard &) = delete; + + private: + loader_life_support *frame = nullptr; + }; + + /// Claims and allocates the private storage for the exact old-style constructor `self` load. + /// The permission is consumed before invoking a potentially user-defined operator new. + static bool try_reserve_old_style_init_storage(value_and_holder &v_h, + const type_info *type, + void *&value) { + auto *frame = tls_current_frame(); + if (frame == nullptr || !frame->old_style_init_self_load_allowed + || frame->old_style_init_self_load_claimed || frame->old_style_init_self == nullptr + || !is_same_value_and_holder(v_h, *frame->old_style_init_self) + || v_h.value_ptr() != nullptr) { + return false; + } + + frame->old_style_init_self_load_claimed = true; + frame->old_style_init_self_load_allowed = false; + if (type->operator_new) { + frame->old_style_init_storage = type->operator_new(type->type_size); + } else { +#if defined(__cpp_aligned_new) + if (type->type_align > __STDCPP_DEFAULT_NEW_ALIGNMENT__) { + frame->old_style_init_storage + = ::operator new(type->type_size, std::align_val_t(type->type_align)); + } else { + frame->old_style_init_storage = ::operator new(type->type_size); + } +#else + frame->old_style_init_storage = ::operator new(type->type_size); +#endif + } + if (frame->old_style_init_storage == nullptr) { + throw std::bad_alloc(); + } + value = frame->old_style_init_storage; + return true; + } + + /// Publishes a successfully placement-constructed value and immediately finalizes its holder. + /// This runs after the C++ callable, but before return-value conversion and post-call + /// policies. + static void complete_old_style_init() { + auto *frame = tls_current_frame(); + if (frame == nullptr || frame->old_style_init_storage == nullptr) { + return; + } + + auto v_h = *frame->old_style_init_self; + scoped_critical_section lock(handle(reinterpret_cast(v_h.inst))); + if (!v_h.value_constructing() || v_h.value_ptr() != nullptr) { + pybind11_fail("loader_life_support: invalid old-style constructor commit"); + } + v_h.value_ptr() = frame->old_style_init_storage; + frame->old_style_init_storage = nullptr; + v_h.type->init_instance(v_h.inst, nullptr); + v_h.set_value_constructing(false); + } + /// Keep `h` alive until the current patient frame is destroyed, if there is one. /// Returns false when called outside a bound function (no frame). Use this, rather /// than `add_patient`, when failing to register is acceptable because the caller @@ -525,6 +661,7 @@ PYBIND11_NOINLINE void instance::allocate_layout() { = reinterpret_cast(&nonsimple.values_and_holders[flags_at]); } owned = true; + simple_value_constructing = false; } // NOLINTNEXTLINE(readability-make-member-function-const) @@ -534,6 +671,59 @@ PYBIND11_NOINLINE void instance::deallocate_layout() { } } +/// Marks the exact value slot targeted by a constructor. This state is never permission to load +/// the value: every load is rejected until construction finishes, apart from the one-shot +/// old-style constructor `self` permission maintained by `loader_life_support`. +class instance_construction_scope { +public: + explicit instance_construction_scope(value_and_holder *v_h) { + if (v_h == nullptr) { + return; + } + v_h_ = *v_h; + started_ = false; + scoped_critical_section lock(handle(reinterpret_cast(v_h_.inst))); + if (v_h_.value_constructing()) { + return; + } + if (v_h_.instance_registered()) { + already_registered_ = true; + return; + } + value_was_null_ = v_h_.value_ptr() == nullptr; + v_h_.set_value_constructing(); + started_ = true; + } + ~instance_construction_scope() { + if (!started_ || v_h_.inst == nullptr) { + return; + } + + scoped_critical_section lock(handle(reinterpret_cast(v_h_.inst))); + // A failed new-style constructor can have published a value without completing its + // holder. Preserve the existing cleanup guarantee for that case. + if (value_was_null_ && !v_h_.holder_constructed() && v_h_.value_ptr() != nullptr) { + if (v_h_.instance_registered()) { + deregister_instance(v_h_.inst, v_h_.value_ptr(), v_h_.type); + v_h_.set_instance_registered(false); + } + v_h_.type->dealloc(v_h_); + } + v_h_.set_value_constructing(false); + } + instance_construction_scope(const instance_construction_scope &) = delete; + instance_construction_scope &operator=(const instance_construction_scope &) = delete; + + bool started() const { return started_; } + bool already_registered() const { return already_registered_; } + +private: + value_and_holder v_h_; + bool started_ = true; + bool already_registered_ = false; + bool value_was_null_ = false; +}; + PYBIND11_NOINLINE bool isinstance_generic(handle obj, const std::type_info &tp) { handle type = detail::get_type_handle(tp, false); if (!type) { @@ -1128,6 +1318,25 @@ class type_caster_generic { // Base methods for generic caster; there are overridden in copyable_holder_caster void load_value(value_and_holder &&v_h) { + scoped_critical_section lock(handle(reinterpret_cast(v_h.inst))); + + // A non-null value pointer is not sufficient while a constructor is running: old-style + // placement-new storage may exist before the C++ object's lifetime has begun. Only the + // exact argument-zero load of the current old-style constructor may access private raw + // storage; reentrant, cross-base, nested, and cross-thread loads must all fail. + if (v_h.value_constructing()) { + void *reserved_value = nullptr; + const auto *type = v_h.type ? v_h.type : typeinfo; + if (loader_life_support::try_reserve_old_style_init_storage( + v_h, type, reserved_value)) { + value = reserved_value; + return; + } + throw value_error("Missing value for wrapped C++ type `" + + clean_type_id(cpptype->name()) + + "`: Python instance is still being constructed."); + } + if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) { smart_holder_type_caster_support::value_and_holder_helper v_h_helper; v_h_helper.loaded_v_h = v_h; @@ -1138,22 +1347,12 @@ class type_caster_generic { } } auto *&vptr = v_h.value_ptr(); - // Lazy allocation for unallocated values: if (vptr == nullptr) { - const auto *type = v_h.type ? v_h.type : typeinfo; - if (type->operator_new) { - vptr = type->operator_new(type->type_size); - } else { -#if defined(__cpp_aligned_new) - if (type->type_align > __STDCPP_DEFAULT_NEW_ALIGNMENT__) { - vptr = ::operator new(type->type_size, std::align_val_t(type->type_align)); - } else { - vptr = ::operator new(type->type_size); - } -#else - vptr = ::operator new(type->type_size); -#endif - } + throw value_error("Missing value for wrapped C++ type `" + + clean_type_id(cpptype->name()) + + "`: Python instance is uninitialized: the C++ object was " + "never constructed (`__init__()` was bypassed, e.g. by " + "calling `__new__()` directly)."); } value = vptr; } diff --git a/include/pybind11/detail/value_and_holder.h b/include/pybind11/detail/value_and_holder.h index b24551e678..3c1442aba6 100644 --- a/include/pybind11/detail/value_and_holder.h +++ b/include/pybind11/detail/value_and_holder.h @@ -74,6 +74,22 @@ struct value_and_holder { &= static_cast(~instance::status_instance_registered); } } + bool value_constructing() const { + return inst->simple_layout + ? inst->simple_value_constructing + : ((inst->nonsimple.status[index] & instance::status_value_constructing) != 0); + } + // NOLINTNEXTLINE(readability-make-member-function-const) + void set_value_constructing(bool v = true) { + if (inst->simple_layout) { + inst->simple_value_constructing = v; + } else if (v) { + inst->nonsimple.status[index] |= instance::status_value_constructing; + } else { + inst->nonsimple.status[index] + &= static_cast(~instance::status_value_constructing); + } + } }; // This is a semi-public API to check if the corresponding instance has been constructed with a diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index f57514ae28..ae4c45a802 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -513,10 +513,13 @@ class cpp_function : public function { handle result; if (call.func.is_setter) { (void) std::move(args_converter).template call(f); + loader_life_support::complete_old_style_init(); result = none().release(); } else { + auto &&cpp_result = std::move(args_converter).template call(f); + loader_life_support::complete_old_style_init(); result = cast_out::cast( - std::move(args_converter).template call(f), policy, call.parent); + std::forward(cpp_result), policy, call.parent); } return result; @@ -993,14 +996,29 @@ class cpp_function : public function { = get_type_info(reinterpret_cast(overloads->scope.ptr())); auto *const pi = reinterpret_cast(parent.ptr()); self_value_and_holder = pi->get_value_and_holder(tinfo, true); + } - // If this value is already registered it must mean __init__ is invoked multiple times; - // we really can't support that in C++, so just ignore the second __init__. - if (self_value_and_holder.instance_registered()) { + detail::instance_construction_scope construction_scope( + overloads->is_constructor ? &self_value_and_holder : nullptr); + if (overloads->is_constructor) { + // Invoking __init__ repeatedly on an already constructed value remains a no-op. + if (construction_scope.already_registered()) { return none().release().ptr(); } + if (!construction_scope.started()) { + set_error(PyExc_ValueError, + "Cannot initialize a wrapped C++ value while it is already being " + "constructed"); + return nullptr; + } } + // On free-threaded Python, serialize the complete constructor transaction. Python + // critical sections are suspended around blocking operations, allowing another thread to + // enter, observe `value_constructing`, and reject access without racing status-byte + // updates. + scoped_critical_section constructor_lock(overloads->is_constructor ? parent : handle{}); + try { // We do this in two passes: in the first pass, we load arguments with `convert=false`; // in the second, we allow conversion (except for arguments with an explicit @@ -1060,8 +1078,9 @@ class cpp_function : public function { // 0. Inject new-style `self` argument if (func.is_new_style_constructor) { - // The `value` may have been preallocated by an old-style `__init__` - // if it was a preceding candidate for overload resolution. + // Retain cleanup for a value partially published by a preceding failed + // new-style candidate. Old-style reservations are private and are cleaned up + // with their loader frame before another candidate is tried. if (self_value_and_holder) { self_value_and_holder.type->dealloc(self_value_and_holder); } @@ -1232,7 +1251,9 @@ class cpp_function : public function { // 6. Call the function. try { - loader_life_support guard{}; + loader_life_support guard{func.is_constructor && !func.is_new_style_constructor + ? &self_value_and_holder + : nullptr}; result = func.impl(call); } catch (reference_cast_error &) { result = PYBIND11_TRY_NEXT_OVERLOAD; @@ -1263,7 +1284,10 @@ class cpp_function : public function { // allowed for (auto &call : second_pass) { try { - loader_life_support guard{}; + loader_life_support guard{call.func.is_constructor + && !call.func.is_new_style_constructor + ? &self_value_and_holder + : nullptr}; result = call.func.impl(call); } catch (reference_cast_error &) { result = PYBIND11_TRY_NEXT_OVERLOAD; diff --git a/tests/test_class.cpp b/tests/test_class.cpp index e520f29ec5..47e363b1d3 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -77,6 +77,35 @@ static_assert(!py::detail::is_same_or_base_of< test_class::pr5396_forward_declared_class::ForwardClass>::value, ""); +// test_new_bypasses_init +struct NewNoInit { + int m_data; + explicit NewNoInit(int data) : m_data(data) {} + NewNoInit(const NewNoInit &) = default; + virtual ~NewNoInit() = default; + int data() const { return m_data; } + // Virtual on purpose: using a not-yet-constructed instance reads the vtable pointer out of + // uninitialized storage, which segfaults rather than merely returning a garbage value. + virtual int v_data() const { return m_data; } +}; + +// test_failed_old_style_init_does_not_leave_lazy_storage +struct OldStyleInit { + int m_data; + explicit OldStyleInit(int data) : m_data(data) {} + virtual ~OldStyleInit() = default; + int data() const { return m_data; } + virtual int v_data() const { return m_data; } +}; + +// test_reentrant_load_during_mixed_style_init +struct MixedStyleInit { + int m_data; + explicit MixedStyleInit(int data) : m_data(data) {} + explicit MixedStyleInit(const std::string &data) : m_data(static_cast(data.size())) {} + int data() const { return m_data; } +}; + TEST_SUBMODULE(class_, m) { m.def("obj_class_name", [](py::handle obj) { return py::detail::obj_class_name(obj.ptr()); }); @@ -597,6 +626,55 @@ TEST_SUBMODULE(class_, m) { m.def("return_universal_recipient", []() -> test_class::ConvertibleFromAnything { return test_class::ConvertibleFromAnything{}; }); + + py::class_(m, "NewNoInit") + .def(py::init()) + .def("data", &NewNoInit::data) + .def("v_data", &NewNoInit::v_data) + .def(py::pickle([](const NewNoInit &p) { return py::make_tuple(p.m_data); }, + [](const py::tuple &t) { + if (t.size() != 1) { + throw std::runtime_error("Invalid state!"); + } + return NewNoInit(t[0].cast()); + })); + + py::class_ old_style_init(m, "OldStyleInit"); + ignoreOldStyleInitWarnings([&old_style_init]() { + old_style_init + .def("__init__", + [](OldStyleInit &self, int x) { + if (x < 0) { + throw std::runtime_error("negative data"); + } + new (&self) OldStyleInit(x); + }) + .def("__setstate__", [](const py::object &self_obj, const py::object &state) { + // Old-style callbacks taking a Python self perform the one authorized self cast + // inside the callable. Keep state conversion ahead of placement-new: Python + // executed by that cast must not be able to load the reserved storage again. + auto &self = self_obj.cast(); + int x = state.cast(); + new (&self) OldStyleInit(x); + }); + }); + old_style_init.def("data", &OldStyleInit::data).def("v_data", &OldStyleInit::v_data); + + py::class_ mixed_style_init(m, "MixedStyleInit"); + mixed_style_init.def(py::init()); + ignoreOldStyleInitWarnings([&mixed_style_init]() { + mixed_style_init.def("__init__", [](MixedStyleInit &self, const std::string &value) { + new (&self) MixedStyleInit(value); + }); + }); + mixed_style_init.def("data", &MixedStyleInit::data); + + // These functions intentionally do not dereference their arguments. They let the Python + // tests probe whether a type caster accepted reserved or uninitialized storage without + // invoking undefined behavior when testing a broken implementation. + m.def("accept_new_no_init", [](NewNoInit *value) { return value != nullptr; }); + m.def("accept_old_style_init", [](OldStyleInit *value) { return value != nullptr; }); + m.def("accept_mixed_style_init", [](MixedStyleInit *value) { return value != nullptr; }); } template diff --git a/tests/test_class.py b/tests/test_class.py index 201c7e339e..0fa54f730e 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -1,7 +1,9 @@ from __future__ import annotations import gc +import pickle import sys +import threading from unittest import mock import pytest @@ -251,6 +253,236 @@ def __init__(self): assert msg(exc_info.value) == expected +def _record_uninitialized_load(seen, function, obj): + try: + seen["accepted"] = function(obj) + except ValueError as exc: + seen["error"] = exc + + +def _assert_uninitialized_load_rejected(seen): + assert "accepted" not in seen, "type caster accepted unconstructed storage" + assert isinstance(seen.get("error"), ValueError) + + +def test_new_bypasses_init(): + """`__new__` allocates the Python object but not the C++ one; using the instance before + `__init__` has run must raise instead of segfaulting.""" + + class PythonDerived(m.NewNoInit): + pass + + for cls in (m.NewNoInit, PythonDerived): + obj = cls.__new__(cls) + with pytest.raises(ValueError) as exc_info: + m.accept_new_no_init(obj) + assert "Python instance is uninitialized" in str(exc_info.value) + assert "NewNoInit" in str(exc_info.value) + + # Calling `__init__()` is the sanctioned way to finish an object made with `__new__()`. + obj.__init__(42) + assert obj.data() == 42 + assert obj.v_data() == 42 + + +def test_new_then_setstate(): + """`__new__` must not be blocked: pickle relies on it, and `__setstate__` finishes the + object off. This walks the protocol by hand, then checks the real thing.""" + real_obj = m.NewNoInit(42) + assert real_obj.data() == 42 + state = real_obj.__getstate__() + + obj = m.NewNoInit.__new__(m.NewNoInit) # NEWOBJ + obj.__setstate__(state) # BUILD + assert obj.data() == 42 + assert obj.v_data() == 42 + + for protocol in range(2, pickle.HIGHEST_PROTOCOL + 1): + assert pickle.loads(pickle.dumps(m.NewNoInit(7), protocol)).v_data() == 7 + + +def test_failed_old_style_init_does_not_leave_lazy_storage(): + """If an old-style placement-new `__init__` throws before constructing the value, the + lazily allocated storage must not linger: later use must still raise, not segfault.""" + obj = m.OldStyleInit.__new__(m.OldStyleInit) + with pytest.raises(RuntimeError, match="negative data"): + obj.__init__(-1) + + # The failed __init__ already reserved storage for `self`. Without cleanup, a later load can + # mistake that storage for a constructed C++ object. + with pytest.raises(ValueError, match="uninitialized"): + m.accept_old_style_init(obj) + + # A successful retry is still allowed. + obj.__init__(42) + assert obj.v_data() == 42 + + +def test_reentrant_load_during_new_style_init(): + """New-style constructors never need lazy allocation, so passing the half-built instance + to another bound function while `__init__` runs must raise, not hand out garbage.""" + obj = m.NewNoInit.__new__(m.NewNoInit) + seen = {} + + class Evil: + def __index__(self): + # Runs during int conversion of the constructor argument while the C++ value is + # unconstructed. A new-style constructor never authorizes lazy allocation for it. + _record_uninitialized_load(seen, m.accept_new_no_init, obj) + raise TypeError("stop the constructor") + + with pytest.raises(TypeError): + obj.__init__(Evil()) + + _assert_uninitialized_load_rejected(seen) + + +def test_reentrant_load_during_old_style_init_argument_conversion(): + """Only the old-style constructor's own `self` load may reserve storage. Python called while + converting a later argument must not be able to load that storage as a C++ object.""" + obj = m.OldStyleInit.__new__(m.OldStyleInit) + seen = {} + + class ReenterThenFail: + def __index__(self): + _record_uninitialized_load(seen, m.accept_old_style_init, obj) + raise TypeError("stop the constructor") + + with pytest.raises(TypeError): + obj.__init__(ReenterThenFail()) + + _assert_uninitialized_load_rejected(seen) + with pytest.raises(ValueError): + m.accept_old_style_init(obj) + + # Failed conversion must release the reservation and leave a successful retry possible. + obj.__init__(42) + assert obj.data() == 42 + + +def test_reentrant_load_during_mixed_style_init(): + """An old-style overload must not authorize loads while a new-style candidate in the same + overload chain is being tried.""" + obj = m.MixedStyleInit.__new__(m.MixedStyleInit) + seen = {} + + class Reenter: + def __index__(self): + _record_uninitialized_load(seen, m.accept_mixed_style_init, obj) + return 42 + + obj.__init__(Reenter()) + _assert_uninitialized_load_rejected(seen) + assert obj.data() == 42 + + # Also retain coverage that the old-style overload itself remains usable. + assert m.MixedStyleInit("four").data() == 4 + + +def test_reentrant_load_during_old_style_setstate(): + """Old-style `__setstate__` may reserve storage for `self`, but Python executed inside its + callback before placement-new must still see the instance as unconstructed.""" + obj = m.OldStyleInit.__new__(m.OldStyleInit) + seen = {} + + class Reenter: + def __index__(self): + _record_uninitialized_load(seen, m.accept_old_style_init, obj) + return 43 + + obj.__setstate__(Reenter()) + _assert_uninitialized_load_rejected(seen) + assert obj.data() == 43 + + +def test_nested_old_style_init_is_rejected(): + """A nested initializer for the same reserved value must be rejected before its placement-new + callback runs; the outer initializer can then complete normally.""" + obj = m.OldStyleInit.__new__(m.OldStyleInit) + seen = {} + + class Reenter: + def __index__(self): + try: + obj.__init__(-1) + except Exception as exc: + seen["error"] = exc + else: + seen["accepted"] = True + return 44 + + obj.__init__(Reenter()) + assert "accepted" not in seen + assert isinstance(seen.get("error"), ValueError) + assert obj.data() == 44 + + +def test_old_style_init_does_not_authorize_another_python_mi_base(): + """Construction permission is for one exact value-and-holder, not every C++ base slot in the + same Python multiple-inheritance instance.""" + + class PythonMI(m.OldStyleInit, m.NewNoInit): + pass + + obj = PythonMI.__new__(PythonMI) + seen = {} + + class Reenter: + def __index__(self): + _record_uninitialized_load(seen, m.accept_new_no_init, obj) + return 45 + + m.OldStyleInit.__init__(obj, Reenter()) + _assert_uninitialized_load_rejected(seen) + assert obj.data() == 45 + + # Loading the unrelated base must remain rejected after the first base finishes construction. + with pytest.raises(ValueError): + m.accept_new_no_init(obj) + + +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") +def test_old_style_init_does_not_authorize_another_thread(): + """While one thread is converting a later constructor argument, another thread must not load + the reserved storage. Events make the interleaving bounded and deterministic with or without + the GIL.""" + obj = m.OldStyleInit.__new__(m.OldStyleInit) + conversion_entered = threading.Event() + allow_conversion_to_finish = threading.Event() + thread_errors = [] + + class BlockingIndex: + def __index__(self): + conversion_entered.set() + if not allow_conversion_to_finish.wait(timeout=10): + raise RuntimeError("timed out waiting to finish conversion") + return 46 + + def initialize(): + try: + obj.__init__(BlockingIndex()) + except BaseException as exc: + thread_errors.append(exc) + + thread = threading.Thread(target=initialize) + thread.start() + try: + assert conversion_entered.wait(timeout=10), ( + "constructor did not enter conversion" + ) + with pytest.raises(ValueError): + m.accept_old_style_init(obj) + with pytest.raises(ValueError): + obj.__init__(47) + finally: + allow_conversion_to_finish.set() + thread.join(timeout=10) + + assert not thread.is_alive(), "constructor thread did not finish" + assert not thread_errors + assert obj.data() == 46 + + @pytest.mark.parametrize( "mock_return_value", [None, (1, 2, 3), m.Pet("Polly", "parrot"), m.Dog("Molly")] )