[pygobject/pygobject-2-28] Fix GC-related crash during PyGObject deallocation

[Daniel Drake <dsd src gnome org>]

commit eca590a3ff6bbfb78eef7b88d36cee59eb51b1d0
Author: Daniel Drake <dsd laptop org>
Date:   Fri Jun 3 16:59:15 2011 +0100

    Fix GC-related crash during PyGObject deallocation
    
    Python-2.7.1's GC source has the following comment:
    
            /* Python's cyclic gc should never see an incoming refcount
             * of 0:  if something decref'ed to 0, it should have been
             * deallocated immediately at that time.
             * Possible cause (if the assert triggers):  a tp_dealloc
             * routine left a gc-aware object tracked during its teardown
             * phase, and did something-- or allowed something to happen --
             * that called back into Python.  gc can trigger then, and may
             * see the still-tracked dying object.  Before this assert
             * was added, such mistakes went on to allow gc to try to
             * delete the object again.  In a debug build, that caused
             * a mysterious segfault, when _Py_ForgetReference tried
             * to remove the object from the doubly-linked list of all
             * objects a second time.  In a release build, an actual
             * double deallocation occurred, which leads to corruption
             * of the allocator's internal bookkeeping pointers.  That's
             * so serious that maybe this should be a release-build
             * check instead of an assert?
             */
    
    As shown in a backtrace at
    https://bugzilla.redhat.com/show_bug.cgi?id=640972 , pygobject is making
    this exact mistake. Before untracking its object, pygobject_dealloc
    calls PyObject_ClearWeakRefs() which can call back into python, create
    new allocations, and trigger the GC.
    
    This is causing Sugar (based on pygobject2 + pygtk2 static bindings) to
    crash on a regular basis while interacting with widgets or launching
    applications.
    
    Fix this by untracking the object early. Also fix the same issue spotted
    in the GSource wrapper.
    
    Thanks to Bernie Innocenti for initial diagnosis.

 glib/pygsource.c    |    6 ++++--
 gobject/pygobject.c |    8 +++++++-
 2 files changed, 11 insertions(+), 3 deletions(-)
---
diff --git a/glib/pygsource.c b/glib/pygsource.c
index 684711e..d0176ab 100644
--- a/glib/pygsource.c
+++ b/glib/pygsource.c
@@ -387,10 +387,12 @@ pyg_source_clear(PyGSource *self)
 static void
 pyg_source_dealloc(PyGSource *self)
 {
-    PyObject_ClearWeakRefs((PyObject *)self);
-
+    /* Must be done first, so that there is no chance of Python's GC being
+     * called while tracking this half-deallocated object */
     PyObject_GC_UnTrack((PyObject *)self);
 
+    PyObject_ClearWeakRefs((PyObject *)self);
+
     pyg_source_clear(self);
 
     PyObject_GC_Del(self);
diff --git a/gobject/pygobject.c b/gobject/pygobject.c
index e6dfbc4..6c2f06c 100644
--- a/gobject/pygobject.c
+++ b/gobject/pygobject.c
@@ -1039,8 +1039,14 @@ PYGLIB_DEFINE_TYPE("gobject.GObject", PyGObject_Type, PyGObject);
 static void
 pygobject_dealloc(PyGObject *self)
 {
-    PyObject_ClearWeakRefs((PyObject *)self);
+    /* Untrack must be done first. This is because followup calls such as
+     * ClearWeakRefs could call into Python and cause new allocations to
+     * happen, which could in turn could trigger the garbage collector,
+     * which would then get confused as it is tracking this half-deallocated
+     * object. */
     PyObject_GC_UnTrack((PyObject *)self);
+
+    PyObject_ClearWeakRefs((PyObject *)self);
       /* this forces inst_data->type to be updated, which could prove
        * important if a new wrapper has to be created and it is of a
        * unregistered type */