diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 88e6d60a9..f501467de 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -312,7 +312,31 @@ inline void traverse_offset_bases(void *valueptr, } } +#ifdef Py_GIL_DISABLED +inline void enable_try_inc_ref(PyObject *obj) { + // TODO: Replace with PyUnstable_Object_EnableTryIncRef when available. + // See https://github.com/python/cpython/issues/128844 + if (_Py_IsImmortal(obj)) { + return; + } + for (;;) { + Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&obj->ob_ref_shared); + if ((shared & _Py_REF_SHARED_FLAG_MASK) != 0) { + // Nothing to do if it's in WEAKREFS, QUEUED, or MERGED states. + return; + } + if (_Py_atomic_compare_exchange_ssize( + &obj->ob_ref_shared, &shared, shared | _Py_REF_MAYBE_WEAKREF)) { + return; + } + } +} +#endif + inline bool register_instance_impl(void *ptr, instance *self) { +#ifdef Py_GIL_DISABLED + enable_try_inc_ref(reinterpret_cast(self)); +#endif with_instance_map(ptr, [&](instance_map &instances) { instances.emplace(ptr, self); }); return true; // unused, but gives the same signature as the deregister func } diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index d5d86dc6c..e2eb825d1 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -241,6 +241,49 @@ PYBIND11_NOINLINE handle get_type_handle(const std::type_info &tp, bool throw_if return handle(type_info ? ((PyObject *) type_info->type) : nullptr); } +inline bool try_incref(PyObject *obj) { + // Tries to increment the reference count of an object if it's not zero. + // TODO: Use PyUnstable_TryIncref when available. + // See https://github.com/python/cpython/issues/128844 +#ifdef Py_GIL_DISABLED + // See + // https://github.com/python/cpython/blob/d05140f9f77d7dfc753dd1e5ac3a5962aaa03eff/Include/internal/pycore_object.h#L761 + uint32_t local = _Py_atomic_load_uint32_relaxed(&obj->ob_ref_local); + local += 1; + if (local == 0) { + // immortal + return true; + } + if (_Py_IsOwnedByCurrentThread(obj)) { + _Py_atomic_store_uint32_relaxed(&obj->ob_ref_local, local); +# ifdef Py_REF_DEBUG + _Py_INCREF_IncRefTotal(); +# endif + return true; + } + Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&obj->ob_ref_shared); + for (;;) { + // If the shared refcount is zero and the object is either merged + // or may not have weak references, then we cannot incref it. + if (shared == 0 || shared == _Py_REF_MERGED) { + return false; + } + + if (_Py_atomic_compare_exchange_ssize( + &obj->ob_ref_shared, &shared, shared + (1 << _Py_REF_SHARED_SHIFT))) { +# ifdef Py_REF_DEBUG + _Py_INCREF_IncRefTotal(); +# endif + return true; + } + } +#else + assert(Py_REFCNT(obj) > 0); + Py_INCREF(obj); + return true; +#endif +} + // Searches the inheritance graph for a registered Python instance, using all_type_info(). PYBIND11_NOINLINE handle find_registered_python_instance(void *src, const detail::type_info *tinfo) { @@ -249,7 +292,10 @@ PYBIND11_NOINLINE handle find_registered_python_instance(void *src, for (auto it_i = it_instances.first; it_i != it_instances.second; ++it_i) { for (auto *instance_type : detail::all_type_info(Py_TYPE(it_i->second))) { if (instance_type && same_type(*instance_type->cpptype, *tinfo->cpptype)) { - return handle((PyObject *) it_i->second).inc_ref(); + auto *wrapper = reinterpret_cast(it_i->second); + if (try_incref(wrapper)) { + return handle(wrapper); + } } } } diff --git a/tests/test_thread.cpp b/tests/test_thread.cpp index e727109d7..eabf39afa 100644 --- a/tests/test_thread.cpp +++ b/tests/test_thread.cpp @@ -28,6 +28,9 @@ struct IntStruct { int value; }; +struct EmptyStruct {}; +EmptyStruct SharedInstance; + } // namespace TEST_SUBMODULE(thread, m) { @@ -61,6 +64,9 @@ TEST_SUBMODULE(thread, m) { }, py::call_guard()); + py::class_(m, "EmptyStruct") + .def_readonly_static("SharedInstance", &SharedInstance); + // 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 index f12020e41..e9d7bafb2 100644 --- a/tests/test_thread.py +++ b/tests/test_thread.py @@ -47,3 +47,22 @@ def test_implicit_conversion_no_gil(): x.start() for x in [c, b, a]: x.join() + + +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") +def test_bind_shared_instance(): + nb_threads = 4 + b = threading.Barrier(nb_threads) + + def access_shared_instance(): + b.wait() + for _ in range(1000): + m.EmptyStruct.SharedInstance # noqa: B018 + + threads = [ + threading.Thread(target=access_shared_instance) for _ in range(nb_threads) + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join()