From 5c07feef2f9b7429cd5aab2802cfbaed33eca63c Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 6 Nov 2024 11:19:25 -0800 Subject: [PATCH 1/4] chore(deps): update pre-commit hooks (#5432) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore(deps): update pre-commit hooks updates: - [github.com/pre-commit/mirrors-clang-format: v18.1.8 → v19.1.3](https://github.com/pre-commit/mirrors-clang-format/compare/v18.1.8...v19.1.3) - [github.com/astral-sh/ruff-pre-commit: v0.6.3 → v0.7.2](https://github.com/astral-sh/ruff-pre-commit/compare/v0.6.3...v0.7.2) - [github.com/pre-commit/mirrors-mypy: v1.11.2 → v1.13.0](https://github.com/pre-commit/mirrors-mypy/compare/v1.11.2...v1.13.0) - [github.com/pre-commit/pre-commit-hooks: v4.6.0 → v5.0.0](https://github.com/pre-commit/pre-commit-hooks/compare/v4.6.0...v5.0.0) - [github.com/adamchainz/blacken-docs: 1.18.0 → 1.19.1](https://github.com/adamchainz/blacken-docs/compare/1.18.0...1.19.1) - [github.com/mgedmin/check-manifest: 0.49 → 0.50](https://github.com/mgedmin/check-manifest/compare/0.49...0.50) - [github.com/PyCQA/pylint: v3.2.7 → v3.3.1](https://github.com/PyCQA/pylint/compare/v3.2.7...v3.3.1) - [github.com/python-jsonschema/check-jsonschema: 0.29.2 → 0.29.4](https://github.com/python-jsonschema/check-jsonschema/compare/0.29.2...0.29.4) * style: pre-commit fixes --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 16 ++++++++-------- include/pybind11/detail/common.h | 8 ++++---- include/pybind11/detail/descr.h | 5 ++--- include/pybind11/detail/internals.h | 4 ++-- include/pybind11/detail/type_caster_base.h | 8 ++++---- include/pybind11/eigen/tensor.h | 6 +++--- 6 files changed, 23 insertions(+), 24 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a8190df441..88f82eea8f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -25,14 +25,14 @@ repos: # Clang format the codebase automatically - repo: https://github.com/pre-commit/mirrors-clang-format - rev: "v18.1.8" + rev: "v19.1.3" hooks: - id: clang-format types_or: [c++, c, cuda] # Ruff, the Python auto-correcting linter/formatter written in Rust - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.6.3 + rev: v0.7.2 hooks: - id: ruff args: ["--fix", "--show-fixes"] @@ -40,7 +40,7 @@ repos: # Check static types with mypy - repo: https://github.com/pre-commit/mirrors-mypy - rev: "v1.11.2" + rev: "v1.13.0" hooks: - id: mypy args: [] @@ -62,7 +62,7 @@ repos: # Standard hooks - repo: https://github.com/pre-commit/pre-commit-hooks - rev: "v4.6.0" + rev: "v5.0.0" hooks: - id: check-added-large-files - id: check-case-conflict @@ -79,7 +79,7 @@ repos: # Also code format the docs - repo: https://github.com/adamchainz/blacken-docs - rev: "1.18.0" + rev: "1.19.1" hooks: - id: blacken-docs additional_dependencies: @@ -108,7 +108,7 @@ repos: # Checks the manifest for missing files (native support) - repo: https://github.com/mgedmin/check-manifest - rev: "0.49" + rev: "0.50" hooks: - id: check-manifest # This is a slow hook, so only run this if --hook-stage manual is passed @@ -142,14 +142,14 @@ repos: # PyLint has native support - not always usable, but works for us - repo: https://github.com/PyCQA/pylint - rev: "v3.2.7" + rev: "v3.3.1" hooks: - id: pylint files: ^pybind11 # Check schemas on some of our YAML files - repo: https://github.com/python-jsonschema/check-jsonschema - rev: 0.29.2 + rev: 0.29.4 hooks: - id: check-readthedocs - id: check-github-workflows diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index c974d89959..4cf11e2838 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -1145,14 +1145,14 @@ struct overload_cast_impl { } template - constexpr auto operator()(Return (Class::*pmf)(Args...), - std::false_type = {}) const noexcept -> decltype(pmf) { + constexpr auto operator()(Return (Class::*pmf)(Args...), std::false_type = {}) const noexcept + -> decltype(pmf) { return pmf; } template - constexpr auto operator()(Return (Class::*pmf)(Args...) const, - std::true_type) const noexcept -> decltype(pmf) { + constexpr auto operator()(Return (Class::*pmf)(Args...) const, std::true_type) const noexcept + -> decltype(pmf) { return pmf; } }; diff --git a/include/pybind11/detail/descr.h b/include/pybind11/detail/descr.h index 7d546311e7..635614b0d6 100644 --- a/include/pybind11/detail/descr.h +++ b/include/pybind11/detail/descr.h @@ -156,9 +156,8 @@ constexpr auto concat(const descr &d, const Args &...args) { } #else template -constexpr auto concat(const descr &d, - const Args &...args) -> decltype(std::declval>() - + concat(args...)) { +constexpr auto concat(const descr &d, const Args &...args) + -> decltype(std::declval>() + concat(args...)) { return d + const_name(", ") + concat(args...); } #endif diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 330f5e1cce..c960a86cb1 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -671,8 +671,8 @@ inline std::uint64_t mix64(std::uint64_t z) { } template -inline auto with_instance_map(const void *ptr, - const F &cb) -> decltype(cb(std::declval())) { +inline auto with_instance_map(const void *ptr, const F &cb) + -> decltype(cb(std::declval())) { auto &internals = get_internals(); #ifdef Py_GIL_DISABLED diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index e7b94aff2a..0898be0140 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -1161,14 +1161,14 @@ class type_caster_base : public type_caster_generic { does not have a private operator new implementation. A comma operator is used in the decltype argument to apply SFINAE to the public copy/move constructors.*/ template ::value>> - static auto make_copy_constructor(const T *) -> decltype(new T(std::declval()), - Constructor{}) { + static auto make_copy_constructor(const T *) + -> decltype(new T(std::declval()), Constructor{}) { return [](const void *arg) -> void * { return new T(*reinterpret_cast(arg)); }; } template ::value>> - static auto make_move_constructor(const T *) -> decltype(new T(std::declval()), - Constructor{}) { + static auto make_move_constructor(const T *) + -> decltype(new T(std::declval()), Constructor{}) { return [](const void *arg) -> void * { return new T(std::move(*const_cast(reinterpret_cast(arg)))); }; diff --git a/include/pybind11/eigen/tensor.h b/include/pybind11/eigen/tensor.h index 0a9d7c2522..9b5d9e89b5 100644 --- a/include/pybind11/eigen/tensor.h +++ b/include/pybind11/eigen/tensor.h @@ -124,9 +124,9 @@ struct eigen_tensor_helper< template struct get_tensor_descriptor { static constexpr auto details - = const_name(", flags.writeable", "") - + const_name(Type::Layout) == static_cast(Eigen::RowMajor)>( - ", flags.c_contiguous", ", flags.f_contiguous"); + = const_name(", flags.writeable", "") + const_name + < static_cast(Type::Layout) + == static_cast(Eigen::RowMajor) > (", flags.c_contiguous", ", flags.f_contiguous"); static constexpr auto value = const_name("numpy.ndarray[") + npy_format_descriptor::name + const_name("[") + eigen_tensor_helper>::dimensions_descriptor From f46f5be4fa4d24c4e5382d0251315f361ce97424 Mon Sep 17 00:00:00 2001 From: Tim Stumbaugh Date: Wed, 6 Nov 2024 12:21:33 -0700 Subject: [PATCH 2/4] Fix incorrect link syntax in upgrade guide (#5434) Looks like some markdown spilled into our restructured text --- docs/upgrade.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/upgrade.rst b/docs/upgrade.rst index 17c26aaa93..5cef2b81ab 100644 --- a/docs/upgrade.rst +++ b/docs/upgrade.rst @@ -24,7 +24,8 @@ changes are that: function is not available anymore. Due to NumPy changes, you may experience difficulties updating to NumPy 2. -Please see the [NumPy 2 migration guide](https://numpy.org/devdocs/numpy_2_0_migration_guide.html) for details. +Please see the `NumPy 2 migration guide `_ +for details. For example, a more direct change could be that the default integer ``"int_"`` (and ``"uint"``) is now ``ssize_t`` and not ``long`` (affects 64bit windows). From ce2f00559498f4070821d540ba446e3a59766154 Mon Sep 17 00:00:00 2001 From: vfdev Date: Thu, 7 Nov 2024 18:32:09 +0100 Subject: [PATCH 3/4] Fixed data race in all_type_info in free-threading mode (#5419) * Fix data race all_type_info_populate in free-threading mode Description: - fixed data race all_type_info_populate in free-threading mode - added test For example, we have 2 threads entering `all_type_info`. Both enter `all_type_info_get_cache`` function and there is a first one which inserts a tuple (type, empty_vector) to the map and second is waiting. Inserting thread gets the (iter_to_key, True) and non-inserting thread after waiting gets (iter_to_key, False). Inserting thread than will add a weakref and will then call into `all_type_info_populate`. However, non-inserting thread is not entering `if (ins.second) {` clause and returns `ins.first->second;`` which is just empty_vector. Finally, non-inserting thread is failing the check in `allocate_layout`: ```c++ if (n_types == 0) { pybind11_fail( "instance allocation failed: new instance has no pybind11-registered base types"); } ``` * style: pre-commit fixes * Addressed PR comments --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- include/pybind11/detail/type_caster_base.h | 9 +------ include/pybind11/pybind11.h | 15 ++++++++--- tests/pybind11_tests.cpp | 5 ++++ tests/pybind11_tests.h | 21 ++++++++++++++++ tests/test_class.py | 29 ++++++++++++++++++++++ 5 files changed, 67 insertions(+), 12 deletions(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 0898be0140..d5d86dc6c1 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -117,7 +117,6 @@ PYBIND11_NOINLINE void all_type_info_populate(PyTypeObject *t, std::vector(t->tp_bases)) { check.push_back((PyTypeObject *) parent.ptr()); } - auto const &type_dict = get_internals().registered_types_py; for (size_t i = 0; i < check.size(); i++) { auto *type = check[i]; @@ -176,13 +175,7 @@ PYBIND11_NOINLINE void all_type_info_populate(PyTypeObject *t, std::vector &all_type_info(PyTypeObject *type) { - auto ins = all_type_info_get_cache(type); - if (ins.second) { - // New cache entry: populate it - all_type_info_populate(type, ins.first->second); - } - - return ins.first->second; + return all_type_info_get_cache(type).first->second; } /** diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 2527d25faf..b4f93f1a6a 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2326,13 +2326,20 @@ keep_alive_impl(size_t Nurse, size_t Patient, function_call &call, handle ret) { inline std::pair all_type_info_get_cache(PyTypeObject *type) { auto res = with_internals([type](internals &internals) { - return internals - .registered_types_py + auto ins = internals + .registered_types_py #ifdef __cpp_lib_unordered_map_try_emplace - .try_emplace(type); + .try_emplace(type); #else - .emplace(type, std::vector()); + .emplace(type, std::vector()); #endif + if (ins.second) { + // For free-threading mode, this call must be under + // the with_internals() mutex lock, to avoid that other threads + // continue running with the empty ins.first->second. + all_type_info_populate(type, ins.first->second); + } + return ins; }); if (res.second) { // New cache entry created; set up a weak reference to automatically remove it if the type diff --git a/tests/pybind11_tests.cpp b/tests/pybind11_tests.cpp index 3d2d84e77a..818d53a548 100644 --- a/tests/pybind11_tests.cpp +++ b/tests/pybind11_tests.cpp @@ -128,4 +128,9 @@ PYBIND11_MODULE(pybind11_tests, m, py::mod_gil_not_used()) { for (const auto &initializer : initializers()) { initializer(m); } + + py::class_(m, "TestContext") + .def(py::init<>(&TestContext::createNewContextForInit)) + .def("__enter__", &TestContext::contextEnter) + .def("__exit__", &TestContext::contextExit); } diff --git a/tests/pybind11_tests.h b/tests/pybind11_tests.h index 7be58feb6c..0eb0398df0 100644 --- a/tests/pybind11_tests.h +++ b/tests/pybind11_tests.h @@ -96,3 +96,24 @@ void ignoreOldStyleInitWarnings(F &&body) { )", py::dict(py::arg("body") = py::cpp_function(body))); } + +// See PR #5419 for background. +class TestContext { +public: + TestContext() = delete; + TestContext(const TestContext &) = delete; + TestContext(TestContext &&) = delete; + static TestContext *createNewContextForInit() { return new TestContext("new-context"); } + + pybind11::object contextEnter() { + py::object contextObj = py::cast(*this); + return contextObj; + } + void contextExit(const pybind11::object & /*excType*/, + const pybind11::object & /*excVal*/, + const pybind11::object & /*excTb*/) {} + +private: + explicit TestContext(const std::string &context) : context(context) {} + std::string context; +}; diff --git a/tests/test_class.py b/tests/test_class.py index f424db5c35..01963d0122 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -1,5 +1,6 @@ from __future__ import annotations +import sys from unittest import mock import pytest @@ -508,3 +509,31 @@ def test_pr4220_tripped_over_this(): m.Empty0().get_msg() == "This is really only meant to exercise successful compilation." ) + + +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") +def test_all_type_info_multithreaded(): + # See PR #5419 for background. + import threading + + from pybind11_tests import TestContext + + class Context(TestContext): + pass + + num_runs = 10 + num_threads = 4 + barrier = threading.Barrier(num_threads) + + def func(): + barrier.wait() + with Context(): + pass + + for _ in range(num_runs): + threads = [threading.Thread(target=func) for _ in range(num_threads)] + for thread in threads: + thread.start() + + for thread in threads: + thread.join() From 85209ea0ce61e015c385f315ae12cae05d0ba25a Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 7 Nov 2024 10:05:12 -0800 Subject: [PATCH 4/4] clang-format auto-fix (consequence of master PR #5432). --- tests/test_class_sh_disowning.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_class_sh_disowning.cpp b/tests/test_class_sh_disowning.cpp index 1c88bf071b..81e17ac437 100644 --- a/tests/test_class_sh_disowning.cpp +++ b/tests/test_class_sh_disowning.cpp @@ -47,7 +47,7 @@ TEST_SUBMODULE(class_sh_disowning, m) { m.def("mixed", mixed); - m.def("overloaded", (int (*)(std::unique_ptr>, int)) & overloaded); - m.def("overloaded", (int (*)(std::unique_ptr>, int)) & overloaded); + m.def("overloaded", (int (*)(std::unique_ptr>, int)) &overloaded); + m.def("overloaded", (int (*)(std::unique_ptr>, int)) &overloaded); #endif // PYBIND11_SMART_HOLDER_ENABLED }