Skip to content

Commit

Permalink
All bells & whistles.
Browse files Browse the repository at this point in the history
  • Loading branch information
rwgk committed Oct 26, 2022
1 parent f3cee36 commit c78c613
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 27 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/ci.yml
Expand Up @@ -102,10 +102,12 @@ jobs:
run: python -m pip install pytest-github-actions-annotate-failures

# First build - C++11 mode and inplace
# More-or-less randomly adding -DPYBIND11_SIMPLE_GIL_MANAGEMENT=ON here.
- name: Configure C++11 ${{ matrix.args }}
run: >
cmake -S . -B .
-DPYBIND11_WERROR=ON
-DPYBIND11_SIMPLE_GIL_MANAGEMENT=ON
-DDOWNLOAD_CATCH=ON
-DDOWNLOAD_EIGEN=ON
-DCMAKE_CXX_STANDARD=11
Expand All @@ -129,10 +131,12 @@ jobs:
run: git clean -fdx

# Second build - C++17 mode and in a build directory
# More-or-less randomly adding -DPYBIND11_SIMPLE_GIL_MANAGEMENT=OFF here.
- name: Configure C++17
run: >
cmake -S . -B build2
-DPYBIND11_WERROR=ON
-DPYBIND11_SIMPLE_GIL_MANAGEMENT=OFF
-DDOWNLOAD_CATCH=ON
-DDOWNLOAD_EIGEN=ON
-DCMAKE_CXX_STANDARD=17
Expand Down
7 changes: 4 additions & 3 deletions CMakeLists.txt
Expand Up @@ -91,13 +91,14 @@ endif()
option(PYBIND11_INSTALL "Install pybind11 header files?" ${PYBIND11_MASTER_PROJECT})
option(PYBIND11_TEST "Build pybind11 test suite?" ${PYBIND11_MASTER_PROJECT})
option(PYBIND11_NOPYTHON "Disable search for Python" OFF)
option(PYBIND11_SIMPLE_GIL "Use simpler GIL access logic that does not support disassociation" OFF)
option(PYBIND11_SIMPLE_GIL_MANAGEMENT
"Use simpler GIL management logic that does not support disassociation" OFF)
set(PYBIND11_INTERNALS_VERSION
""
CACHE STRING "Override the ABI version, may be used to enable the unstable ABI.")

if(PYBIND11_SIMPLE_GIL)
add_compile_definitions(PYBIND11_SIMPLE_GIL)
if(PYBIND11_SIMPLE_GIL_MANAGEMENT)
add_compile_definitions(PYBIND11_SIMPLE_GIL_MANAGEMENT)
endif()

cmake_dependent_option(
Expand Down
18 changes: 8 additions & 10 deletions docs/upgrade.rst
Expand Up @@ -13,16 +13,14 @@ modernization and other useful information.
v2.10
=====

The current scoped GIL implementation doesn't support nested access. In pybind11
In 2.10.1, a configuration option ``PYBIND11_SIMPLE_GIL`` was added, defaulting
to OFF; the simpler GIL implementation supports nested access, but does not
support dissociation (the ``true`` parameter of ``gil_scope_release``). In
pybind11 2.11, we plan to change the default to ON. If you need the old
behavior, please set ``PYBIND11_SIMPLE_GIL`` to OFF. We plan to have an example
for manually supporting dissociation.

There may be an unconfirmed ABI breakage between 2.9 and 2.10. We plan to bump
the internals number in 2.11.
``py::gil_scoped_acquire`` & ``py::gil_scoped_release`` in pybind11 versions
< v2.10.1 do not support nested access. In v2.10.1, a configuration option
``PYBIND11_SIMPLE_GIL_MANAGEMENT`` was added, defaulting to ``OFF``; the
simpler implementations support nested access, but do not support dissociation
(``py::gil_scoped_release(true)``). In pybind11 2.11, we plan to change the
default to ``ON``, to avoid pitfalls of the implementations with dissociation
(see #4216 for more information). Note that the dissociation feature is very
rarely used and not exercised in any pybind11 unit tests.

.. _upgrade-guide-2.9:

Expand Down
4 changes: 4 additions & 0 deletions include/pybind11/detail/common.h
Expand Up @@ -229,6 +229,10 @@
# undef copysign
#endif

#if defined(PYPY_VERSION) && !defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
# define PYBIND11_SIMPLE_GIL_MANAGEMENT
#endif

#if defined(_MSC_VER)
# if defined(PYBIND11_DEBUG_MARKER)
# define _DEBUG
Expand Down
12 changes: 12 additions & 0 deletions include/pybind11/detail/internals.h
Expand Up @@ -9,6 +9,10 @@

#pragma once

#if defined(WITH_THREAD) && defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
# include "../gil.h"
#endif

#include "../pytypes.h"

#include <exception>
Expand Down Expand Up @@ -169,10 +173,12 @@ struct internals {
PyTypeObject *default_metaclass;
PyObject *instance_base;
#if defined(WITH_THREAD)
// Unused if PYBIND11_SIMPLE_GIL_MANAGEMENT is defined:
PYBIND11_TLS_KEY_INIT(tstate)
# if PYBIND11_INTERNALS_VERSION > 4
PYBIND11_TLS_KEY_INIT(loader_life_support_tls_key)
# endif // PYBIND11_INTERNALS_VERSION > 4
// Unused if PYBIND11_SIMPLE_GIL_MANAGEMENT is defined:
PyInterpreterState *istate = nullptr;
~internals() {
# if PYBIND11_INTERNALS_VERSION > 4
Expand Down Expand Up @@ -408,6 +414,10 @@ PYBIND11_NOINLINE internals &get_internals() {
return **internals_pp;
}

#if defined(WITH_THREAD)
# if defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
gil_scoped_acquire gil;
# else
// Ensure that the GIL is held since we will need to make Python calls.
// Cannot use py::gil_scoped_acquire here since that constructor calls get_internals.
struct gil_scoped_acquire_local {
Expand All @@ -417,6 +427,8 @@ PYBIND11_NOINLINE internals &get_internals() {
~gil_scoped_acquire_local() { PyGILState_Release(state); }
const PyGILState_STATE state;
} gil;
# endif
#endif
error_scope err_scope;

PYBIND11_STR_TYPE id(PYBIND11_INTERNALS_ID);
Expand Down
53 changes: 40 additions & 13 deletions include/pybind11/gil.h
Expand Up @@ -10,7 +10,10 @@
#pragma once

#include "detail/common.h"
#include "detail/internals.h"

#if defined(WITH_THREAD) && !defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
# include "detail/internals.h"
#endif

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)

Expand All @@ -21,7 +24,9 @@ PyThreadState *get_thread_state_unchecked();

PYBIND11_NAMESPACE_END(detail)

#if defined(WITH_THREAD) && !defined(PYPY_VERSION) && !defined(PYBIND11_SIMPLE_GIL)
#if defined(WITH_THREAD)

# if !defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)

/* The functions below essentially reproduce the PyGILState_* API using a RAII
* pattern, but there are a few important differences:
Expand Down Expand Up @@ -62,11 +67,11 @@ class gil_scoped_acquire {

if (!tstate) {
tstate = PyThreadState_New(internals.istate);
# if defined(PYBIND11_DETAILED_ERROR_MESSAGES)
# if defined(PYBIND11_DETAILED_ERROR_MESSAGES)
if (!tstate) {
pybind11_fail("scoped_acquire: could not create thread state!");
}
# endif
# endif
tstate->gilstate_counter = 0;
PYBIND11_TLS_REPLACE_VALUE(internals.tstate, tstate);
} else {
Expand All @@ -87,20 +92,20 @@ class gil_scoped_acquire {

PYBIND11_NOINLINE void dec_ref() {
--tstate->gilstate_counter;
# if defined(PYBIND11_DETAILED_ERROR_MESSAGES)
# if defined(PYBIND11_DETAILED_ERROR_MESSAGES)
if (detail::get_thread_state_unchecked() != tstate) {
pybind11_fail("scoped_acquire::dec_ref(): thread state must be current!");
}
if (tstate->gilstate_counter < 0) {
pybind11_fail("scoped_acquire::dec_ref(): reference count underflow!");
}
# endif
# endif
if (tstate->gilstate_counter == 0) {
# if defined(PYBIND11_DETAILED_ERROR_MESSAGES)
# if defined(PYBIND11_DETAILED_ERROR_MESSAGES)
if (!release) {
pybind11_fail("scoped_acquire::dec_ref(): internal error!");
}
# endif
# endif
PyThreadState_Clear(tstate);
if (active) {
PyThreadState_DeleteCurrent();
Expand Down Expand Up @@ -178,12 +183,14 @@ class gil_scoped_release {
bool disassoc;
bool active = true;
};
#elif defined(PYPY_VERSION) || defined(PYBIND11_SIMPLE_GIL)

# else // PYBIND11_SIMPLE_GIL_MANAGEMENT

class gil_scoped_acquire {
PyGILState_STATE state;

public:
gil_scoped_acquire() { state = PyGILState_Ensure(); }
gil_scoped_acquire() : state{PyGILState_Ensure()} {}
gil_scoped_acquire(const gil_scoped_acquire &) = delete;
gil_scoped_acquire &operator=(const gil_scoped_acquire &) = delete;
~gil_scoped_acquire() { PyGILState_Release(state); }
Expand All @@ -194,19 +201,39 @@ class gil_scoped_release {
PyThreadState *state;

public:
gil_scoped_release() { state = PyEval_SaveThread(); }
gil_scoped_release() : state{PyEval_SaveThread()} {}
gil_scoped_release(const gil_scoped_release &) = delete;
gil_scoped_release &operator=(const gil_scoped_acquire &) = delete;
~gil_scoped_release() { PyEval_RestoreThread(state); }
void disarm() {}
};
#else

# endif // PYBIND11_SIMPLE_GIL_MANAGEMENT

#else // WITH_THREAD

class gil_scoped_acquire {
public:
gil_scoped_acquire() {
// Trick to suppress `unused variable` error messages (at call sites).
(void) (this != (this + 1));
}
gil_scoped_acquire(const gil_scoped_acquire &) = delete;
gil_scoped_acquire &operator=(const gil_scoped_acquire &) = delete;
void disarm() {}
};

class gil_scoped_release {
public:
gil_scoped_release() {
// Trick to suppress `unused variable` error messages (at call sites).
(void) (this != (this + 1));
}
gil_scoped_release(const gil_scoped_release &) = delete;
gil_scoped_release &operator=(const gil_scoped_acquire &) = delete;
void disarm() {}
};
#endif

#endif // WITH_THREAD

PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
1 change: 0 additions & 1 deletion tests/test_embed/test_interpreter.cpp
Expand Up @@ -293,7 +293,6 @@ TEST_CASE("Threads") {

{
py::gil_scoped_release gil_release{};
REQUIRE(has_pybind11_internals_static());

auto threads = std::vector<std::thread>();
for (auto i = 0; i < num_threads; ++i) {
Expand Down

0 comments on commit c78c613

Please sign in to comment.