Contents of /trunk/pygobject/patches/pygobject-2.28.6-deallocation-gc-crash.patch
Parent Directory | Revision Log
Revision 1697 -
(show annotations)
(download)
Tue Mar 13 08:51:22 2012 UTC (12 years, 6 months ago) by niro
File size: 3584 byte(s)
Tue Mar 13 08:51:22 2012 UTC (12 years, 6 months ago) by niro
File size: 3584 byte(s)
-added patch to fix a crash
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 |