Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add pybind11/gil_safe_call_once.h (to fix deadlocks in pybind11/numpy.h) #4877

Merged
merged 31 commits into from
Oct 12, 2023
Merged
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
38317c3
LazyInitializeAtLeastOnceDestroyNever v1
Oct 9, 2023
e7b8c4f
Go back to using `union` as originally suggested by jbms@. The trick …
Oct 9, 2023
74ac0d9
Revert "Go back to using `union` as originally suggested by jbms@. Th…
Oct 9, 2023
109a165
Remove `#include <stdalign.h>`
Oct 9, 2023
88cec11
Suppress gcc 4.8.5 (CentOS 7) warning.
Oct 9, 2023
1ce2715
Replace comments:
Oct 9, 2023
e7be9c2
Adopt suggestion by @tkoeppe:
Oct 9, 2023
f07b28b
Add `PYBIND11_CONSTINIT`, but it does not work for the current use ca…
Oct 9, 2023
36be645
Revert "Add `PYBIND11_CONSTINIT`, but it does not work for the curren…
Oct 9, 2023
7bc16a6
Reapply "Add `PYBIND11_CONSTINIT`, but it does not work for the curre…
Oct 9, 2023
78f4e93
Add Default Member Initializer on `value_storage_` as suggested by @t…
Oct 9, 2023
6d9441d
Fix copy-paste-missed-a-change mishap in commit 88cec1152ab5576db19ba…
Oct 9, 2023
a864f21
Semi-paranoid placement new (based on https://github.com/pybind/pybin…
Oct 9, 2023
6689b06
Move PYBIND11_CONSTINIT to detail/common.h
Oct 9, 2023
d965f29
Move code to the right places, rename new class and some variables.
Oct 9, 2023
398a42c
Fix oversight: update tests/extra_python_package/test_files.py
Oct 9, 2023
ab2cf8e
Get the name right first.
Oct 9, 2023
6d5bdd8
Use `std::call_once`, `std::atomic`, following a pattern developed by…
Oct 9, 2023
4c5dd1b
Make the API more self-documenting (and possibly more easily reusable).
Oct 9, 2023
8633c5b
google-clang-tidy IWYU fixes
Oct 9, 2023
82f3efc
Rewrite comment as suggested by @tkoeppe
Oct 10, 2023
3ebd139
Update test_exceptions.cpp and exceptions.rst
Oct 10, 2023
c33712d
Fix oversight in previous commit: add `PYBIND11_CONSTINIT`
Oct 10, 2023
dcf2b92
Make `get_stored()` non-const for simplicity.
Oct 10, 2023
4557dce
Add comment regarding `KeyboardInterrupt` behavior, based heavily on …
Oct 10, 2023
704fe13
Add `assert(PyGILState_Check())` in `gil_scoped_release` ctor (simple…
Oct 10, 2023
b2f87a8
Fix oversight in previous commit (missing include cassert).
Oct 10, 2023
fad1017
Remove use of std::atomic, leaving comments with rationale, why it is…
Oct 10, 2023
8453302
Rewrite comment re `std:optional` based on deeper reflection (aka 2nd…
Oct 10, 2023
66bbc67
Additional comment with the conclusion of a discussion under PR #4877.
Oct 11, 2023
7cd9390
Small comment changes suggested by @tkoeppe.
Oct 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add PYBIND11_CONSTINIT, but it does not work for the current use ca…
…ses:

```
g++ -o pybind11/tests/test_numpy_array.os -c -std=c++20 -fPIC -fvisibility=hidden -O0 -g -Wall -Wextra -Wconversion -Wcast-qual -Wdeprecated -Wundef -Wnon-virtual-dtor -Wunused-result -Werror -isystem /usr/include/python3.11 -isystem /usr/include/eigen3 -DPYBIND11_STRICT_ASSERTS_CLASS_HOLDER_VS_TYPE_CASTER_MIX -DPYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD_IF_AVAILABLE -DPYBIND11_TEST_BOOST -Ipybind11/include -I/usr/local/google/home/rwgk/forked/pybind11/include -I/usr/local/google/home/rwgk/clone/pybind11/include /usr/local/google/home/rwgk/forked/pybind11/tests/test_numpy_array.cpp
```

```
In file included from /usr/local/google/home/rwgk/forked/pybind11/tests/test_numpy_array.cpp:10:
/usr/local/google/home/rwgk/forked/pybind11/include/pybind11/numpy.h: In static member function ‘static pybind11::detail::npy_api& pybind11::detail::npy_api::get()’:
/usr/local/google/home/rwgk/forked/pybind11/include/pybind11/numpy.h:258:82: error: ‘constinit’ variable ‘api_init’ does not have a constant initializer
  258 |         PYBIND11_CONSTINIT static LazyInitializeAtLeastOnceDestroyNever<npy_api> api_init;
      |                                                                                  ^~~~~~~~
```

```
In file included from /usr/local/google/home/rwgk/forked/pybind11/tests/test_numpy_array.cpp:10:
/usr/local/google/home/rwgk/forked/pybind11/include/pybind11/numpy.h: In static member function ‘static pybind11::object& pybind11::dtype::_dtype_from_pep3118()’:
/usr/local/google/home/rwgk/forked/pybind11/include/pybind11/numpy.h:697:13: error: ‘constinit’ variable ‘imported_obj’ does not have a constant initializer
  697 |             imported_obj;
      |             ^~~~~~~~~~~~
```
  • Loading branch information
Ralf W. Grosse-Kunstleve committed Oct 9, 2023
commit f07b28bda9f91fb723aa898a21c81b6dd6857072
13 changes: 11 additions & 2 deletions include/pybind11/numpy.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ class array; // Forward declaration

PYBIND11_NAMESPACE_BEGIN(detail)

#ifndef PYBIND11_CONSTINIT
# if __cplusplus >= 202002L
# define PYBIND11_CONSTINIT constinit
# else
# define PYBIND11_CONSTINIT
# endif
#endif

// Main author of this class: jbms@
template <typename T>
class LazyInitializeAtLeastOnceDestroyNever {
Expand Down Expand Up @@ -247,7 +255,7 @@ struct npy_api {
};

static npy_api &get() {
static LazyInitializeAtLeastOnceDestroyNever<npy_api> api_init;
PYBIND11_CONSTINIT static LazyInitializeAtLeastOnceDestroyNever<npy_api> api_init;
return api_init.Get(lookup);
}

Expand Down Expand Up @@ -685,7 +693,8 @@ class dtype : public object {

private:
static object &_dtype_from_pep3118() {
static detail::LazyInitializeAtLeastOnceDestroyNever<object> imported_obj;
PYBIND11_CONSTINIT static detail::LazyInitializeAtLeastOnceDestroyNever<object>
imported_obj;
return imported_obj.Get([]() {
return detail::import_numpy_core_submodule("_internal").attr("_dtype_from_pep3118");
});
Expand Down