Skip to content

Commit

Permalink
Change internals ID and versioning scheme to avoid module conflicts
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dean0x7d committed Aug 23, 2017
1 parent 024932b commit 96997a4
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 4 deletions.
2 changes: 1 addition & 1 deletion include/pybind11/cast.h
Expand Up @@ -603,7 +603,7 @@ class type_caster_generic {
/// 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;
Expand Down
19 changes: 17 additions & 2 deletions include/pybind11/detail/internals.h
Expand Up @@ -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<void *> registered_types_cpp; // std::type_index -> type_info
std::unordered_map<PyTypeObject *, std::vector<type_info *>> registered_types_py; // PyTypeObject* -> base type_info(s)
Expand All @@ -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;
Expand All @@ -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`.
Expand Down
2 changes: 1 addition & 1 deletion include/pybind11/pybind11.h
Expand Up @@ -870,7 +870,7 @@ class generic_type : public object {
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));
}
}

Expand Down

0 comments on commit 96997a4

Please sign in to comment.