From dbf848aff7c37ef8798bc9459a86193e28b1032f Mon Sep 17 00:00:00 2001 From: Ralf Gommers Date: Thu, 18 Jul 2024 08:26:45 +0200 Subject: [PATCH 1/3] docs: extend `PYBIND11_MODULE` documentation, mention `mod_gil_not_used` (#5250) This follows up on PR 5148, which introduced support for free-threaded CPython. --- include/pybind11/detail/common.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index fc66e6f75..9418334ca 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -462,6 +462,22 @@ PYBIND11_WARNING_POP return "Hello, World!"; }); } + + The third macro argument is optional (available since 2.13.0), and can be used to + mark the extension module as safe to run without the GIL under a free-threaded CPython + interpreter. Passing this argument has no effect on other interpreters. + + .. code-block:: cpp + + PYBIND11_MODULE(example, m, py::mod_gil_not_used()) { + m.doc() = "pybind11 example module safe to run without the GIL"; + + // Add bindings here + m.def("foo", []() { + return "Hello, Free-threaded World!"; + }); + } + \endrst */ #define PYBIND11_MODULE(name, variable, ...) \ static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name) \ From a582ca8a8e2705be848df164158c605e15e010fb Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Thu, 18 Jul 2024 14:50:38 -0400 Subject: [PATCH 2/3] tests: run on pyodide (#4745) * tests: run on pyodide Signed-off-by: Henry Schreiner * ci: use cibuildwheel for pyodide test Signed-off-by: Henry Schreiner * tests: revert changes to test_embed Signed-off-by: Henry Schreiner --------- Signed-off-by: Henry Schreiner --- .github/workflows/emscripten.yaml | 30 ++++++++++++++++++++++++++++++ tests/CMakeLists.txt | 10 +++++++++- tests/pyproject.toml | 17 +++++++++++++++++ tests/test_async.py | 5 +++++ tests/test_callbacks.py | 3 +++ tests/test_exceptions.py | 2 +- tests/test_gil_scoped.py | 13 +++++++++---- tests/test_iostream.py | 4 ++++ tests/test_thread.py | 5 +++++ tests/test_virtual_functions.py | 3 +++ 10 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/emscripten.yaml create mode 100644 tests/pyproject.toml diff --git a/.github/workflows/emscripten.yaml b/.github/workflows/emscripten.yaml new file mode 100644 index 000000000..4f12e81c2 --- /dev/null +++ b/.github/workflows/emscripten.yaml @@ -0,0 +1,30 @@ +name: WASM + +on: + workflow_dispatch: + pull_request: + branches: + - master + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + build-wasm-emscripten: + name: Pyodide wheel + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v4 + with: + submodules: true + fetch-depth: 0 + + - uses: pypa/cibuildwheel@v2.19 + env: + PYODIDE_BUILD_EXPORTS: whole_archive + CFLAGS: -fexceptions + LDFLAGS: -fexceptions + with: + package-dir: tests + only: cp312-pyodide_wasm32 diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f182e2499..aae9be720 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -88,7 +88,12 @@ set(PYBIND11_TEST_FILTER if(CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR) # We're being loaded directly, i.e. not via add_subdirectory, so make this # work as its own project and load the pybind11Config to get the tools we need - find_package(pybind11 REQUIRED CONFIG) + + if(SKBUILD) + add_subdirectory(.. pybind11_src) + else() + find_package(pybind11 REQUIRED CONFIG) + endif() endif() if(NOT CMAKE_BUILD_TYPE AND NOT DEFINED CMAKE_CONFIGURATION_TYPES) @@ -489,6 +494,9 @@ foreach(target ${test_targets}) endforeach() endif() endif() + if(SKBUILD) + install(TARGETS ${target} LIBRARY DESTINATION .) + endif() endforeach() # Provide nice organisation in IDEs diff --git a/tests/pyproject.toml b/tests/pyproject.toml new file mode 100644 index 000000000..469c145df --- /dev/null +++ b/tests/pyproject.toml @@ -0,0 +1,17 @@ +# Warning: this is currently used for pyodide, and is not a general out-of-tree +# builder for the tests (yet). Specifically, wheels can't be built from SDists. + +[build-system] +requires = ["scikit-build-core"] +build-backend = "scikit_build_core.build" + +[project] +name = "pybind11_tests" +version = "0.0.1" +dependencies = ["pytest", "pytest-timeout", "numpy", "scipy"] + +[tool.scikit-build.cmake.define] +PYBIND11_FINDPYTHON = true + +[tool.cibuildwheel] +test-command = "pytest -o timeout=0 -p no:cacheprovider {project}/tests/test_*.py" diff --git a/tests/test_async.py b/tests/test_async.py index 4d33ba65f..1705196d1 100644 --- a/tests/test_async.py +++ b/tests/test_async.py @@ -1,10 +1,15 @@ from __future__ import annotations +import sys + import pytest asyncio = pytest.importorskip("asyncio") m = pytest.importorskip("pybind11_tests.async_module") +if sys.platform.startswith("emscripten"): + pytest.skip("Can't run a new event_loop in pyodide", allow_module_level=True) + @pytest.fixture() def event_loop(): diff --git a/tests/test_callbacks.py b/tests/test_callbacks.py index ce2a6d254..db6d8dece 100644 --- a/tests/test_callbacks.py +++ b/tests/test_callbacks.py @@ -1,5 +1,6 @@ from __future__ import annotations +import sys import time from threading import Thread @@ -153,6 +154,7 @@ def test_python_builtins(): assert m.test_sum_builtin(sum, []) == 0 +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_async_callbacks(): # serves as state for async callback class Item: @@ -176,6 +178,7 @@ def test_async_callbacks(): assert sum(res) == sum(x + 3 for x in work) +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_async_async_callbacks(): t = Thread(target=test_async_callbacks) t.start() diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index b33997eee..db2a551e4 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -75,7 +75,7 @@ def test_cross_module_exceptions(msg): # TODO: FIXME @pytest.mark.xfail( - "env.MACOS and (env.PYPY or pybind11_tests.compiler_info.startswith('Homebrew Clang'))", + "env.MACOS and (env.PYPY or pybind11_tests.compiler_info.startswith('Homebrew Clang')) or sys.platform.startswith('emscripten')", raises=RuntimeError, reason="See Issue #2847, PR #2999, PR #4324", ) diff --git a/tests/test_gil_scoped.py b/tests/test_gil_scoped.py index a18387684..eab92093c 100644 --- a/tests/test_gil_scoped.py +++ b/tests/test_gil_scoped.py @@ -71,24 +71,28 @@ def test_cross_module_gil_inner_pybind11_acquired(): m.test_cross_module_gil_inner_pybind11_acquired() +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_cross_module_gil_nested_custom_released(): """Makes sure that the GIL can be nested acquired/released by another module from a GIL-released state using custom locking logic.""" m.test_cross_module_gil_nested_custom_released() +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_cross_module_gil_nested_custom_acquired(): """Makes sure that the GIL can be nested acquired/acquired by another module from a GIL-acquired state using custom locking logic.""" m.test_cross_module_gil_nested_custom_acquired() +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_cross_module_gil_nested_pybind11_released(): """Makes sure that the GIL can be nested acquired/released by another module from a GIL-released state using pybind11 locking logic.""" m.test_cross_module_gil_nested_pybind11_released() +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_cross_module_gil_nested_pybind11_acquired(): """Makes sure that the GIL can be nested acquired/acquired by another module from a GIL-acquired state using pybind11 locking logic.""" @@ -103,6 +107,7 @@ def test_nested_acquire(): assert m.test_nested_acquire(0xAB) == "171" +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_multi_acquire_release_cross_module(): for bits in range(16 * 8): internals_ids = m.test_multi_acquire_release_cross_module(bits) @@ -204,7 +209,7 @@ def _run_in_threads(test_fn, num_threads, parallel): thread.join() -# TODO: FIXME, sometimes returns -11 (segfault) instead of 0 on macOS Python 3.9 +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") @pytest.mark.parametrize("test_fn", ALL_BASIC_TESTS_PLUS_INTENTIONAL_DEADLOCK) def test_run_in_process_one_thread(test_fn): """Makes sure there is no GIL deadlock when running in a thread. @@ -214,7 +219,7 @@ def test_run_in_process_one_thread(test_fn): assert _run_in_process(_run_in_threads, test_fn, num_threads=1, parallel=False) == 0 -# TODO: FIXME on macOS Python 3.9 +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") @pytest.mark.parametrize("test_fn", ALL_BASIC_TESTS_PLUS_INTENTIONAL_DEADLOCK) def test_run_in_process_multiple_threads_parallel(test_fn): """Makes sure there is no GIL deadlock when running in a thread multiple times in parallel. @@ -224,7 +229,7 @@ def test_run_in_process_multiple_threads_parallel(test_fn): assert _run_in_process(_run_in_threads, test_fn, num_threads=8, parallel=True) == 0 -# TODO: FIXME on macOS Python 3.9 +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") @pytest.mark.parametrize("test_fn", ALL_BASIC_TESTS_PLUS_INTENTIONAL_DEADLOCK) def test_run_in_process_multiple_threads_sequential(test_fn): """Makes sure there is no GIL deadlock when running in a thread multiple times sequentially. @@ -234,7 +239,7 @@ def test_run_in_process_multiple_threads_sequential(test_fn): assert _run_in_process(_run_in_threads, test_fn, num_threads=8, parallel=False) == 0 -# TODO: FIXME on macOS Python 3.9 +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") @pytest.mark.parametrize("test_fn", ALL_BASIC_TESTS_PLUS_INTENTIONAL_DEADLOCK) def test_run_in_process_direct(test_fn): """Makes sure there is no GIL deadlock when using processes. diff --git a/tests/test_iostream.py b/tests/test_iostream.py index f7eeff502..c3d987787 100644 --- a/tests/test_iostream.py +++ b/tests/test_iostream.py @@ -1,8 +1,11 @@ from __future__ import annotations +import sys from contextlib import redirect_stderr, redirect_stdout from io import StringIO +import pytest + from pybind11_tests import iostream as m @@ -270,6 +273,7 @@ def test_redirect_both(capfd): assert stream2.getvalue() == msg2 +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_threading(): with m.ostream_redirect(stdout=True, stderr=False): # start some threads diff --git a/tests/test_thread.py b/tests/test_thread.py index 4541a305e..f12020e41 100644 --- a/tests/test_thread.py +++ b/tests/test_thread.py @@ -1,7 +1,10 @@ from __future__ import annotations +import sys import threading +import pytest + from pybind11_tests import thread as m @@ -24,6 +27,7 @@ class Thread(threading.Thread): raise self.e +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_implicit_conversion(): a = Thread(m.test) b = Thread(m.test) @@ -34,6 +38,7 @@ def test_implicit_conversion(): x.join() +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_implicit_conversion_no_gil(): a = Thread(m.test_no_gil) b = Thread(m.test_no_gil) diff --git a/tests/test_virtual_functions.py b/tests/test_virtual_functions.py index 08acaa190..bc0177787 100644 --- a/tests/test_virtual_functions.py +++ b/tests/test_virtual_functions.py @@ -1,5 +1,7 @@ from __future__ import annotations +import sys + import pytest import env # noqa: F401 @@ -435,6 +437,7 @@ def test_inherited_virtuals(): assert obj.say_everything() == "BT -7" +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_issue_1454(): # Fix issue #1454 (crash when acquiring/releasing GIL on another thread in Python 2.7) m.test_gil() From 6d4805ced1f8fc8f503d41381853f6082f3064a3 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 19 Jul 2024 07:34:06 +0700 Subject: [PATCH 3/3] Small cleanup/refactoring in support of PR #5213 (#5251) * Factor out detail/value_and_holder.h (from detail/type_caster_base.h) This is in support of PR #5213: * trampoline_self_life_support.h depends on value_and_holder.h * type_caster_base.h depends on trampoline_self_life_support.h * Fix a minor and inconsequential inconsistency in `copyable_holder_caster`: the correct `load_value()` return type is `void` (as defined in `type_caster_generic`) For easy future reference, this is the long-standing inconsistency: * https://github.com/pybind/pybind11/blob/dbf848aff7c37ef8798bc9459a86193e28b1032f/include/pybind11/detail/type_caster_base.h#L634 * https://github.com/pybind/pybind11/blob/dbf848aff7c37ef8798bc9459a86193e28b1032f/include/pybind11/cast.h#L797 Noticed in passing while working on PR #5213. * Add `DANGER ZONE` comment in detail/init.h, similar to a comment added on the smart_holder branch (all the way back in 2021). --- CMakeLists.txt | 1 + include/pybind11/cast.h | 4 +- include/pybind11/detail/init.h | 6 +- include/pybind11/detail/type_caster_base.h | 62 +---------------- include/pybind11/detail/value_and_holder.h | 77 ++++++++++++++++++++++ tests/extra_python_package/test_files.py | 1 + 6 files changed, 86 insertions(+), 65 deletions(-) create mode 100644 include/pybind11/detail/value_and_holder.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 3526a1a66..6e5b8c8f3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -154,6 +154,7 @@ set(PYBIND11_HEADERS include/pybind11/detail/internals.h include/pybind11/detail/type_caster_base.h include/pybind11/detail/typeid.h + include/pybind11/detail/value_and_holder.h include/pybind11/attr.h include/pybind11/buffer_info.h include/pybind11/cast.h diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index e41ad2abf..0f3091f68 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -794,11 +794,11 @@ protected: } } - bool load_value(value_and_holder &&v_h) { + void load_value(value_and_holder &&v_h) { if (v_h.holder_constructed()) { value = v_h.value_ptr(); holder = v_h.template holder(); - return true; + return; } throw cast_error("Unable to cast from non-held to held instance (T& to Holder) " #if !defined(PYBIND11_DETAILED_ERROR_MESSAGES) diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index 4509bd131..79cc930c8 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -128,11 +128,13 @@ void construct(value_and_holder &v_h, Cpp *ptr, bool need_alias) { // the holder and destruction happens when we leave the C++ scope, and the holder // class gets to handle the destruction however it likes. v_h.value_ptr() = ptr; - v_h.set_instance_registered(true); // To prevent init_instance from registering it - v_h.type->init_instance(v_h.inst, nullptr); // Set up the holder + v_h.set_instance_registered(true); // Trick to prevent init_instance from registering it + // DANGER ZONE BEGIN: exceptions will leave v_h in an invalid state. + v_h.type->init_instance(v_h.inst, nullptr); // Set up the holder Holder temp_holder(std::move(v_h.holder>())); // Steal the holder v_h.type->dealloc(v_h); // Destroys the moved-out holder remains, resets value ptr to null v_h.set_instance_registered(false); + // DANGER ZONE END. construct_alias_from_cpp(is_alias_constructible{}, v_h, std::move(*ptr)); } else { diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index fd8c81b9a..1b57ee83c 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -14,6 +14,7 @@ #include "descr.h" #include "internals.h" #include "typeid.h" +#include "value_and_holder.h" #include #include @@ -259,67 +260,6 @@ PYBIND11_NOINLINE handle find_registered_python_instance(void *src, }); } -struct value_and_holder { - instance *inst = nullptr; - size_t index = 0u; - const detail::type_info *type = nullptr; - void **vh = nullptr; - - // Main constructor for a found value/holder: - value_and_holder(instance *i, const detail::type_info *type, size_t vpos, size_t index) - : inst{i}, index{index}, type{type}, - vh{inst->simple_layout ? inst->simple_value_holder - : &inst->nonsimple.values_and_holders[vpos]} {} - - // Default constructor (used to signal a value-and-holder not found by get_value_and_holder()) - value_and_holder() = default; - - // Used for past-the-end iterator - explicit value_and_holder(size_t index) : index{index} {} - - template - V *&value_ptr() const { - return reinterpret_cast(vh[0]); - } - // True if this `value_and_holder` has a non-null value pointer - explicit operator bool() const { return value_ptr() != nullptr; } - - template - H &holder() const { - return reinterpret_cast(vh[1]); - } - bool holder_constructed() const { - return inst->simple_layout - ? inst->simple_holder_constructed - : (inst->nonsimple.status[index] & instance::status_holder_constructed) != 0u; - } - // NOLINTNEXTLINE(readability-make-member-function-const) - void set_holder_constructed(bool v = true) { - if (inst->simple_layout) { - inst->simple_holder_constructed = v; - } else if (v) { - inst->nonsimple.status[index] |= instance::status_holder_constructed; - } else { - inst->nonsimple.status[index] &= (std::uint8_t) ~instance::status_holder_constructed; - } - } - bool instance_registered() const { - return inst->simple_layout - ? inst->simple_instance_registered - : ((inst->nonsimple.status[index] & instance::status_instance_registered) != 0); - } - // NOLINTNEXTLINE(readability-make-member-function-const) - void set_instance_registered(bool v = true) { - if (inst->simple_layout) { - inst->simple_instance_registered = v; - } else if (v) { - inst->nonsimple.status[index] |= instance::status_instance_registered; - } else { - inst->nonsimple.status[index] &= (std::uint8_t) ~instance::status_instance_registered; - } - } -}; - // Container for accessing and iterating over an instance's values/holders struct values_and_holders { private: diff --git a/include/pybind11/detail/value_and_holder.h b/include/pybind11/detail/value_and_holder.h new file mode 100644 index 000000000..ca37d70ad --- /dev/null +++ b/include/pybind11/detail/value_and_holder.h @@ -0,0 +1,77 @@ +// Copyright (c) 2016-2024 The Pybind Development Team. +// All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +#pragma once + +#include "common.h" + +#include +#include + +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) +PYBIND11_NAMESPACE_BEGIN(detail) + +struct value_and_holder { + instance *inst = nullptr; + size_t index = 0u; + const detail::type_info *type = nullptr; + void **vh = nullptr; + + // Main constructor for a found value/holder: + value_and_holder(instance *i, const detail::type_info *type, size_t vpos, size_t index) + : inst{i}, index{index}, type{type}, + vh{inst->simple_layout ? inst->simple_value_holder + : &inst->nonsimple.values_and_holders[vpos]} {} + + // Default constructor (used to signal a value-and-holder not found by get_value_and_holder()) + value_and_holder() = default; + + // Used for past-the-end iterator + explicit value_and_holder(size_t index) : index{index} {} + + template + V *&value_ptr() const { + return reinterpret_cast(vh[0]); + } + // True if this `value_and_holder` has a non-null value pointer + explicit operator bool() const { return value_ptr() != nullptr; } + + template + H &holder() const { + return reinterpret_cast(vh[1]); + } + bool holder_constructed() const { + return inst->simple_layout + ? inst->simple_holder_constructed + : (inst->nonsimple.status[index] & instance::status_holder_constructed) != 0u; + } + // NOLINTNEXTLINE(readability-make-member-function-const) + void set_holder_constructed(bool v = true) { + if (inst->simple_layout) { + inst->simple_holder_constructed = v; + } else if (v) { + inst->nonsimple.status[index] |= instance::status_holder_constructed; + } else { + inst->nonsimple.status[index] &= (std::uint8_t) ~instance::status_holder_constructed; + } + } + bool instance_registered() const { + return inst->simple_layout + ? inst->simple_instance_registered + : ((inst->nonsimple.status[index] & instance::status_instance_registered) != 0); + } + // NOLINTNEXTLINE(readability-make-member-function-const) + void set_instance_registered(bool v = true) { + if (inst->simple_layout) { + inst->simple_instance_registered = v; + } else if (v) { + inst->nonsimple.status[index] |= instance::status_instance_registered; + } else { + inst->nonsimple.status[index] &= (std::uint8_t) ~instance::status_instance_registered; + } + } +}; + +PYBIND11_NAMESPACE_END(detail) +PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py index 5a3f779a7..aedbdf1c1 100644 --- a/tests/extra_python_package/test_files.py +++ b/tests/extra_python_package/test_files.py @@ -58,6 +58,7 @@ detail_headers = { "include/pybind11/detail/internals.h", "include/pybind11/detail/type_caster_base.h", "include/pybind11/detail/typeid.h", + "include/pybind11/detail/value_and_holder.h", } eigen_headers = {