From 96997a4b9d4ec3d389a570604394af5d5eee2557 Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Sun, 20 Aug 2017 17:14:09 +0200 Subject: [PATCH] Change internals ID and versioning scheme to avoid module conflicts The current PYBIND11_INTERNALS_ID depends on the version of the library in order to isolate binary incompatible internals capsules. However, this does not preclude conflicts between modules built from different (binary incompatible) commits with the same version number. For example, if one module was built with an early v2.2.dev and submitted to PyPI, it could not be loaded alongside a v2.2.x release module -- it would segfault because of incompatible internals with the same ID. This PR changes the ID to depend on PYBIND11_INTERNALS_VERSION which is independent of the main library version. It's an integer which should be incremented whenever a binary incompatible change is made to internals. PYBIND11_INTERNALS_KIND is also introduced for a similar reason. The same versioning scheme is also applied to `type_info` and the `module_local` type attribute. --- include/pybind11/cast.h | 2 +- include/pybind11/detail/internals.h | 19 +++++++++++++++++-- include/pybind11/pybind11.h | 2 +- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 4c3abc1c..537b217e 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -603,7 +603,7 @@ public: /// Try to load with foreign typeinfo, if available. Used when there is no /// native typeinfo, or when the native one wasn't able to produce a value. PYBIND11_NOINLINE bool try_load_foreign_module_local(handle src) { - constexpr auto *local_key = "_pybind11_module_local_typeinfo"; + constexpr auto *local_key = PYBIND11_MODULE_LOCAL_ID; const auto pytype = src.get_type(); if (!hasattr(pytype, local_key)) return false; diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 73f34df5..f1881fa4 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -62,6 +62,8 @@ struct overload_hash { }; /// Internal data structure used to track registered instances and types. +/// Whenever binary incompatible changes are made to this structure, +/// `PYBIND11_INTERNALS_VERSION` must be incremented. struct internals { type_map registered_types_cpp; // std::type_index -> type_info std::unordered_map> registered_types_py; // PyTypeObject* -> base type_info(s) @@ -83,6 +85,7 @@ struct internals { }; /// Additional type information which does not fit into the PyTypeObject. +/// Changes to this struct also require bumping `PYBIND11_INTERNALS_VERSION`. struct type_info { PyTypeObject *type; const std::type_info *cpptype; @@ -107,8 +110,20 @@ struct type_info { bool module_local : 1; }; -#define PYBIND11_INTERNALS_ID "__pybind11_" \ - PYBIND11_TOSTRING(PYBIND11_VERSION_MAJOR) "_" PYBIND11_TOSTRING(PYBIND11_VERSION_MINOR) "__" +/// Tracks the `internals` and `type_info` ABI version independent of the main library version +#define PYBIND11_INTERNALS_VERSION 1 + +#if defined(WITH_THREAD) +# define PYBIND11_INTERNALS_KIND "" +#else +# define PYBIND11_INTERNALS_KIND "_without_thread" +#endif + +#define PYBIND11_INTERNALS_ID "__pybind11_internals_v" \ + PYBIND11_TOSTRING(PYBIND11_INTERNALS_VERSION) PYBIND11_INTERNALS_KIND "__" + +#define PYBIND11_MODULE_LOCAL_ID "__pybind11_module_local_v" \ + PYBIND11_TOSTRING(PYBIND11_INTERNALS_VERSION) PYBIND11_INTERNALS_KIND "__" /// Each module locally stores a pointer to the `internals` data. The data /// itself is shared among modules with the same `PYBIND11_INTERNALS_ID`. diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 5c78abee..366ad81f 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -870,7 +870,7 @@ protected: if (rec.module_local) { // Stash the local typeinfo and loader so that external modules can access it. tinfo->module_local_load = &type_caster_generic::local_load; - setattr(m_ptr, "_pybind11_module_local_typeinfo", capsule(tinfo)); + setattr(m_ptr, PYBIND11_MODULE_LOCAL_ID, capsule(tinfo)); } }