pygobject/patches/pygobject-2.28.6-deallocation-gc-crash.patch

From 92aca4416a7930e5870b8d1a4016bae8140462ee Mon Sep 17 00:00:00 2001
From: Daniel Drake <dsd@laptop.org>
Date: Fri, 3 Jun 2011 16:59:15 +0100
Subject: [PATCH] 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 0faf221..3193890 100644
--- a/gobject/pygobject.c
+++ b/gobject/pygobject.c
@@ -1028,8 +1028,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 */
-- 
1.7.5.2

1	From 92aca4416a7930e5870b8d1a4016bae8140462ee Mon Sep 17 00:00:00 2001
2	From: Daniel Drake <dsd@laptop.org>
3	Date: Fri, 3 Jun 2011 16:59:15 +0100
4	Subject: [PATCH] Fix GC-related crash during PyGObject deallocation
5
6	Python-2.7.1's GC source has the following comment:
7
8	/* Python's cyclic gc should never see an incoming refcount
9	* of 0: if something decref'ed to 0, it should have been
10	* deallocated immediately at that time.
11	* Possible cause (if the assert triggers): a tp_dealloc
12	* routine left a gc-aware object tracked during its teardown
13	* phase, and did something-- or allowed something to happen --
14	* that called back into Python. gc can trigger then, and may
15	* see the still-tracked dying object. Before this assert
16	* was added, such mistakes went on to allow gc to try to
17	* delete the object again. In a debug build, that caused
18	* a mysterious segfault, when _Py_ForgetReference tried
19	* to remove the object from the doubly-linked list of all
20	* objects a second time. In a release build, an actual
21	* double deallocation occurred, which leads to corruption
22	* of the allocator's internal bookkeeping pointers. That's
23	* so serious that maybe this should be a release-build
24	* check instead of an assert?
25	*/
26
27	As shown in a backtrace at
28	https://bugzilla.redhat.com/show_bug.cgi?id=640972 , pygobject is making
29	this exact mistake. Before untracking its object, pygobject_dealloc
30	calls PyObject_ClearWeakRefs() which can call back into python, create
31	new allocations, and trigger the GC.
32
33	This is causing Sugar (based on pygobject2 + pygtk2 static bindings) to
34	crash on a regular basis while interacting with widgets or launching
35	applications.
36
37	Fix this by untracking the object early. Also fix the same issue spotted
38	in the GSource wrapper.
39
40	Thanks to Bernie Innocenti for initial diagnosis.
41	---
42	glib/pygsource.c \| 6 ++++--
43	gobject/pygobject.c \| 8 +++++++-
44	2 files changed, 11 insertions(+), 3 deletions(-)
45
46	diff --git a/glib/pygsource.c b/glib/pygsource.c
47	index 684711e..d0176ab 100644
48	--- a/glib/pygsource.c
49	+++ b/glib/pygsource.c
50	@@ -387,10 +387,12 @@ pyg_source_clear(PyGSource *self)
51	static void
52	pyg_source_dealloc(PyGSource *self)
53	{
54	- PyObject_ClearWeakRefs((PyObject *)self);
55	-
56	+ /* Must be done first, so that there is no chance of Python's GC being
57	+ * called while tracking this half-deallocated object */
58	PyObject_GC_UnTrack((PyObject *)self);
59
60	+ PyObject_ClearWeakRefs((PyObject *)self);
61	+
62	pyg_source_clear(self);
63
64	PyObject_GC_Del(self);
65	diff --git a/gobject/pygobject.c b/gobject/pygobject.c
66	index 0faf221..3193890 100644
67	--- a/gobject/pygobject.c
68	+++ b/gobject/pygobject.c
69	@@ -1028,8 +1028,14 @@ PYGLIB_DEFINE_TYPE("gobject.GObject", PyGObject_Type, PyGObject);
70	static void
71	pygobject_dealloc(PyGObject *self)
72	{
73	- PyObject_ClearWeakRefs((PyObject *)self);
74	+ /* Untrack must be done first. This is because followup calls such as
75	+ * ClearWeakRefs could call into Python and cause new allocations to
76	+ * happen, which could in turn could trigger the garbage collector,
77	+ * which would then get confused as it is tracking this half-deallocated
78	+ * object. */
79	PyObject_GC_UnTrack((PyObject *)self);
80	+
81	+ PyObject_ClearWeakRefs((PyObject *)self);
82	/* this forces inst_data->type to be updated, which could prove
83	* important if a new wrapper has to be created and it is of a
84	* unregistered type */
85	--
86	1.7.5.2
87