diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index a8fd7d917..8aeb79fb7 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -11,11 +11,11 @@ #define PYBIND11_VERSION_MAJOR 2 #define PYBIND11_VERSION_MINOR 8 -#define PYBIND11_VERSION_PATCH 0.dev1 +#define PYBIND11_VERSION_PATCH 0.dev2 // Similar to Python's convention: https://docs.python.org/3/c-api/apiabiversion.html // Additional convention: 0xD = dev -#define PYBIND11_VERSION_HEX 0x020800D1 +#define PYBIND11_VERSION_HEX 0x020800D2 #define PYBIND11_NAMESPACE_BEGIN(name) namespace name { #define PYBIND11_NAMESPACE_END(name) } diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index b177801a1..39c28e773 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -106,7 +106,7 @@ struct internals { std::unordered_map> patients; std::forward_list registered_exception_translators; std::unordered_map shared_data; // Custom data to be shared across extensions - std::vector loader_patient_stack; // Used by `loader_life_support` + std::vector unused_loader_patient_stack_remove_at_v5; std::forward_list static_strings; // Stores the std::strings backing detail::c_str() PyTypeObject *static_property_type; PyTypeObject *default_metaclass; @@ -298,12 +298,12 @@ PYBIND11_NOINLINE internals &get_internals() { #if PY_VERSION_HEX >= 0x03070000 internals_ptr->tstate = PyThread_tss_alloc(); if (!internals_ptr->tstate || (PyThread_tss_create(internals_ptr->tstate) != 0)) - pybind11_fail("get_internals: could not successfully initialize the TSS key!"); + pybind11_fail("get_internals: could not successfully initialize the tstate TSS key!"); PyThread_tss_set(internals_ptr->tstate, tstate); #else internals_ptr->tstate = PyThread_create_key(); if (internals_ptr->tstate == -1) - pybind11_fail("get_internals: could not successfully initialize the TLS key!"); + pybind11_fail("get_internals: could not successfully initialize the tstate TLS key!"); PyThread_set_key_value(internals_ptr->tstate, tstate); #endif internals_ptr->istate = tstate->interp; diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 4c9ac7b8f..fa8433cfe 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -31,47 +31,54 @@ PYBIND11_NAMESPACE_BEGIN(detail) /// A life support system for temporary objects created by `type_caster::load()`. /// Adding a patient will keep it alive up until the enclosing function returns. class loader_life_support { +private: + loader_life_support* parent = nullptr; + std::unordered_set keep_alive; + + static loader_life_support** get_stack_pp() { +#if defined(WITH_THREAD) + thread_local static loader_life_support* per_thread_stack = nullptr; + return &per_thread_stack; +#else + static loader_life_support* global_stack = nullptr; + return &global_stack; +#endif + } + public: /// A new patient frame is created when a function is entered loader_life_support() { - get_internals().loader_patient_stack.push_back(nullptr); + loader_life_support** stack = get_stack_pp(); + parent = *stack; + *stack = this; } /// ... and destroyed after it returns ~loader_life_support() { - auto &stack = get_internals().loader_patient_stack; - if (stack.empty()) + loader_life_support** stack = get_stack_pp(); + if (*stack != this) pybind11_fail("loader_life_support: internal error"); - - auto ptr = stack.back(); - stack.pop_back(); - Py_CLEAR(ptr); - - // A heuristic to reduce the stack's capacity (e.g. after long recursive calls) - if (stack.capacity() > 16 && !stack.empty() && stack.capacity() / stack.size() > 2) - stack.shrink_to_fit(); + *stack = parent; + for (auto* item : keep_alive) + Py_DECREF(item); } /// This can only be used inside a pybind11-bound function, either by `argument_loader` /// at argument preparation time or by `py::cast()` at execution time. PYBIND11_NOINLINE static void add_patient(handle h) { - auto &stack = get_internals().loader_patient_stack; - if (stack.empty()) + loader_life_support* frame = *get_stack_pp(); + if (!frame) { + // NOTE: It would be nice to include the stack frames here, as this indicates + // use of pybind11::cast<> outside the normal call framework, finding such + // a location is challenging. Developers could consider printing out + // stack frame addresses here using something like __builtin_frame_address(0) throw cast_error("When called outside a bound function, py::cast() cannot " "do Python -> C++ conversions which require the creation " "of temporary values"); - - auto &list_ptr = stack.back(); - if (list_ptr == nullptr) { - list_ptr = PyList_New(1); - if (!list_ptr) - pybind11_fail("loader_life_support: error allocating list"); - PyList_SET_ITEM(list_ptr, 0, h.inc_ref().ptr()); - } else { - auto result = PyList_Append(list_ptr, h.ptr()); - if (result == -1) - pybind11_fail("loader_life_support: error adding patient"); } + + if (frame->keep_alive.insert(h.ptr()).second) + Py_INCREF(h.ptr()); } }; diff --git a/pybind11/_version.py b/pybind11/_version.py index 610d39bf1..d212f1dfb 100644 --- a/pybind11/_version.py +++ b/pybind11/_version.py @@ -8,5 +8,5 @@ def _to_int(s): return s -__version__ = "2.8.0.dev1" +__version__ = "2.8.0.dev2" version_info = tuple(_to_int(s) for s in __version__.split(".")) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index d71a51e6a..f014771d5 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -129,6 +129,7 @@ set(PYBIND11_TEST_FILES test_stl.cpp test_stl_binders.cpp test_tagbased_polymorphic.cpp + test_thread.cpp test_union.cpp test_virtual_functions.cpp) diff --git a/tests/test_thread.cpp b/tests/test_thread.cpp new file mode 100644 index 000000000..19d91768b --- /dev/null +++ b/tests/test_thread.cpp @@ -0,0 +1,66 @@ +/* + tests/test_thread.cpp -- call pybind11 bound methods in threads + + Copyright (c) 2021 Laramie Leavitt (Google LLC) + + All rights reserved. Use of this source code is governed by a + BSD-style license that can be found in the LICENSE file. +*/ + +#include +#include + +#include +#include + +#include "pybind11_tests.h" + +namespace py = pybind11; + +namespace { + +struct IntStruct { + explicit IntStruct(int v) : value(v) {}; + ~IntStruct() { value = -value; } + IntStruct(const IntStruct&) = default; + IntStruct& operator=(const IntStruct&) = default; + + int value; +}; + +} // namespace + +TEST_SUBMODULE(thread, m) { + + py::class_(m, "IntStruct").def(py::init([](const int i) { return IntStruct(i); })); + + // implicitly_convertible uses loader_life_support when an implicit + // conversion is required in order to lifetime extend the reference. + // + // This test should be run with ASAN for better effectiveness. + py::implicitly_convertible(); + + m.def("test", [](int expected, const IntStruct &in) { + { + py::gil_scoped_release release; + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } + + if (in.value != expected) { + throw std::runtime_error("Value changed!!"); + } + }); + + m.def( + "test_no_gil", + [](int expected, const IntStruct &in) { + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + if (in.value != expected) { + throw std::runtime_error("Value changed!!"); + } + }, + py::call_guard()); + + // NOTE: std::string_view also uses loader_life_support to ensure that + // the string contents remain alive, but that's a C++ 17 feature. +} diff --git a/tests/test_thread.py b/tests/test_thread.py new file mode 100644 index 000000000..f9db1baba --- /dev/null +++ b/tests/test_thread.py @@ -0,0 +1,44 @@ +# -*- coding: utf-8 -*- + +import threading + +from pybind11_tests import thread as m + + +class Thread(threading.Thread): + def __init__(self, fn): + super(Thread, self).__init__() + self.fn = fn + self.e = None + + def run(self): + try: + for i in range(10): + self.fn(i, i) + except Exception as e: + self.e = e + + def join(self): + super(Thread, self).join() + if self.e: + raise self.e + + +def test_implicit_conversion(): + a = Thread(m.test) + b = Thread(m.test) + c = Thread(m.test) + for x in [a, b, c]: + x.start() + for x in [c, b, a]: + x.join() + + +def test_implicit_conversion_no_gil(): + a = Thread(m.test_no_gil) + b = Thread(m.test_no_gil) + c = Thread(m.test_no_gil) + for x in [a, b, c]: + x.start() + for x in [c, b, a]: + x.join()