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

bpo-31095: fix potential crash during GC. #2974

Merged
merged 4 commits into from
Aug 24, 2017
Merged
Changes from 1 commit
Commits
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
bpo-31095: Document PyObject_GC_UnTrack requirement
methane committed Aug 3, 2017
commit 4e97f02130839052dba633382912214e73084653
29 changes: 20 additions & 9 deletions Doc/extending/newtypes.rst
Original file line number Diff line number Diff line change
@@ -728,8 +728,9 @@ functions. With :c:func:`Py_VISIT`, :c:func:`Noddy_traverse` can be simplified:
uniformity across these boring implementations.

We also need to provide a method for clearing any subobjects that can
participate in cycles. We implement the method and reimplement the deallocator
to use it::
participate in cycles.

::

static int
Noddy_clear(Noddy *self)
@@ -747,13 +748,6 @@ to use it::
return 0;
}

static void
Noddy_dealloc(Noddy* self)
{
Noddy_clear(self);
Py_TYPE(self)->tp_free((PyObject*)self);
}

Notice the use of a temporary variable in :c:func:`Noddy_clear`. We use the
temporary variable so that we can set each member to *NULL* before decrementing
its reference count. We do this because, as was discussed earlier, if the
@@ -776,6 +770,23 @@ be simplified::
return 0;
}

Note that :c:func:`Noddy_dealloc` may call arbitrary functions through
``__del__`` method or weakref callback. It means circular GC can be
triggered inside the function. Since GC assumes reference count is not zero,
we need to untrack the object from GC by calling :c:func:`PyObject_GC_UnTrack`
before clearing members. Here is reimplemented deallocator which uses
:c:func:`PyObject_GC_UnTrack` and :c:func:`Noddy_clear`.

::

static void
Noddy_dealloc(Noddy* self)
{
PyObject_GC_UnTrack(self);
Noddy_clear(self);
Py_TYPE(self)->tp_free((PyObject*)self);
}

Finally, we add the :const:`Py_TPFLAGS_HAVE_GC` flag to the class flags::

Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, /* tp_flags */
1 change: 1 addition & 0 deletions Doc/includes/noddy4.c
Original file line number Diff line number Diff line change
@@ -46,6 +46,7 @@ Noddy_clear(Noddy *self)
static void
Noddy_dealloc(Noddy* self)
{
PyObject_GC_UnTrack(self);
Noddy_clear(self);
Py_TYPE(self)->tp_free((PyObject*)self);
}