From c47d498c3546008c3b663c0449ec3421bd3d0382 Mon Sep 17 00:00:00 2001 From: Wenzel Jakob Date: Mon, 11 Jul 2016 23:40:28 +0200 Subject: [PATCH] fix rare GC issue during type creation (fixes #277) --- docs/changelog.rst | 11 ++++-- include/pybind11/pybind11.h | 78 +++++++++++++++++++++---------------- 2 files changed, 52 insertions(+), 37 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 5fff177bd..684036c58 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,12 +21,15 @@ Breaking changes queued for v2.0.0 (Not yet released) * Added ``eval`` and ``eval_file`` functions for evaluating expressions and statements from a string or file * eigen.h type converter fixed for non-contiguous arrays (e.g. slices) -* print more informative error messages when ``make_tuple()`` or ``cast()`` fail +* Print more informative error messages when ``make_tuple()`` or ``cast()`` fail * ``std::enable_shared_from_this<>`` now also works for ``const`` values -* a return value policy can now be passed to ``handle::operator()`` -* ``make_iterator()`` improvements for better compatibility with various types (now uses prefix increment operator) +* A return value policy can now be passed to ``handle::operator()`` +* ``make_iterator()`` improvements for better compatibility with various types + (now uses prefix increment operator) * ``arg()`` now accepts a wider range of argument types for default values -* various minor improvements of library internals (no user-visible changes) +* Fixed a rare but potentially very severe issue when the garbage collector ran + during pybind11 type creation. +* Various minor improvements of library internals (no user-visible changes) 1.8.0 (June 14, 2016) ---------------------- diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index d41e48097..5718d031b 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -559,8 +559,40 @@ protected: pybind11_fail("generic_type: type \"" + std::string(rec->name) + "\" is already registered!"); - object type_holder(PyType_Type.tp_alloc(&PyType_Type, 0), false); object name(PYBIND11_FROM_STRING(rec->name), false); + object scope_module; + if (rec->scope) { + scope_module = (object) rec->scope.attr("__module__"); + if (!scope_module) + scope_module = (object) rec->scope.attr("__name__"); + } + +#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3 + /* Qualified names for Python >= 3.3 */ + object scope_qualname; + if (rec->scope) + scope_qualname = (object) rec->scope.attr("__qualname__"); + object ht_qualname; + if (scope_qualname) { + ht_qualname = object(PyUnicode_FromFormat( + "%U.%U", scope_qualname.ptr(), name.ptr()), false); + } else { + ht_qualname = name; + } +#endif + std::string full_name = (scope_module ? ((std::string) scope_module.str() + "." + rec->name) + : std::string(rec->name)); + + char *tp_doc = nullptr; + if (rec->doc) { + /* Allocate memory for docstring (using PyObject_MALLOC, since + Python will free this later on) */ + size_t size = strlen(rec->doc) + 1; + tp_doc = (char *) PyObject_MALLOC(size); + memcpy((void *) tp_doc, rec->doc, size); + } + + object type_holder(PyType_Type.tp_alloc(&PyType_Type, 0), false); auto type = (PyHeapTypeObject*) type_holder.ptr(); if (!type_holder || !name) @@ -574,36 +606,18 @@ protected: internals.registered_types_cpp[tindex] = tinfo; internals.registered_types_py[type] = tinfo; - object scope_module; - if (rec->scope) { - scope_module = (object) rec->scope.attr("__module__"); - if (!scope_module) - scope_module = (object) rec->scope.attr("__name__"); - } - - std::string full_name = (scope_module ? ((std::string) scope_module.str() + "." + rec->name) - : std::string(rec->name)); /* Basic type attributes */ type->ht_type.tp_name = strdup(full_name.c_str()); type->ht_type.tp_basicsize = (ssize_t) rec->instance_size; type->ht_type.tp_base = (PyTypeObject *) rec->base_handle.ptr(); rec->base_handle.inc_ref(); -#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3 - /* Qualified names for Python >= 3.3 */ - object scope_qualname; - if (rec->scope) - scope_qualname = (object) rec->scope.attr("__qualname__"); - if (scope_qualname) { - type->ht_qualname = PyUnicode_FromFormat( - "%U.%U", scope_qualname.ptr(), name.ptr()); - } else { - type->ht_qualname = name.ptr(); - name.inc_ref(); - } -#endif type->ht_name = name.release().ptr(); +#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3 + type->ht_qualname = ht_qualname.release().ptr(); +#endif + /* Supported protocols */ type->ht_type.tp_as_number = &type->as_number; type->ht_type.tp_as_sequence = &type->as_sequence; @@ -624,13 +638,7 @@ protected: #endif type->ht_type.tp_flags &= ~Py_TPFLAGS_HAVE_GC; - if (rec->doc) { - /* Allocate memory for docstring (using PyObject_MALLOC, since - Python will free this later on) */ - size_t size = strlen(rec->doc) + 1; - type->ht_type.tp_doc = (char *) PyObject_MALLOC(size); - memcpy((void *) type->ht_type.tp_doc, rec->doc, size); - } + type->ht_type.tp_doc = tp_doc; if (PyType_Ready(&type->ht_type) < 0) pybind11_fail("generic_type: PyType_Ready failed!"); @@ -654,17 +662,21 @@ protected: if (ob_type == &PyType_Type) { std::string name_ = std::string(ht_type.tp_name) + "__Meta"; - object type_holder(PyType_Type.tp_alloc(&PyType_Type, 0), false); +#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3 + object ht_qualname(PyUnicode_FromFormat( + "%U__Meta", ((object) attr("__qualname__")).ptr()), false); +#endif object name(PYBIND11_FROM_STRING(name_.c_str()), false); + object type_holder(PyType_Type.tp_alloc(&PyType_Type, 0), false); if (!type_holder || !name) pybind11_fail("generic_type::metaclass(): unable to create type object!"); auto type = (PyHeapTypeObject*) type_holder.ptr(); type->ht_name = name.release().ptr(); + #if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3 /* Qualified names for Python >= 3.3 */ - type->ht_qualname = PyUnicode_FromFormat( - "%U__Meta", ((object) attr("__qualname__")).ptr()); + type->ht_qualname = ht_qualname.release().ptr(); #endif type->ht_type.tp_name = strdup(name_.c_str()); type->ht_type.tp_base = ob_type;