Skip to content

Commit 9f75202

Browse files
authored
Fix thread-safety in get_local_type_info() (#5856)
Fixes potential thread-safety issues if types are concurrently registered while `get_local_type_info()` is called in free threaded Python. Use the `internals` mutex to also protect `local_internals`. This keeps the locking strategy simpler, and we already follow this pattern in some places, such as `pybind11_meta_dealloc`.
1 parent aa4259b commit 9f75202

File tree

2 files changed

+34
-22
lines changed

2 files changed

+34
-22
lines changed

include/pybind11/detail/function_record_pyobject.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ static PyType_Spec function_record_PyType_Spec
9191
function_record_PyType_Slots};
9292

9393
inline PyTypeObject *get_function_record_PyTypeObject() {
94+
PYBIND11_LOCK_INTERNALS(get_internals());
9495
PyTypeObject *&py_type_obj = detail::get_local_internals().function_record_py_type;
9596
if (!py_type_obj) {
9697
PyObject *py_obj = PyType_FromSpec(&function_record_PyType_Spec);

include/pybind11/detail/type_caster_base.h

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ PYBIND11_NOINLINE detail::type_info *get_type_info(PyTypeObject *type) {
205205
return bases.front();
206206
}
207207

208-
inline detail::type_info *get_local_type_info(const std::type_info &tp) {
208+
inline detail::type_info *get_local_type_info_lock_held(const std::type_info &tp) {
209209
const auto &locals = get_local_internals().registered_types_cpp;
210210
auto it = locals.find(&tp);
211211
if (it != locals.end()) {
@@ -214,48 +214,59 @@ inline detail::type_info *get_local_type_info(const std::type_info &tp) {
214214
return nullptr;
215215
}
216216

217-
inline detail::type_info *get_global_type_info(const std::type_info &tp) {
217+
inline detail::type_info *get_local_type_info(const std::type_info &tp) {
218+
// NB: internals and local_internals share a single mutex
219+
PYBIND11_LOCK_INTERNALS(get_internals());
220+
return get_local_type_info_lock_held(tp);
221+
}
222+
223+
inline detail::type_info *get_global_type_info_lock_held(const std::type_info &tp) {
218224
// This is a two-level lookup. Hopefully we find the type info in
219225
// registered_types_cpp_fast, but if not we try
220226
// registered_types_cpp and fill registered_types_cpp_fast for
221227
// next time.
222-
return with_internals([&](internals &internals) {
223-
detail::type_info *type_info = nullptr;
228+
detail::type_info *type_info = nullptr;
229+
auto &internals = get_internals();
224230
#if PYBIND11_INTERNALS_VERSION >= 12
225-
auto &fast_types = internals.registered_types_cpp_fast;
231+
auto &fast_types = internals.registered_types_cpp_fast;
226232
#endif
227-
auto &types = internals.registered_types_cpp;
233+
auto &types = internals.registered_types_cpp;
228234
#if PYBIND11_INTERNALS_VERSION >= 12
229-
auto fast_it = fast_types.find(&tp);
230-
if (fast_it != fast_types.end()) {
235+
auto fast_it = fast_types.find(&tp);
236+
if (fast_it != fast_types.end()) {
231237
# ifndef NDEBUG
232-
auto types_it = types.find(std::type_index(tp));
233-
assert(types_it != types.end());
234-
assert(types_it->second == fast_it->second);
238+
auto types_it = types.find(std::type_index(tp));
239+
assert(types_it != types.end());
240+
assert(types_it->second == fast_it->second);
235241
# endif
236-
return fast_it->second;
237-
}
242+
return fast_it->second;
243+
}
238244
#endif // PYBIND11_INTERNALS_VERSION >= 12
239245

240-
auto it = types.find(std::type_index(tp));
241-
if (it != types.end()) {
246+
auto it = types.find(std::type_index(tp));
247+
if (it != types.end()) {
242248
#if PYBIND11_INTERNALS_VERSION >= 12
243-
fast_types.emplace(&tp, it->second);
249+
fast_types.emplace(&tp, it->second);
244250
#endif
245-
type_info = it->second;
246-
}
247-
return type_info;
248-
});
251+
type_info = it->second;
252+
}
253+
return type_info;
254+
}
255+
256+
inline detail::type_info *get_global_type_info(const std::type_info &tp) {
257+
PYBIND11_LOCK_INTERNALS(get_internals());
258+
return get_global_type_info_lock_held(tp);
249259
}
250260

251261
/// Return the type info for a given C++ type; on lookup failure can either throw or return
252262
/// nullptr.
253263
PYBIND11_NOINLINE detail::type_info *get_type_info(const std::type_info &tp,
254264
bool throw_if_missing = false) {
255-
if (auto *ltype = get_local_type_info(tp)) {
265+
PYBIND11_LOCK_INTERNALS(get_internals());
266+
if (auto *ltype = get_local_type_info_lock_held(tp)) {
256267
return ltype;
257268
}
258-
if (auto *gtype = get_global_type_info(tp)) {
269+
if (auto *gtype = get_global_type_info_lock_held(tp)) {
259270
return gtype;
260271
}
261272

0 commit comments

Comments
 (0)